Title: [211940] releases/WebKitGTK/webkit-2.14/Source/WebCore
Revision
211940
Author
[email protected]
Date
2017-02-09 00:55:52 -0800 (Thu, 09 Feb 2017)

Log Message

Merge r210288 - Correct DOMWindow handling during FrameLoader::clear
https://bugs.webkit.org/show_bug.cgi?id=166357
<rdar://problem/29741862>

Reviewed by Andy Estes.

Make sure that we always clean up the DOM window when clearing Window properties, even if the document will
remain in the page cache. Since 'clearWindowShell' is only used in FrameLoader, divide it's beahvior into
two steps:

1. Rename 'clearWindowShell' to 'clearWIndowShellsNotMatchingDOMWindow' to better describe its function.
Switch to a modern C++ loop. Do not switch to the new DOMWindow here, but detach and clear existing
DOMWindow connections.

2. Add a new method 'setDOMWindowForWindowShell'. Complete switch to the new DOMWindow.

This change allows us to disconnect the old DOMWindow, perform the 'setDocument(nullptr)' operation, and then
connect to the new Window without leaving the loader in an inconsistent state.

* loader/bindings/js/ScriptController.cpp:
(WebCore::clearWindowShellsNotMatchingDOMWindow): Renamed from 'clearWindowShell'
(WebCore::setDOMWindowForWindowShell): Added.
* loader/bindings/js/ScriptController.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clear): Revise to use the new two-step DOMWindow switch logic.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (211939 => 211940)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog	2017-02-09 08:55:44 UTC (rev 211939)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog	2017-02-09 08:55:52 UTC (rev 211940)
@@ -1,3 +1,31 @@
+2016-01-04  Brent Fulgham  <[email protected]>
+
+        Correct DOMWindow handling during FrameLoader::clear
+        https://bugs.webkit.org/show_bug.cgi?id=166357
+        <rdar://problem/29741862>
+
+        Reviewed by Andy Estes.
+
+        Make sure that we always clean up the DOM window when clearing Window properties, even if the document will
+        remain in the page cache. Since 'clearWindowShell' is only used in FrameLoader, divide it's beahvior into
+        two steps:
+        
+        1. Rename 'clearWindowShell' to 'clearWIndowShellsNotMatchingDOMWindow' to better describe its function.
+        Switch to a modern C++ loop. Do not switch to the new DOMWindow here, but detach and clear existing
+        DOMWindow connections.
+
+        2. Add a new method 'setDOMWindowForWindowShell'. Complete switch to the new DOMWindow.
+
+        This change allows us to disconnect the old DOMWindow, perform the 'setDocument(nullptr)' operation, and then
+        connect to the new Window without leaving the loader in an inconsistent state.
+
+        * loader/bindings/js/ScriptController.cpp:
+        (WebCore::clearWindowShellsNotMatchingDOMWindow): Renamed from 'clearWindowShell'
+        (WebCore::setDOMWindowForWindowShell): Added.
+        * loader/bindings/js/ScriptController.h:
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::clear): Revise to use the new two-step DOMWindow switch logic.
+
 2016-12-18  Brent Fulgham  <[email protected]>
 
         Side effects while restting form elements

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/bindings/js/ScriptController.cpp (211939 => 211940)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/bindings/js/ScriptController.cpp	2017-02-09 08:55:44 UTC (rev 211939)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/bindings/js/ScriptController.cpp	2017-02-09 08:55:52 UTC (rev 211940)
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999-2001 Harri Porten ([email protected])
  *  Copyright (C) 2001 Peter Kelly ([email protected])
- *  Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
+ *  Copyright (C) 2006-2016 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
@@ -196,7 +196,7 @@
     static_cast<JSVMClientData*>(JSDOMWindow::commonVM().clientData)->getAllWorlds(worlds);
 }
 
