Title: [266099] trunk/Source/WebKit
Revision
266099
Author
beid...@apple.com
Date
2020-08-24 19:44:39 -0700 (Mon, 24 Aug 2020)

Log Message

CrashTracer: com.apple.WebKit.WebContent at com.apple.WebKit: WebKit::PDFPlugin::createScrollbar
<rdar://problem/67473335> and https://bugs.webkit.org/show_bug.cgi?id=215787

Reviewed by Tim Horton.

To quote Tim from r264945:
No new tests; timing is such that I can't reproduce without inserting
intentional delays into the main thread hops, which is further than
I'm willing to go for a test.

This is a speculative fix due to the aforementioned reproducibility issue.

* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::receivedNonLinearizedPDFSentinel): Check on the main thread whenever this is called,
  instead of when deciding to dispatch to the main thread.
(WebKit::PDFPlugin::threadEntry): We can't do this check on the background thread when considering the dispatch
  to the main thread, as the flag might've changed by then. Let's *just* check it on the main thread.
(WebKit::PDFPlugin::adoptBackgroundThreadDocument): We can't do the check on the background thread when
(WebKit::PDFPlugin::updateScrollbars): This is where the crash itself is. All of the Obj-C code in here is
  safe to do after destroy(), up until the very end when we get into pluginView() derefencing.
  So it seems prudent to add another check here.
(WebKit::PDFPlugin::documentDataDidFinishLoading): In addition to receivedNonLinearizedPDFSentinel and
  adoptBackgroundThreadDocument, this is the final of the (3) calls that end up calling installPDFDocument,
  so for added coverage it seems like a prudent place to add the check.

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (266098 => 266099)


--- trunk/Source/WebKit/ChangeLog	2020-08-25 02:03:51 UTC (rev 266098)
+++ trunk/Source/WebKit/ChangeLog	2020-08-25 02:44:39 UTC (rev 266099)
@@ -1,3 +1,30 @@
+2020-08-24  Brady Eidson  <beid...@apple.com>
+
+        CrashTracer: com.apple.WebKit.WebContent at com.apple.WebKit: WebKit::PDFPlugin::createScrollbar
+        <rdar://problem/67473335> and https://bugs.webkit.org/show_bug.cgi?id=215787
+
+        Reviewed by Tim Horton.
+
+        To quote Tim from r264945:
+        No new tests; timing is such that I can't reproduce without inserting
+        intentional delays into the main thread hops, which is further than
+        I'm willing to go for a test.
+
+        This is a speculative fix due to the aforementioned reproducibility issue.
+
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::receivedNonLinearizedPDFSentinel): Check on the main thread whenever this is called,
+          instead of when deciding to dispatch to the main thread.
+        (WebKit::PDFPlugin::threadEntry): We can't do this check on the background thread when considering the dispatch
+          to the main thread, as the flag might've changed by then. Let's *just* check it on the main thread.
+        (WebKit::PDFPlugin::adoptBackgroundThreadDocument): We can't do the check on the background thread when 
+        (WebKit::PDFPlugin::updateScrollbars): This is where the crash itself is. All of the Obj-C code in here is
+          safe to do after destroy(), up until the very end when we get into pluginView() derefencing.
+          So it seems prudent to add another check here.
+        (WebKit::PDFPlugin::documentDataDidFinishLoading): In addition to receivedNonLinearizedPDFSentinel and
+          adoptBackgroundThreadDocument, this is the final of the (3) calls that end up calling installPDFDocument,
+          so for added coverage it seems like a prudent place to add the check.
+
 2020-08-24  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Unreviewed, fix the internal iOS 13.4 build

Modified: trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm (266098 => 266099)


--- trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm	2020-08-25 02:03:51 UTC (rev 266098)
+++ trunk/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm	2020-08-25 02:44:39 UTC (rev 266099)
@@ -705,13 +705,14 @@
 {
     m_incrementalPDFLoadingEnabled = false;
 
+    if (m_hasBeenDestroyed)
+        return;
+
     if (!isMainThread()) {
 #if !LOG_DISABLED
         pdfLog("Disabling incremental PDF loading on background thread");
 #endif
         callOnMainThread([this, protectedThis = makeRef(*this)] {
-            if (m_hasBeenDestroyed)
-                return;
             receivedNonLinearizedPDFSentinel();
         });
         return;
@@ -890,8 +891,6 @@
     [m_backgroundThreadDocument preloadDataOfPagesInRange:NSMakeRange(0, 1) onQueue:firstPageQueue->dispatchQueue() completion:[&firstPageSemaphore, this] (NSIndexSet *) mutable {
         if (m_incrementalPDFLoadingEnabled) {
             callOnMainThread([this] {
-                if (m_hasBeenDestroyed)
-                    return;
                 adoptBackgroundThreadDocument();
             });
         } else
@@ -986,6 +985,9 @@
 
 void PDFPlugin::adoptBackgroundThreadDocument()
 {
+    if (m_hasBeenDestroyed)
+        return;
+
     ASSERT(!m_pdfDocument);
     ASSERT(m_backgroundThreadDocument);
     ASSERT(isMainThread());
@@ -1219,6 +1221,9 @@
 
 void PDFPlugin::updateScrollbars()
 {
+    if (m_hasBeenDestroyed)
+        return;
+
     bool hadScrollbars = m_horizontalScrollbar || m_verticalScrollbar;
 
     if (m_horizontalScrollbar) {
@@ -1527,6 +1532,9 @@
 
 void PDFPlugin::documentDataDidFinishLoading()
 {
+    if (m_hasBeenDestroyed)
+        return;
+
     addArchiveResource();
 
     m_documentFinishedLoading = true;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to