MobileSafari was constantly using 10-15% CPU viewing a PDF
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 04:37:53 +0000 (04:37 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 04:37:53 +0000 (04:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173944
<rdar://problem/33039910>

Reviewed by Simon Fraser.

Source/WebKit2:

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _doAfterNextStablePresentationUpdate:]):
(-[WKWebView _doAfterNextPresentationUpdate:]):
(-[WKWebView _doAfterNextPresentationUpdateWithoutWaitingForPainting:]):
Bail early and just dispatch_async the completion block if we are using a custom
content view; these methods are very specific to the implementation of WKContentView
and don't make sense with custom content views.

doAfterNextStablePresentationUpdate is particularly egregious because, since
we will never call the stable update callbacks (because we bail from didCommitLayerTree
if we aren't using WKContentView), it will keep calling doAfterNextPresentationUpdate
over and over again.

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm:
Add a test that we ever call the stable presentation update callback
when we have a WKPDFView up, instead of infinitely looping.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218917 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm [new file with mode: 0644]

index e88860e..8f0798b 100644 (file)
@@ -1,3 +1,24 @@
+2017-06-28  Tim Horton  <timothy_horton@apple.com>
+
+        MobileSafari was constantly using 10-15% CPU viewing a PDF
+        https://bugs.webkit.org/show_bug.cgi?id=173944
+        <rdar://problem/33039910>
+
+        Reviewed by Simon Fraser.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _doAfterNextStablePresentationUpdate:]):
+        (-[WKWebView _doAfterNextPresentationUpdate:]):
+        (-[WKWebView _doAfterNextPresentationUpdateWithoutWaitingForPainting:]):
+        Bail early and just dispatch_async the completion block if we are using a custom
+        content view; these methods are very specific to the implementation of WKContentView
+        and don't make sense with custom content views.
+
+        doAfterNextStablePresentationUpdate is particularly egregious because, since
+        we will never call the stable update callbacks (because we bail from didCommitLayerTree
+        if we aren't using WKContentView), it will keep calling doAfterNextPresentationUpdate
+        over and over again.
+
 2017-06-28  Brent Fulgham  <bfulgham@apple.com>
 
         [WK2][macOS][iOS] Don't request microphone access for clients that don't need it.
index ee9e12d..af504ea 100644 (file)
@@ -5350,6 +5350,11 @@ static WebCore::UserInterfaceLayoutDirection toUserInterfaceLayoutDirection(UISe
 
 - (void)_doAfterNextStablePresentationUpdate:(dispatch_block_t)updateBlock
 {
+    if (![self usesStandardContentView]) {
+        dispatch_async(dispatch_get_main_queue(), updateBlock);
+        return;
+    }
+
     auto updateBlockCopy = makeBlockPtr(updateBlock);
 
     if (_stableStatePresentationUpdateCallbacks)
@@ -5540,20 +5545,30 @@ static WebCore::UserInterfaceLayoutDirection toUserInterfaceLayoutDirection(UISe
 // Execute the supplied block after the next transaction from the WebProcess.
 - (void)_doAfterNextPresentationUpdate:(void (^)(void))updateBlock
 {
-    void (^updateBlockCopy)(void) = nil;
-    if (updateBlock)
-        updateBlockCopy = Block_copy(updateBlock);
+#if PLATFORM(IOS)
+    if (![self usesStandardContentView]) {
+        dispatch_async(dispatch_get_main_queue(), updateBlock);
+        return;
+    }
+#endif
+
+    auto updateBlockCopy = makeBlockPtr(updateBlock);
 
     _page->callAfterNextPresentationUpdate([updateBlockCopy](WebKit::CallbackBase::Error error) {
-        if (updateBlockCopy) {
+        if (updateBlockCopy)
             updateBlockCopy();
-            Block_release(updateBlockCopy);
-        }
     });
 }
 
 - (void)_doAfterNextPresentationUpdateWithoutWaitingForPainting:(void (^)(void))updateBlock
 {
+#if PLATFORM(IOS)
+    if (![self usesStandardContentView]) {
+        dispatch_async(dispatch_get_main_queue(), updateBlock);
+        return;
+    }
+#endif
+
     _page->setShouldSkipWaitingForPaintAfterNextViewDidMoveToWindow(true);
     [self _doAfterNextPresentationUpdate:updateBlock];
 }
index e831a2c..548cf76 100644 (file)
@@ -1,3 +1,16 @@
+2017-06-28  Tim Horton  <timothy_horton@apple.com>
+
+        MobileSafari was constantly using 10-15% CPU viewing a PDF
+        https://bugs.webkit.org/show_bug.cgi?id=173944
+        <rdar://problem/33039910>
+
+        Reviewed by Simon Fraser.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm:
+        Add a test that we ever call the stable presentation update callback
+        when we have a WKPDFView up, instead of infinitely looping.
+
 2017-06-28  Alex Christensen  <achristensen@webkit.org>
 
         Prevent displaying URLs with small capital letters
index 204be5c..982790b 100644 (file)
@@ -65,6 +65,7 @@
                2D00065F1C1F589A0088E6A7 /* WKPDFViewResizeCrash.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D00065D1C1F58940088E6A7 /* WKPDFViewResizeCrash.mm */; };
                2D1646E21D1862CD00015A1A /* DeferredViewInWindowStateChange.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D1646E11D1862CD00015A1A /* DeferredViewInWindowStateChange.mm */; };
                2D1C04A71D76298B000A6816 /* TestNavigationDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D1C04A61D76298B000A6816 /* TestNavigationDelegate.mm */; };
+               2D21FE591F04642900B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D21FE581F04642800B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm */; };
                2D4CF8BD1D8360CC0001CE8D /* WKThumbnailView.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D4CF8BC1D8360CC0001CE8D /* WKThumbnailView.mm */; };
                2D51A0C71C8BF00C00765C45 /* DOMHTMLVideoElementWrapper.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D51A0C51C8BF00400765C45 /* DOMHTMLVideoElementWrapper.mm */; };
                2D838B1F1EEF3A5C009B980E /* WKContentViewEditingActions.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D838B1E1EEF3A5B009B980E /* WKContentViewEditingActions.mm */; };
                2D1C04A51D76298B000A6816 /* TestNavigationDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = TestNavigationDelegate.h; path = cocoa/TestNavigationDelegate.h; sourceTree = "<group>"; };
                2D1C04A61D76298B000A6816 /* TestNavigationDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = TestNavigationDelegate.mm; path = cocoa/TestNavigationDelegate.mm; sourceTree = "<group>"; };
                2D1FE0AF1AD465C1006CD9E6 /* FixedLayoutSize.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FixedLayoutSize.mm; sourceTree = "<group>"; };
+               2D21FE581F04642800B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKPDFViewStablePresentationUpdateCallback.mm; sourceTree = "<group>"; };
                2D4CF8BC1D8360CC0001CE8D /* WKThumbnailView.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WKThumbnailView.mm; path = WebKit2/WKThumbnailView.mm; sourceTree = "<group>"; };
                2D51A0C51C8BF00400765C45 /* DOMHTMLVideoElementWrapper.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DOMHTMLVideoElementWrapper.mm; sourceTree = "<group>"; };
                2D640B5417875DFF00BFAF99 /* ScrollPinningBehaviors.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ScrollPinningBehaviors.cpp; sourceTree = "<group>"; };
                                37B47E2E1D64E7CA005F4EFF /* WKObject.mm */,
                                A14AAB611E78D7DE00C1ADC2 /* WKPDFView.mm */,
                                2D00065D1C1F58940088E6A7 /* WKPDFViewResizeCrash.mm */,