-void ScriptController::clearWindowShell(DOMWindow* newDOMWindow, bool goingIntoPageCache)
+void ScriptController::clearWindowShellsNotMatchingDOMWindow(DOMWindow* newDOMWindow, bool goingIntoPageCache)
 {
     if (m_windowShells.isEmpty())
         return;
@@ -203,20 +203,35 @@
 
     JSLockHolder lock(JSDOMWindowBase::commonVM());
 
-    Vector<JSC::Strong<JSDOMWindowShell>> windowShells = this->windowShells();
-    for (size_t i = 0; i < windowShells.size(); ++i) {
-        JSDOMWindowShell* windowShell = windowShells[i].get();
-
+    for (auto& windowShell : windowShells()) {
         if (&windowShell->window()->wrapped() == newDOMWindow)
             continue;
 
         // Clear the debugger and console from the current window before setting the new window.
-        attachDebugger(windowShell, nullptr);
+        attachDebugger(windowShell.get(), nullptr);
         windowShell->window()->setConsoleClient(nullptr);
+        windowShell->window()->willRemoveFromWindowShell();
+    }
 
-        windowShell->window()->willRemoveFromWindowShell();
+    // It's likely that resetting our windows created a lot of garbage, unless
+    // it went in a back/forward cache.
+    if (!goingIntoPageCache)
+        collectGarbageAfterWindowShellDestruction();
+}
+
+void ScriptController::setDOMWindowForWindowShell(DOMWindow* newDOMWindow)
+{
+    if (m_windowShells.isEmpty())
+        return;
+    
+    JSLockHolder lock(JSDOMWindowBase::commonVM());
+    
+    for (auto& windowShell : windowShells()) {
+        if (&windowShell->window()->wrapped() == newDOMWindow)
+            continue;
+        
         windowShell->setWindow(newDOMWindow);
-
+        
         // An m_cacheableBindingRootObject persists between page navigations
         // so needs to know about the new JSDOMWindow.
         if (m_cacheableBindingRootObject)
@@ -223,16 +238,11 @@
             m_cacheableBindingRootObject->updateGlobalObject(windowShell->window());
 
         if (Page* page = m_frame.page()) {
-            attachDebugger(windowShell, page->debugger());
+            attachDebugger(windowShell.get(), page->debugger());
             windowShell->window()->setProfileGroup(page->group().identifier());
             windowShell->window()->setConsoleClient(&page->console());
         }
     }
-
-    // It's likely that resetting our windows created a lot of garbage, unless
-    // it went in a back/forward cache.
-    if (!goingIntoPageCache)
-        collectGarbageAfterWindowShellDestruction();
 }
 
 JSDOMWindowShell* ScriptController::initScript(DOMWrapperWorld& world)

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/bindings/js/ScriptController.h (211939 => 211940)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/bindings/js/ScriptController.h	2017-02-09 08:55:44 UTC (rev 211939)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/bindings/js/ScriptController.h	2017-02-09 08:55:52 UTC (rev 211940)
@@ -1,7 +1,7 @@
 /*
  *  Copyright (C) 1999 Harri Porten ([email protected])
  *  Copyright (C) 2001 Peter Kelly ([email protected])
- *  Copyright (C) 2008 Apple Inc. All rights reserved.
+ *  Copyright (C) 2008-2016 Apple Inc. All rights reserved.
  *  Copyright (C) 2008 Eric Seidel <[email protected]>
  *
  *  This library is free software; you can redistribute it and/or
@@ -135,7 +135,8 @@
 
     const String* sourceURL() const { return m_sourceURL; } // 0 if we are not evaluating any script
 
-    void clearWindowShell(DOMWindow* newDOMWindow, bool goingIntoPageCache);
+    void clearWindowShellsNotMatchingDOMWindow(DOMWindow*, bool goingIntoPageCache);
+    void setDOMWindowForWindowShell(DOMWindow*);
     void updateDocument();
 
     void namedItemAdded(HTMLDocument*, const AtomicString&) { }

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/FrameLoader.cpp (211939 => 211940)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/FrameLoader.cpp	2017-02-09 08:55:44 UTC (rev 211939)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/FrameLoader.cpp	2017-02-09 08:55:52 UTC (rev 211940)
@@ -604,7 +604,7 @@
     if (clearWindowProperties) {
         InspectorInstrumentation::frameWindowDiscarded(&m_frame, m_frame.document()->domWindow());
         m_frame.document()->domWindow()->resetUnlessSuspendedForDocumentSuspension();
-        m_frame.script().clearWindowShell(newDocument->domWindow(), m_frame.document()->inPageCache());
+        m_frame.script().clearWindowShellsNotMatchingDOMWindow(newDocument->domWindow(), m_frame.document()->inPageCache());
     }
 
     m_frame.selection().prepareForDestruction();
@@ -622,6 +622,9 @@
 
     subframeLoader().clear();
 
+    if (clearWindowProperties)
+        m_frame.script().setDOMWindowForWindowShell(newDocument->domWindow());
+
     if (clearScriptObjects)
         m_frame.script().clearScriptObjects();
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to