Attachment 340443 Details for Bug 185665 – Patch

[patch] Patch

bug-185665-20180516093344.patch (text/plain), 8.76 KB, created by Dean Jackson on 2018-05-15 16:33:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Dean Jackson

Created: 2018-05-15 16:33:46 PDT

Size: 8.76 KB

patch

obsolete

>Subversion Revision: 231578
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index ff49fd05d4fc7c35a23cfe43dc15d59c10982c4a..2e16bc1220b701a89e23d74128423cca65aba911 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+2018-05-15  Dean Jackson  <dino@apple.com>
>+
>+        Post-review cleanup for 185459
>+        https://bugs.webkit.org/show_bug.cgi?id=185665
>+        <rdar://problem/40276689>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Jon made some comments in 185459 that I'm addressing here.
>+
>+        * UIProcess/Cocoa/DownloadClient.h:
>+        * UIProcess/Cocoa/DownloadClient.mm: Guard the activity token for iOS
>+        in a way that means it will still work ok on macOS.
>+        (WebKit::DownloadClient::didStart):
>+        (WebKit::DownloadClient::processDidCrash):
>+        (WebKit::DownloadClient::didFinish):
>+        (WebKit::DownloadClient::didFail):
>+        (WebKit::DownloadClient::didCancel):
>+        (WebKit::DownloadClient::takeActivityToken):
>+        (WebKit::DownloadClient::releaseActivityTokenIfNecessary):
>+        (WebKit::DownloadClient::releaseActivityToken): Deleted.
>+
>+        * UIProcess/Cocoa/SystemPreviewControllerCocoa.mm: Add an early return.
>+        (-[_WKPreviewControllerDataSource previewController:previewItemAtIndex:]):
>+
> 2018-05-15  Dean Jackson  <dino@apple.com>
> 
>         Provide UIView and UIImage for zoom transition
>diff --git a/Source/WebKit/UIProcess/Cocoa/DownloadClient.h b/Source/WebKit/UIProcess/Cocoa/DownloadClient.h
>index db3efb79928cd94b0957878f9ac7b18eba9fd7a2..57179ac4bda6e631ebc30e11afbd06fe46051416 100644
>--- a/Source/WebKit/UIProcess/Cocoa/DownloadClient.h
>+++ b/Source/WebKit/UIProcess/Cocoa/DownloadClient.h
>@@ -60,12 +60,17 @@ private:
>     void didCreateDestination(WebProcessPool&, DownloadProxy&, const String&) final;
>     void processDidCrash(WebProcessPool&, DownloadProxy&) final;
> 
>-#if PLATFORM(IOS) && USE(SYSTEM_PREVIEW)
>-    void releaseActivityToken(DownloadProxy&);
>+#if USE(SYSTEM_PREVIEW)
>+    void takeActivityToken(DownloadProxy&);
>+    void releaseActivityTokenIfNecessary(DownloadProxy&);
> #endif
> 
>     WeakObjCPtr<id <_WKDownloadDelegate>> m_delegate;
> 
>+#if PLATFORM(IOS) && USE(SYSTEM_PREVIEW)
>+    ProcessThrottler::BackgroundActivityToken m_activityToken { nullptr };
>+#endif
>+
>     struct {
>         bool downloadDidStart : 1;            
>         bool downloadDidReceiveResponse : 1;
>@@ -81,10 +86,6 @@ private:
>         bool downloadDidCreateDestination : 1;
>         bool downloadProcessDidCrash : 1;
>     } m_delegateMethods;
>-
>-#if PLATFORM(IOS) && USE(SYSTEM_PREVIEW)
>-    ProcessThrottler::BackgroundActivityToken m_activityToken;
>-#endif
> };
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/UIProcess/Cocoa/DownloadClient.mm b/Source/WebKit/UIProcess/Cocoa/DownloadClient.mm
>index 13ae6da7bb30b282a341ffc3cbcc51ace9e52a03..5dc55877f1183ede17e96325f123ce0893f16669 100644
>--- a/Source/WebKit/UIProcess/Cocoa/DownloadClient.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/DownloadClient.mm
>@@ -76,11 +76,7 @@ void DownloadClient::didStart(WebProcessPool&, DownloadProxy& downloadProxy)
> {
> #if USE(SYSTEM_PREVIEW)
>     if (downloadProxy.isSystemPreviewDownload()) {
>-        if (auto* webPage = downloadProxy.originatingPage()) {
>-            RELEASE_LOG_IF(webPage->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - UIProcess is taking a background assertion because it is downloading a system preview", this);
>-            ASSERT(!m_activityToken);
>-            m_activityToken = webPage->process().throttler().backgroundActivityToken();
>-        }
>+        takeActivityToken(downloadProxy);
>         return;
>     }
> #endif
>@@ -179,8 +175,7 @@ void DownloadClient::processDidCrash(WebProcessPool&, DownloadProxy& downloadPro
> {
> #if USE(SYSTEM_PREVIEW)
>     if (downloadProxy.isSystemPreviewDownload()) {
>-        if (m_activityToken)
>-            releaseActivityToken(downloadProxy);
>+        releaseActivityTokenIfNecessary(downloadProxy);
>         return;
>     }
> #endif
>@@ -228,8 +223,7 @@ void DownloadClient::didFinish(WebProcessPool&, DownloadProxy& downloadProxy)
>             NSURL *destinationURL = [NSURL fileURLWithPath:(NSString *)downloadProxy.destinationFilename()];
>             webPage->systemPreviewController()->finish(WebCore::URL(destinationURL));
>         }
>-        if (m_activityToken)
>-            releaseActivityToken(downloadProxy);
>+        releaseActivityTokenIfNecessary(downloadProxy);
>         return;
>     }
> #endif
>@@ -244,8 +238,7 @@ void DownloadClient::didFail(WebProcessPool&, DownloadProxy& downloadProxy, cons
>     if (downloadProxy.isSystemPreviewDownload()) {
>         if (auto* webPage = downloadProxy.originatingPage())
>             webPage->systemPreviewController()->cancel();
>-        if (m_activityToken)
>-            releaseActivityToken(downloadProxy);
>+        releaseActivityTokenIfNecessary(downloadProxy);
>         return;
>     }
> #endif
>@@ -260,8 +253,7 @@ void DownloadClient::didCancel(WebProcessPool&, DownloadProxy& downloadProxy)
>     if (downloadProxy.isSystemPreviewDownload()) {
>         if (auto* webPage = downloadProxy.originatingPage())
>             webPage->systemPreviewController()->cancel();
>-        if (m_activityToken)
>-            releaseActivityToken(downloadProxy);
>+        releaseActivityTokenIfNecessary(downloadProxy);
>         return;
>     }
> #endif
>@@ -278,12 +270,30 @@ void DownloadClient::willSendRequest(WebProcessPool&, DownloadProxy& downloadPro
>     completionHandler(WTFMove(request));
> }
> 
>-#if PLATFORM(IOS) && USE(SYSTEM_PREVIEW)
>-void DownloadClient::releaseActivityToken(DownloadProxy& downloadProxy)
>+#if USE(SYSTEM_PREVIEW)
>+void DownloadClient::takeActivityToken(DownloadProxy& downloadProxy)
>+{
>+#if PLATFORM(IOS)
>+    if (auto* webPage = downloadProxy.originatingPage()) {
>+        RELEASE_LOG_IF(webPage->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p - UIProcess is taking a background assertion because it is downloading a system preview", this);
>+        ASSERT(!m_activityToken);
>+        m_activityToken = webPage->process().throttler().backgroundActivityToken();
>+    }
>+#else
>+    UNUSED_PARAM(downloadProxy);
>+#endif
>+}
>+
>+void DownloadClient::releaseActivityTokenIfNecessary(DownloadProxy& downloadProxy)
> {
>-    RELEASE_LOG_IF(downloadProxy.originatingPage()->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p UIProcess is releasing a background assertion because a system preview download completed", this);
>-    ASSERT(m_activityToken);
>-    m_activityToken = nullptr;
>+#if PLATFORM(IOS)
>+    if (m_activityToken) {
>+        RELEASE_LOG_IF(downloadProxy.originatingPage()->isAlwaysOnLoggingAllowed(), ProcessSuspension, "%p UIProcess is releasing a background assertion because a system preview download completed", this);
>+        m_activityToken = nullptr;
>+    }
>+#else
>+    UNUSED_PARAM(downloadProxy);
>+#endif
> }
> #endif
> 
>diff --git a/Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm b/Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm
>index 14b92d8b1a8edd738ef89928c4303c6ccaf72f71..72231d258457c72ec66975901c62c74ba1610320 100644
>--- a/Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/SystemPreviewControllerCocoa.mm
>@@ -74,20 +74,21 @@ SOFT_LINK_CLASS(QuickLook, QLItem);
> 
> - (id<QLPreviewItem>)previewController:(QLPreviewController *)controller previewItemAtIndex:(NSInteger)index
> {
>-    if (!_item) {
>-        _itemProvider = adoptNS([[NSItemProvider alloc] init]);
>-        NSString *contentType = @"public.content";
>+    if (_item)
>+        return _item.get();
>+
>+    _itemProvider = adoptNS([[NSItemProvider alloc] init]);
>+    NSString *contentType = @"public.content";
> #if USE(APPLE_INTERNAL_SDK)
>-        contentType = WebKit::getUTIForMIMEType(self.mimeType);
>+    contentType = WebKit::getUTIForMIMEType(self.mimeType);
> #endif
>-        _item = adoptNS([allocQLItemInstance() initWithPreviewItemProvider:_itemProvider.get() contentType:contentType previewTitle:@"Preview" fileSize:@(0)]);
>-        [_item setUseLoadingTimeout:NO];
>+    _item = adoptNS([allocQLItemInstance() initWithPreviewItemProvider:_itemProvider.get() contentType:contentType previewTitle:@"Preview" fileSize:@(0)]);
>+    [_item setUseLoadingTimeout:NO];
> 
>-        [_itemProvider registerItemForTypeIdentifier:contentType loadHandler:^(NSItemProviderCompletionHandler completionHandler, Class expectedValueClass, NSDictionary * options) {
>-            // This will get called once the download completes.
>-            self.completionHandler = completionHandler;
>-        }];
>-    }
>+    [_itemProvider registerItemForTypeIdentifier:contentType loadHandler:^(NSItemProviderCompletionHandler completionHandler, Class expectedValueClass, NSDictionary * options) {
>+        // This will get called once the download completes.
>+        self.completionHandler = completionHandler;
>+    }];
>     return _item.get();
> }
>

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185665: 340443