Attachment 372102 Details for Bug 198741 – Patch

[patch] Patch

bug-198741-20190613204243.patch (text/plain), 4.26 KB, created by Devin Rousso on 2019-06-13 20:42:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Devin Rousso

Created: 2019-06-13 20:42:44 PDT

Size: 4.26 KB

patch

obsolete

>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 0d99009f46444ce0945cd803b716d6aa1b115639..ead0cd8026b471359a4d18ecea559a7f65ef62dd 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-06-13  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::isLoadingUncommitted const): Added.
>+
> 2019-06-12  Antoine Quint  <graouts@apple.com>
> 
>         Show the web page URL when sharing an AR model
>diff --git a/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp b/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
>index 2f9cfe9424026172d906dbd1a03c4cd01544bb8b..b93107726e6313f9bd16be23aba1dc208073457f 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().hasPendingLoad())) {
>         callback->sendSuccess(JSON::Object::create());
>         return;
>     }
>diff --git a/Source/WebKit/UIProcess/PageLoadState.cpp b/Source/WebKit/UIProcess/PageLoadState.cpp
>index d10148bacb7f47f2987dffa196be6907921448e2..111c6bd361400af004aefe1b74189b2b0314150d 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::hasPendingLoad() 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..aceae5c102e82a672d9116f1deebbf48405d2b12 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 hasPendingLoad() 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