Reland "Reinitialize isolated world window proxies on navigation if needed."

This is a reland of d24a515a, which
triggered false positives in the Blink leak detector. Now that isolated
worlds are correctly reinitialized after a navigation, they remains live
as long as the associated browsing context is live, persisting across
navigations. At the end of the test, the leak detector navigates to an
empty page and performs a comparison of live objects against the
captured baseline, causing leak detection  to incorrectly report the
isolated world contexts as leaked. To address this, the web test harness
now also disposes any isolated worlds created for a test when resetting
the state at the end.

Original change's description:
> 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}

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

content/web_test/renderer/web_frame_test_proxy.cc

@@ -263,7 +263,7 @@ void WebFrameTestProxy::Reset() {
     GetWebFrame()->SetName(blink::WebString());
     GetWebFrame()->ClearOpener();
 
-    blink::WebTestingSupport::ResetInternalsObject(GetWebFrame());
+    blink::WebTestingSupport::ResetMainFrame(GetWebFrame());
     // Resetting the internals object also overrides the WebPreferences, so we
     // have to sync them to WebKit again.
     blink::WebView* web_view = GetWebFrame()->View();

third_party/blink/public/web/web_testing_support.h

@@ -42,9 +42,12 @@ class WebTestingSupport {
   // Injects the |internals| object into the frame for Javascript to use.
   static void InjectInternalsObject(WebLocalFrame*);
   static void InjectInternalsObject(v8::Local<v8::Context>);
-  // Resets the state of the |internals| object, and things that it modifies,
-  // back to their starting state at the end of a test.
-  static void ResetInternalsObject(WebLocalFrame*);
+  // Resets state on the main frame once a test is complete, including:
+  // - disposing any isolated worlds created by the test.
+  // - resetting the state of the |internals| object, and things that it
+  // modifies,
+  //   back to their starting state
+  static void ResetMainFrame(WebLocalFrame*);
 
   // Use this to install a mock scrollbar theme for tests. To use, simply
   // inherit your test class from this or instantiate it manually. The

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

@@ -169,8 +169,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();
@@ -433,7 +433,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)
@@ -449,10 +448,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

@@ -68,6 +68,13 @@ void WindowProxyManager::SetGlobalProxies(
     WindowProxyMaybeUninitialized(*entry.first)->InitializeIfNeeded();
 }
 
+void WindowProxyManager::ResetIsolatedWorldsForTesting() {
+  for (auto& world_info : isolated_worlds_) {
+    world_info.value->ClearForClose();
+  }
+  isolated_worlds_.clear();
+}
+
 WindowProxyManager::WindowProxyManager(Frame& frame, FrameType frame_type)
     : isolate_(V8PerIsolateData::MainThreadIsolate()),
       frame_(&frame),
@@ -114,6 +121,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

@@ -27,7 +27,7 @@ class WindowProxyManager : public GarbageCollected<WindowProxyManager> {
   v8::Isolate* GetIsolate() const { return isolate_; }
 
   void ClearForClose();
-  void CORE_EXPORT ClearForNavigation();
+  CORE_EXPORT void ClearForNavigation();
   void ClearForSwap();
   void ClearForV8MemoryPurge();
 
@@ -45,6 +45,8 @@ class WindowProxyManager : public GarbageCollected<WindowProxyManager> {
     return window_proxy;
   }
 
+  CORE_EXPORT void ResetIsolatedWorldsForTesting();
+
  protected:
   using IsolatedWorldMap = HeapHashMap<int, Member<WindowProxy>>;
   enum class FrameType { kLocal, kRemote };
@@ -93,6 +95,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/renderer/modules/exported/BUILD.gn

@@ -31,6 +31,7 @@ static_library("test_support") {
   deps = [
     "//skia",
     "//third_party/blink/renderer/bindings/modules/v8:testing",
+    "//third_party/blink/renderer/core",
     "//third_party/blink/renderer/core:testing",
     "//third_party/blink/renderer/modules:modules_testing",
     "//third_party/blink/renderer/platform",

third_party/blink/renderer/modules/exported/web_testing_support.cc

@@ -25,7 +25,8 @@
 
 #include "third_party/blink/public/web/web_testing_support.h"
 
-#include "third_party/blink/public/web/web_local_frame.h"
+#include "third_party/blink/renderer/bindings/core/v8/window_proxy_manager.h"
+#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
 #include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
 #include "third_party/blink/renderer/core/testing/v8/web_core_test_support.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
@@ -108,9 +109,14 @@ void WebTestingSupport::InjectInternalsObject(v8::Local<v8::Context> context) {
   web_core_test_support::InjectInternalsObject(context);
 }
 
-void WebTestingSupport::ResetInternalsObject(WebLocalFrame* frame) {
+void WebTestingSupport::ResetMainFrame(WebLocalFrame* main_frame) {
+  auto* main_frame_impl = To<WebLocalFrameImpl>(main_frame);
   v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
-  web_core_test_support::ResetInternalsObject(frame->MainWorldScriptContext());
+  web_core_test_support::ResetInternalsObject(
+      main_frame_impl->MainWorldScriptContext());
+  main_frame_impl->GetFrame()
+      ->GetWindowProxyManager()
+      ->ResetIsolatedWorldsForTesting();
 }
 
 }  // namespace blink

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

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