+                               2D21FE581F04642800B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm */,
                                5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */,
                                51C683DD1EA134DB00650183 /* WKURLSchemeHandler-1.mm */,
                                5CE354D81E70D9C300BEFE3B /* WKContentExtensionStore.mm */,
                                8E4A85371E1D1AB200F53B0F /* GridPosition.cpp in Sources */,
                                7CCE7EFA1A411AE600447C4C /* HitTestResultNodeHandle.cpp in Sources */,
                                7CCE7EC11A411A7E00447C4C /* HTMLCollectionNamedItem.mm in Sources */,
+                               2D21FE591F04642900B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm in Sources */,
                                7CCE7EC21A411A7E00447C4C /* HTMLFormCollectionNamedItem.mm in Sources */,
                                7C83E0501D0A641800FEBCF3 /* HTMLParserIdioms.cpp in Sources */,
                                7A95BDE11E9BEC5F00865498 /* InjectedBundleAppleEvent.cpp in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm
new file mode 100644 (file)
index 0000000..a9171f2
--- /dev/null
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#import "PlatformUtilities.h"
+#import "Test.h"
+#import "TestNavigationDelegate.h"
+#import <WebKit/WKWebView.h>
+#import <wtf/RetainPtr.h>
+
+#if WK_API_ENABLED && PLATFORM(IOS)
+
+TEST(WebKit2, WKPDFViewStablePresentationUpdateCallback)
+{
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"test" withExtension:@"pdf" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+    [webView _test_waitForDidFinishNavigation];
+
+    __block bool finished;
+
+    [webView _doAfterNextStablePresentationUpdate:^ {
+        finished = true;
+    }];
+
+    TestWebKitAPI::Util::run(&finished);
+}
+
+#endif