Attachment 349678 Details for Bug 189568 – Patch

[patch] Patch

bug-189568-20180913102437.patch (text/plain), 3.27 KB, created by Keith Rollin on 2018-09-13 10:24:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Rollin

Created: 2018-09-13 10:24:37 PDT

Size: 3.27 KB

patch

obsolete

>Subversion Revision: 235975
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e0b90ceb73f3fc56b24112632d5f0478b8e3e9e9..f4e582ceafa16373f3c1ebde5bf0648245443c06 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-09-12  Keith Rollin  <krollin@apple.com>
>+
>+        WebPageProxy::reportPageLoadResult can crash on some code paths
>+        https://bugs.webkit.org/show_bug.cgi?id=189568
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        WebPageProxy::reportPageLoadResult (which is called from
>+        WebPageProxy::didFinishLoadForFrame) can sometimes crash when
>+        accessing m_pageLoadStart (a std::optional) in its unloaded state.
>+        Normally, m_pageLoadStart is initialized in
>+        WebPageProxy::didStartProvisionalLoadForFrame, which one would expect
>+        would be called before WebPageProxy::didFinishLoadForFrame. But that
>+        turns out to not always be the case. It's not apparent under what
>+        conditions didStartProvisionalLoadForFrame will not be called, but
>+        it's happening in the wild, leading to crashes now that std::optional
>+        asserts in release builds on bad accesses (see
>+        https://bugs.webkit.org/show_bug.cgi?id=189568).
>+
>+        Fix this by checking m_pageLoadState on entry to reportPageLoadResult.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::didFailProvisionalLoadForFrame):
>+        (WebKit::WebPageProxy::didFinishLoadForFrame):
>+        (WebKit::WebPageProxy::didFailLoadForFrame):
>+        (WebKit::WebPageProxy::reportPageLoadResult):
>+
> 2018-09-13  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         [GTK][WPE] Allow to run script dialogs asynchronously in the UI process
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 79dcc5980039dca239869d77f11287b7ae1a1ad5..d30d30cba04f595107bc97412201c70167c18d76 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -884,8 +884,7 @@ void WebPageProxy::close()
> 
>     m_isClosed = true;
> 
>-    if (m_pageLoadStart)
>-        reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
>+    reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
> 
>     if (m_activePopupMenu)
>         m_activePopupMenu->cancelTracking();
>@@ -3459,8 +3458,7 @@ void WebPageProxy::didStartProvisionalLoadForFrame(uint64_t frameID, uint64_t na
>     m_pageLoadState.clearPendingAPIRequestURL(transaction);
> 
>     if (frame->isMainFrame()) {
>-        if (m_pageLoadStart)
>-            reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
>+        reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
>         m_pageLoadStart = MonotonicTime::now();
>         m_pageLoadState.didStartProvisionalLoad(transaction, url, unreachableURL);
>         pageClient().didStartProvisionalLoadForMainFrame();
>@@ -7903,7 +7901,8 @@ void WebPageProxy::reportPageLoadResult(const ResourceError& error)
>         { CompletionCondition::Timeout, Seconds::infinity(), DiagnosticLoggingKeys::timedOutKey() }
>         });
> 
>-    ASSERT(m_pageLoadStart);
>+    if (!m_pageLoadStart)
>+        return;
> 
>     auto pageLoadTime = MonotonicTime::now() - *m_pageLoadStart;
>     m_pageLoadStart = std::nullopt;

Actions: View | Formatted Diff | Diff

Attachments on bug 189568: 349599 | 349678