Attachment 372169 Details for Bug 198741 – Patch

[patch] Patch

bug-198741-20190614193748.patch (text/plain), 4.31 KB, created by Devin Rousso on 2019-06-14 19:37:49 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-06-14 19:37:49 PDT

Size: 4.31 KB

patch

obsolete

>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index f04e1bf5160b3ea092d62360c1f99da654d05b3b..e95706933b67527e25c34be459feb466c1301198 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-06-14  Devin Rousso  <drousso@apple.com>
>+
>+        waitForNavigationToComplete may be called before WebPageProxy knows it's loading
>+        https://bugs.webkit.org/show_bug.cgi?id=198741
>+        <rdar://problem/31164316>
>+
>+        Reviewed by Joseph Pecoraro.
>+
>+        There's a potential race in `WebAutomationSession::waitForNavigationToCompleteOnPage` when
>+        querying for the `WebPageProxy`'s loading state (via `PageLoadingState::isLoading`), in that
>+        a pending load may be committed _after_ the `WebAutomationSession` checks it's value. This
>+        makes the automation session think that it isn't loading, so it will continue running
>+        commands, which can lead to a JavaScript error ("Callback was not called before the unload
>+        event") as any injected scripts will be cleared by the impending navigation, leaving the
>+        script evaluation callbacks "dangling".
>+
>+        Expose more information from `PageLoadState` about whether it thinks there _may_ be a
>+        navigation currently happening, which the `WebAutomationSession` can use to delay commands.
>+
>+        In the best case, no navigations are "missed".
>+
>+        In the worst case, the automation session will wait `pageLoadTimeout` before continuing.
>+
>+        * UIProcess/Automation/WebAutomationSession.cpp:
>+        (WebKit::WebAutomationSession::waitForNavigationToCompleteOnPage):
>+
>+        * UIProcess/PageLoadState.h:
>+        * UIProcess/PageLoadState.cpp:
>+        (WebKit::PageLoadState::hasUncommittedLoad const): Added.
>+
> 2019-06-14  Youenn Fablet  <youenn@apple.com>
> 
>         WebProcessPool::clearWebProcessHasUploads cannot assume its given processIdentifier is valid
>diff --git a/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp b/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>index 2f9cfe9424026172d906dbd1a03c4cd01544bb8b..06226ea2ddb24a0a1930f93f7c18e4d8d9951fc6 100644
>--- a/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>+++ b/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>@@ -484,7 +484,7 @@ void WebAutomationSession::waitForNavigationToComplete(const String& browsingCon
> void WebAutomationSession::waitForNavigationToCompleteOnPage(WebPageProxy& page, Inspector::Protocol::Automation::PageLoadStrategy loadStrategy, Seconds timeout, Ref<Inspector::BackendDispatcher::CallbackBase>&& callback)
> {
>     ASSERT(!m_loadTimer.isActive());
>-    if (loadStrategy == Inspector::Protocol::Automation::PageLoadStrategy::None || !page.pageLoadState().isLoading()) {
>+    if (loadStrategy == Inspector::Protocol::Automation::PageLoadStrategy::None || (!page.pageLoadState().isLoading() && !page.pageLoadState().hasUncommittedLoad())) {
>         callback->sendSuccess(JSON::Object::create());
>         return;
>     }
>diff --git a/Source/WebKit/UIProcess/PageLoadState.cpp b/Source/WebKit/UIProcess/PageLoadState.cpp
>index d10148bacb7f47f2987dffa196be6907921448e2..10185a467764902d004f80c83767a1069510e3a5 100644
>--- a/Source/WebKit/UIProcess/PageLoadState.cpp
>+++ b/Source/WebKit/UIProcess/PageLoadState.cpp
>@@ -172,6 +172,11 @@ bool PageLoadState::isLoading() const
>     return isLoading(m_committedState);
> }
> 
>+bool PageLoadState::hasUncommittedLoad() const
>+{
>+    return isLoading(m_uncommittedState);
>+}
>+
> String PageLoadState::activeURL(const Data& data)
> {
>     // If there is a currently pending URL, it is the active URL,
>diff --git a/Source/WebKit/UIProcess/PageLoadState.h b/Source/WebKit/UIProcess/PageLoadState.h
>index 652d360f83b1ebc4cacf6bc94a418d09d19f1c1c..dbeaa9a1744a2cd90b58c457f816ae5989d47f7f 100644
>--- a/Source/WebKit/UIProcess/PageLoadState.h
>+++ b/Source/WebKit/UIProcess/PageLoadState.h
>@@ -125,6 +125,8 @@ public:
>     bool isCommitted() const { return m_committedState.state == State::Committed; }
>     bool isFinished() const { return m_committedState.state == State::Finished; }
> 
>+    bool hasUncommittedLoad() const;
>+
>     const String& provisionalURL() const { return m_committedState.provisionalURL; }
>     const String& url() const { return m_committedState.url; }
>     const String& unreachableURL() const { return m_committedState.unreachableURL; }

Actions: View | Formatted Diff | Diff

Attachments on bug 198741: 371820 | 372102 | 372169