- Revision
- 247017
- Author
- [email protected]
- Date
- 2019-07-01 13:29:20 -0700 (Mon, 01 Jul 2019)
Log Message
More judiciously handle clearing/creation of DOMWindows for new Documents.
<rdar://problem/51665406> and https://bugs.webkit.org/show_bug.cgi?id=198786
Reviewed by Chris Dumez.
* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::executeIfJavaScriptURL):
* loader/DocumentWriter.cpp:
(WebCore::DocumentWriter::replaceDocumentWithResultOfExecutingJavascriptURL): Rename for clarity.
(WebCore::DocumentWriter::begin): Handle DOMWindow taking/creation inside FrameLoader::clear via a lambda.
(WebCore::DocumentWriter::replaceDocument): Deleted.
* loader/DocumentWriter.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clear): Take a "handleDOMWindowCreation" lambda to run after clearing the previous document.
* loader/FrameLoader.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (247016 => 247017)
--- trunk/Source/WebCore/ChangeLog 2019-07-01 20:14:19 UTC (rev 247016)
+++ trunk/Source/WebCore/ChangeLog 2019-07-01 20:29:20 UTC (rev 247017)
@@ -1,3 +1,23 @@
+2019-07-01 Brady Eidson <[email protected]>
+
+ More judiciously handle clearing/creation of DOMWindows for new Documents.
+ <rdar://problem/51665406> and https://bugs.webkit.org/show_bug.cgi?id=198786
+
+ Reviewed by Chris Dumez.
+
+ * bindings/js/ScriptController.cpp:
+ (WebCore::ScriptController::executeIfJavaScriptURL):
+
+ * loader/DocumentWriter.cpp:
+ (WebCore::DocumentWriter::replaceDocumentWithResultOfExecutingJavascriptURL): Rename for clarity.
+ (WebCore::DocumentWriter::begin): Handle DOMWindow taking/creation inside FrameLoader::clear via a lambda.
+ (WebCore::DocumentWriter::replaceDocument): Deleted.
+ * loader/DocumentWriter.h:
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::clear): Take a "handleDOMWindowCreation" lambda to run after clearing the previous document.
+ * loader/FrameLoader.h:
+
2019-07-01 Zalan Bujtas <[email protected]>
[iPadOS] Tapping on the bottom part of youtube video behaves as if controls were visible
Modified: trunk/Source/WebCore/bindings/js/ScriptController.cpp (247016 => 247017)
--- trunk/Source/WebCore/bindings/js/ScriptController.cpp 2019-07-01 20:14:19 UTC (rev 247016)
+++ trunk/Source/WebCore/bindings/js/ScriptController.cpp 2019-07-01 20:29:20 UTC (rev 247017)
@@ -652,7 +652,7 @@
// DocumentWriter::replaceDocument can cause the DocumentLoader to get deref'ed and possible destroyed,
// so protect it with a RefPtr.
if (RefPtr<DocumentLoader> loader = m_frame.document()->loader())
- loader->writer().replaceDocument(scriptResult, ownerDocument.get());
+ loader->writer().replaceDocumentWithResultOfExecutingJavascriptURL(scriptResult, ownerDocument.get());
}
return true;
}
Modified: trunk/Source/WebCore/loader/DocumentWriter.cpp (247016 => 247017)
--- trunk/Source/WebCore/loader/DocumentWriter.cpp 2019-07-01 20:14:19 UTC (rev 247016)
+++ trunk/Source/WebCore/loader/DocumentWriter.cpp 2019-07-01 20:29:20 UTC (rev 247017)
@@ -61,7 +61,7 @@
// This is only called by ScriptController::executeIfJavaScriptURL
// and always contains the result of evaluating a _javascript_: url.
// This is the <iframe src="" case.
-void DocumentWriter::replaceDocument(const String& source, Document* ownerDocument)
+void DocumentWriter::replaceDocumentWithResultOfExecutingJavascriptURL(const String& source, Document* ownerDocument)
{
m_frame->loader().stopAllLoaders();
@@ -137,10 +137,6 @@
// FIXME: Do we need to consult the content security policy here about blocked plug-ins?
bool shouldReuseDefaultView = m_frame->loader().stateMachine().isDisplayingInitialEmptyDocument() && m_frame->document()->isSecureTransitionTo(url);
- if (shouldReuseDefaultView)
- document->takeDOMWindowFrom(*m_frame->document());
- else
- document->createDOMWindow();
// Temporarily extend the lifetime of the existing document so that FrameLoader::clear() doesn't destroy it as
// we need to retain its ongoing set of upgraded requests in new navigation contexts per <http://www.w3.org/TR/upgrade-insecure-requests/>
@@ -148,7 +144,14 @@
RefPtr<Document> existingDocument = m_frame->document();
auto* previousContentSecurityPolicy = existingDocument ? existingDocument->contentSecurityPolicy() : nullptr;
- m_frame->loader().clear(document.ptr(), !shouldReuseDefaultView, !shouldReuseDefaultView);
+ WTF::Function<void()> handleDOMWindowCreation = [this, document = document.copyRef(), url] {
+ if (m_frame->loader().stateMachine().isDisplayingInitialEmptyDocument() && m_frame->document()->isSecureTransitionTo(url))
+ document->takeDOMWindowFrom(*m_frame->document());
+ else
+ document->createDOMWindow();
+ };
+
+ m_frame->loader().clear(document.ptr(), !shouldReuseDefaultView, !shouldReuseDefaultView, true, WTFMove(handleDOMWindowCreation));
clear();
// m_frame->loader().clear() might fire unload event which could remove the view of the document.
Modified: trunk/Source/WebCore/loader/DocumentWriter.h (247016 => 247017)
--- trunk/Source/WebCore/loader/DocumentWriter.h 2019-07-01 20:14:19 UTC (rev 247016)
+++ trunk/Source/WebCore/loader/DocumentWriter.h 2019-07-01 20:29:20 UTC (rev 247017)
@@ -42,9 +42,7 @@
public:
DocumentWriter() = default;
- // This is only called by ScriptController::executeIfJavaScriptURL
- // and always contains the result of evaluating a _javascript_: url.
- void replaceDocument(const String&, Document* ownerDocument);
+ void replaceDocumentWithResultOfExecutingJavascriptURL(const String&, Document* ownerDocument);
bool begin();
bool begin(const URL&, bool dispatchWindowObjectAvailable = true, Document* ownerDocument = nullptr);
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (247016 => 247017)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2019-07-01 20:14:19 UTC (rev 247016)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2019-07-01 20:29:20 UTC (rev 247017)
@@ -626,15 +626,14 @@
return !newDocument.securityOrigin().isSameOriginAs(frame.document()->securityOrigin());
}
-void FrameLoader::clear(Document* newDocument, bool clearWindowProperties, bool clearScriptObjects, bool clearFrameView)
+void FrameLoader::clear(Document* newDocument, bool clearWindowProperties, bool clearScriptObjects, bool clearFrameView, WTF::Function<void()>&& handleDOMWindowCreation)
{
m_frame.editor().clear();
- if (!m_needsClear)
- return;
+ bool neededClear = m_needsClear;
m_needsClear = false;
-
- if (m_frame.document()->pageCacheState() != Document::InPageCache) {
+
+ if (neededClear && m_frame.document()->pageCacheState() != Document::InPageCache) {
m_frame.document()->cancelParsing();
m_frame.document()->stopActiveDOMObjects();
bool hadLivingRenderTree = m_frame.document()->hasLivingRenderTree();
@@ -643,6 +642,12 @@
m_frame.document()->adjustFocusedNodeOnNodeRemoval(*m_frame.document());
}
+ if (handleDOMWindowCreation)
+ handleDOMWindowCreation();
+
+ if (!neededClear)
+ return;
+
// Do this after detaching the document so that the unload event works.
if (clearWindowProperties) {
InspectorInstrumentation::frameWindowDiscarded(m_frame, m_frame.document()->domWindow());
Modified: trunk/Source/WebCore/loader/FrameLoader.h (247016 => 247017)
--- trunk/Source/WebCore/loader/FrameLoader.h 2019-07-01 20:14:19 UTC (rev 247016)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2019-07-01 20:29:20 UTC (rev 247017)
@@ -152,7 +152,7 @@
void cancelAndClear();
void clearProvisionalLoadForPolicyCheck();
// FIXME: clear() is trying to do too many things. We should break it down into smaller functions (ideally with fewer raw Boolean parameters).
- void clear(Document* newDocument, bool clearWindowProperties = true, bool clearScriptObjects = true, bool clearFrameView = true);
+ void clear(Document* newDocument, bool clearWindowProperties = true, bool clearScriptObjects = true, bool clearFrameView = true, WTF::Function<void()>&& handleDOMWindowCreation = nullptr);
bool isLoading() const;
WEBCORE_EXPORT bool frameHasLoaded() const;
