Changeset 195982 in webkit

Timestamp:

Feb 1, 2016 3:31:30 PM (8 years ago)

Author:

timothy_horton@apple.com

Message:

Crash using WKPDFView, in -[UIPDFPageView showContent]
​https://bugs.webkit.org/show_bug.cgi?id=153770
<rdar://problem/20157864>

Reviewed by Anders Carlsson.

When scrolling, we throw away UIPDFPageViews, but forget to unhook their delegates.
In some cases (also because their delegate is a raw pointer, not weak), they can call
back into a deleted WKPDFView, causing a crash.

• UIProcess/ios/WKPDFView.mm:

(detachViewForPage):
Add a helper to detach a page view, and unhook its delegates.

(-[WKPDFView _clearPages]):
(-[WKPDFView _revalidateViews]):
Make use of the new helper.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ios/WKPDFView.mm (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-02-01  Tim Horton  <timothy_horton@apple.com>
+
+        Crash using WKPDFView, in -[UIPDFPageView showContent]
+        https://bugs.webkit.org/show_bug.cgi?id=153770
+        <rdar://problem/20157864>
+
+        Reviewed by Anders Carlsson.
+
+        When scrolling, we throw away UIPDFPageViews, but forget to unhook their delegates.
+        In some cases (also because their delegate is a raw pointer, not weak), they can call
+        back into a deleted WKPDFView, causing a crash.
+
+        * UIProcess/ios/WKPDFView.mm:
+        (detachViewForPage):
+        Add a helper to detach a page view, and unhook its delegates.
+
+        (-[WKPDFView _clearPages]):
+        (-[WKPDFView _revalidateViews]):
+        Make use of the new helper.
+
 2016-02-01  Dan Bernstein  <mitz@apple.com>
 

trunk/Source/WebKit2/UIProcess/ios/WKPDFView.mm

@@ -157 +157 @@
 }
 
+static void detachViewForPage(PDFPageInfo& page)
+{
+    [page.view removeFromSuperview];
+    [page.view setDelegate:nil];
+    [[page.view annotationController] setDelegate:nil];
+    page.view = nil;
+}
+
 - (void)_clearPages
 {
-    for (auto& page : _pages) {
-        [page.view removeFromSuperview];
-        [page.view setDelegate:nil];
-        [[page.view annotationController] setDelegate:nil];
-    }
+    for (auto& page : _pages)
+        detachViewForPage(page);
     
     _pages.clear();
@@ -269 +274 @@
     for (auto& pageInfo : _pages) {
         if (!CGRectIntersectsRect(pageInfo.frame, targetRectWithOverdraw) && pageInfo.index != _currentFindPageIndex) {
-            [pageInfo.view removeFromSuperview];
-            pageInfo.view = nullptr;
+            detachViewForPage(pageInfo);
             continue;
         }