Title: [236193] releases/WebKitGTK/webkit-2.22/Source/WebKit
Revision
236193
Author
[email protected]
Date
2018-09-19 06:19:49 -0700 (Wed, 19 Sep 2018)

Log Message

Merge r235992 - WebPageProxy::reportPageLoadResult can crash on some code paths
https://bugs.webkit.org/show_bug.cgi?id=189568

Reviewed by Chris Dumez.

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):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.22/Source/WebKit/ChangeLog (236192 => 236193)


--- releases/WebKitGTK/webkit-2.22/Source/WebKit/ChangeLog	2018-09-19 13:19:46 UTC (rev 236192)
+++ releases/WebKitGTK/webkit-2.22/Source/WebKit/ChangeLog	2018-09-19 13:19:49 UTC (rev 236193)
@@ -1,3 +1,30 @@
+2018-09-13  Keith Rollin  <[email protected]>
+
+        WebPageProxy::reportPageLoadResult can crash on some code paths
+        https://bugs.webkit.org/show_bug.cgi?id=189568
+
+        Reviewed by Chris Dumez.
+
+        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-08-31  John Wilander  <[email protected]>
 
         Storage Access API: Maintain access through same-site navigations

Modified: releases/WebKitGTK/webkit-2.22/Source/WebKit/UIProcess/WebPageProxy.cpp (236192 => 236193)


--- releases/WebKitGTK/webkit-2.22/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-09-19 13:19:46 UTC (rev 236192)
+++ releases/WebKitGTK/webkit-2.22/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-09-19 13:19:49 UTC (rev 236193)
@@ -871,8 +871,7 @@
 
     m_isClosed = true;
 
-    if (m_pageLoadStart)
-        reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
+    reportPageLoadResult(ResourceError { ResourceError::Type::Cancellation });
 
     if (m_activePopupMenu)
         m_activePopupMenu->cancelTracking();
@@ -3439,8 +3438,7 @@
     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);
         m_pageClient.didStartProvisionalLoadForMainFrame();
@@ -7852,7 +7850,8 @@
         { CompletionCondition::Timeout, Seconds::infinity(), DiagnosticLoggingKeys::timedOutKey() }
         });
 
-    ASSERT(m_pageLoadStart);
+    if (!m_pageLoadStart)
+        return;
 
     auto pageLoadTime = MonotonicTime::now() - *m_pageLoadStart;
     m_pageLoadStart = std::nullopt;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to