Reinitialize isolated world window proxies on navigation if needed.

When a window proxy is initialized for a child frame, it must be
reinitialized on every subsequent navigation to correctly hook up the
window proxy (which may have cached JS references).

Previously, this was only happening for the main world window proxy. One
visible side effect is that if an extension cached a window proxy
reference to a frame that was then navigated, the cached reference would
not work until something else forced the window proxy to be
reinitialized.

Fixing this has an unintended side effect in an inspector protocol test
which logs context creation: since isolated world window proxies are
reinitialized in an arbitrary order (specifically, HashMap iteration
order), the expected results become arbitrary as well. Instead of hoping
that iteration order remains consistent, update the test to only log the
context creation events it actually cares about.

Change-Id: I11d7e13f7c2810d1c662ccf40f37c880e5026db5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2607680
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840983}

third_party/blink/renderer/bindings/core/v8/local_window_proxy.cc

@@ -184,8 +184,8 @@ void LocalWindowProxy::Initialize() {
 
   scoped_refptr<const SecurityOrigin> origin;
   if (world_->IsMainWorld()) {
-    // ActivityLogger for main world is updated within updateDocumentInternal().
-    UpdateDocumentInternal();
+    // This also updates the ActivityLogger for the main world.
+    UpdateDocumentForMainWorld();
     origin = GetFrame()->DomWindow()->GetSecurityOrigin();
   } else {
     UpdateActivityLogger();
@@ -448,7 +448,6 @@ void LocalWindowProxy::SetSecurityToken(const SecurityOrigin* origin) {
 }
 
 void LocalWindowProxy::UpdateDocument() {
-  DCHECK(world_->IsMainWorld());
   // For an uninitialized main window proxy, there's nothing we need
   // to update. The update is done when the window proxy gets initialized later.
   if (lifecycle_ == Lifecycle::kContextIsUninitialized)
@@ -464,10 +463,14 @@ void LocalWindowProxy::UpdateDocument() {
     return;
   }
 
-  UpdateDocumentInternal();
+  if (!world_->IsMainWorld())
+    return;
+
+  UpdateDocumentForMainWorld();
 }
 
-void LocalWindowProxy::UpdateDocumentInternal() {
+void LocalWindowProxy::UpdateDocumentForMainWorld() {
+  DCHECK(world_->IsMainWorld());
   UpdateActivityLogger();
   UpdateDocumentProperty();
   UpdateSecurityOrigin(GetFrame()->DomWindow()->GetSecurityOrigin());

third_party/blink/renderer/bindings/core/v8/local_window_proxy.h

@@ -92,9 +92,10 @@ class LocalWindowProxy final : public WindowProxy {
 
   // Triggers updates of objects that are associated with a Document:
   // - the activity logger
-  // - the document DOM wrapper
+  // - the document DOM wrapper (performance optimization for accessing
+  //   window.document in the main world)
   // - the security origin
-  void UpdateDocumentInternal();
+  void UpdateDocumentForMainWorld();
 
   // The JavaScript wrapper for the document object is cached on the global
   // object for fast access. UpdateDocumentProperty sets the wrapper

third_party/blink/renderer/bindings/core/v8/script_controller.cc

@@ -177,7 +177,7 @@ v8::ExtensionConfiguration ScriptController::ExtensionsFor(
 }
 
 void ScriptController::UpdateDocument() {
-  window_proxy_manager_->MainWorldProxyMaybeUninitialized()->UpdateDocument();
+  window_proxy_manager_->UpdateDocument();
 }
 
 void ScriptController::ExecuteJavaScriptURL(

third_party/blink/renderer/bindings/core/v8/window_proxy_manager.cc

@@ -114,6 +114,16 @@ WindowProxy* WindowProxyManager::WindowProxyMaybeUninitialized(
   return window_proxy;
 }
 
+void LocalWindowProxyManager::UpdateDocument() {
+  MainWorldProxyMaybeUninitialized()->UpdateDocument();
+
+  for (auto& entry : isolated_worlds_) {
+    auto* isolated_window_proxy =
+        static_cast<LocalWindowProxy*>(entry.value.Get());
+    isolated_window_proxy->UpdateDocument();
+  }
+}
+
 void LocalWindowProxyManager::UpdateSecurityOrigin(
     const SecurityOrigin* security_origin) {
   static_cast<LocalWindowProxy*>(window_proxy_.Get())

third_party/blink/renderer/bindings/core/v8/window_proxy_manager.h

@@ -93,6 +93,8 @@ class LocalWindowProxyManager
     return static_cast<LocalWindowProxy*>(window_proxy_.Get());
   }
 
+  void UpdateDocument();
+
   // Sets the given security origin to the main world's context.  Also updates
   // the security origin of the context for each isolated world.
   void UpdateSecurityOrigin(const SecurityOrigin*);

third_party/blink/renderer/bindings/core/v8/window_proxy_test.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "third_party/blink/public/web/web_script_source.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_request.h"
 #include "third_party/blink/renderer/core/testing/sim/sim_test.h"
 #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
@@ -49,4 +50,46 @@ TEST_F(WindowProxyTest, ReinitializedAfterNavigation) {
   ASSERT_GT(ConsoleMessages().size(), 0U);
   EXPECT_EQ("PASSED", ConsoleMessages()[0]);
 }
+
+TEST_F(WindowProxyTest, IsolatedWorldReinitializedAfterNavigation) {
+  SimRequest main_resource("https://example.com/index.html", "text/html");
+  LoadURL("https://example.com/index.html");
+  main_resource.Complete(R"HTML(
+    <!DOCTYPE html>
+    <html><body><iframe></iframe></body></html>
+  )HTML");
+
+  ASSERT_TRUE(MainFrame().FirstChild());
+
+  v8::HandleScope scope(v8::Isolate::GetCurrent());
+
+  const int32_t kIsolatedWorldId = 42;
+
+  // Save a reference to the top `window` in the isolated world.
+  v8::Local<v8::Value> window_top =
+      MainFrame().ExecuteScriptInIsolatedWorldAndReturnValue(
+          kIsolatedWorldId, WebScriptSource("window"));
+  ASSERT_TRUE(window_top->IsObject());
+
+  // Save a reference to the child frame's window proxy in the isolated world.
+  v8::Local<v8::Value> saved_child_window =
+      MainFrame().ExecuteScriptInIsolatedWorldAndReturnValue(
+          kIsolatedWorldId, WebScriptSource("saved = window[0]"));
+  ASSERT_TRUE(saved_child_window->IsObject());
+
+  frame_test_helpers::LoadFrame(MainFrame().FirstChild()->ToWebLocalFrame(),
+                                "data:text/html,<body><p>Hello</p></body>");
+  ASSERT_TRUE(MainFrame().FirstChild());
+
+  // Test if the window proxy of the navigated frame was reinitialized. The
+  // `top` attribute of the saved child frame's window proxy reference should
+  // refer to the same object as the top-level window proxy reference that was
+  // cached earlier.
+  v8::Local<v8::Value> top_via_saved =
+      MainFrame().ExecuteScriptInIsolatedWorldAndReturnValue(
+          kIsolatedWorldId, WebScriptSource("saved.top"));
+  EXPECT_TRUE(top_via_saved->IsObject());
+  EXPECT_TRUE(window_top->StrictEquals(top_via_saved));
+}
+
 }  // namespace blink

third_party/blink/web_tests/inspector-protocol/page/add-script-to-evaluate-in-world.js

@@ -6,9 +6,13 @@
 
   const scriptIds = [];
   dp.Runtime.onConsoleAPICalled(msg => testRunner.log(msg.params.args[0].value));
-  dp.Runtime.onExecutionContextCreated(msg => {
+  let logContextCreationCallback = msg => {
     if (msg.params.context.name.includes('world'))
       testRunner.log(msg.params.context.name);
+  };
+  dp.Runtime.onExecutionContextCreated(msg => {
+    if (logContextCreationCallback)
+      logContextCreationCallback(msg);
   });
 
   testRunner.log('Adding scripts');
@@ -27,6 +31,7 @@
       testRunner.log('Failed script removal');
   }
 
+  logContextCreationCallback = null;
   await session.navigate('../resources/blank.html');
 
   testRunner.completeTest();