[wasm] Refine installation of the WebAssembly.Tag constructor

This makes the installation sequence of WebAssembly.Tag slightly
shorter, slightly faster, slightly cleaner in corner-case semantics,
and slightly better documented.

To allow testing this code, Isolate::InstallConditionalFeatures is
exposed as d8.test.installConditionalFeatures().

Fixed: chromium:1314616
Change-Id: I44285e398b8797e0e7d2d8c782cecec3ba68a503
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3582382
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79956}

src/d8/d8.cc

@@ -2183,6 +2183,12 @@ void Shell::TestVerifySourcePositions(
   }
 }
 
+void Shell::InstallConditionalFeatures(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  isolate->InstallConditionalFeatures(isolate->GetCurrentContext());
+}
+
 // async_hooks.createHook() registers functions to be called for different
 // lifetime events of each async operation.
 void Shell::AsyncHooksCreateHook(
@@ -3140,6 +3146,11 @@ Local<ObjectTemplate> Shell::CreateD8Template(Isolate* isolate) {
       test_template->Set(isolate, "LeafInterfaceType",
                          Shell::CreateLeafInterfaceTypeTemplate(isolate));
     }
+    // Allows testing code paths that are triggered when Origin Trials are
+    // added in the browser.
+    test_template->Set(
+        isolate, "installConditionalFeatures",
+        FunctionTemplate::New(isolate, Shell::InstallConditionalFeatures));
 
     d8_template->Set(isolate, "test", test_template);
   }

src/d8/d8.h

@@ -557,6 +557,9 @@ class Shell : public i::AllStatic {
   static void TestVerifySourcePositions(
       const v8::FunctionCallbackInfo<v8::Value>& args);
 
+  static void InstallConditionalFeatures(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
+
   static void AsyncHooksCreateHook(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void AsyncHooksExecutionAsyncId(

src/wasm/wasm-js.cc

@@ -2992,23 +2992,25 @@ void WasmJs::InstallConditionalFeatures(Isolate* isolate,
     MaybeHandle<Object> maybe_webassembly =
         JSObject::GetProperty(isolate, global, "WebAssembly");
     Handle<Object> webassembly_obj;
-    if (!maybe_webassembly.ToHandle(&webassembly_obj)) {
-      // There is not {WebAssembly} object. We just return without adding the
-      // {Tag} constructor.
-      return;
-    }
-    if (!webassembly_obj->IsJSObject()) {
-      // The {WebAssembly} object is invalid. As we cannot add the {Tag}
-      // constructor, we just return.
+    if (!maybe_webassembly.ToHandle(&webassembly_obj) ||
+        !webassembly_obj->IsJSObject()) {
+      // There is no {WebAssembly} object, or it's not what we expect.
+      // Just return without adding the {Tag} constructor.
       return;
     }
     Handle<JSObject> webassembly = Handle<JSObject>::cast(webassembly_obj);
-    // Setup Exception
+    // Setup Tag.
     Handle<String> tag_name = v8_str(isolate, "Tag");
+    // The {WebAssembly} object may already have been modified. The following
+    // code is designed to:
+    //  - check for existing {Tag} properties on the object itself, and avoid
+    //    overwriting them or adding duplicate properties
+    //  - disregard any setters or read-only properties on the prototype chain
+    //  - only make objects accessible to user code after all internal setup
+    //    has been completed.
     if (JSObject::HasOwnProperty(isolate, webassembly, tag_name)
             .FromMaybe(true)) {
-      // The {Exception} constructor already exists, there is nothing more to
-      // do.
+      // Existing property, or exception.
       return;
     }
 
@@ -3017,14 +3019,6 @@ void WasmJs::InstallConditionalFeatures(Isolate* isolate,
         CreateFunc(isolate, tag_name, WebAssemblyTag, has_prototype,
                    SideEffectType::kHasNoSideEffect);
     tag_constructor->shared().set_length(1);
-    auto result =
-        Object::SetProperty(isolate, webassembly, tag_name, tag_constructor,
-                            StoreOrigin::kNamed, Just(ShouldThrow::kDontThrow));
-    if (result.is_null()) {
-      // Setting the {Tag} constructor failed. We just bail out.
-      return;
-    }
-    // Install the constructor on the context.
     context->set_wasm_tag_constructor(*tag_constructor);
     Handle<JSObject> tag_proto =
         SetupConstructor(isolate, tag_constructor, i::WASM_TAG_OBJECT_TYPE,
@@ -3032,6 +3026,12 @@ void WasmJs::InstallConditionalFeatures(Isolate* isolate,
     if (enabled_features.has_type_reflection()) {
       InstallFunc(isolate, tag_proto, "type", WebAssemblyTagType, 0);
     }
+    LookupIterator it(isolate, webassembly, tag_name, LookupIterator::OWN);
+    Maybe<bool> result = JSObject::DefineOwnPropertyIgnoreAttributes(
+        &it, tag_constructor, DONT_ENUM, Just(kDontThrow));
+    // This could still fail if the object was non-extensible, but now we
+    // return anyway so there's no need to even check.
+    USE(result);
   }
 }
 #undef ASSIGN