Title: [218917] trunk
Revision
218917
Author
[email protected]
Date
2017-06-28 21:37:53 -0700 (Wed, 28 Jun 2017)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (218916 => 218917)


--- trunk/Source/WebKit2/ChangeLog	2017-06-29 04:04:37 UTC (rev 218916)
+++ trunk/Source/WebKit2/ChangeLog	2017-06-29 04:37:53 UTC (rev 218917)
@@ -1,3 +1,24 @@
+2017-06-28  Tim Horton  <[email protected]>
+
+        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  <[email protected]>
 
         [WK2][macOS][iOS] Don't request microphone access for clients that don't need it.

Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (218916 => 218917)


--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm	2017-06-29 04:04:37 UTC (rev 218916)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm	2017-06-29 04:37:53 UTC (rev 218917)
@@ -5350,6 +5350,11 @@
 
 - (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 @@
 // 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];
 }

Modified: trunk/Tools/ChangeLog (218916 => 218917)


--- trunk/Tools/ChangeLog	2017-06-29 04:04:37 UTC (rev 218916)
+++ trunk/Tools/ChangeLog	2017-06-29 04:37:53 UTC (rev 218917)
@@ -1,3 +1,16 @@
+2017-06-28  Tim Horton  <[email protected]>
+
+        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  <[email protected]>
 
         Prevent displaying URLs with small capital letters

Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (218916 => 218917)


--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2017-06-29 04:04:37 UTC (rev 218916)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj	2017-06-29 04:37:53 UTC (rev 218917)
@@ -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 */; };
@@ -1036,6 +1037,7 @@
 		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>"; };
@@ -1877,6 +1879,7 @@
 				37B47E2E1D64E7CA005F4EFF /* WKObject.mm */,
 				A14AAB611E78D7DE00C1ADC2 /* WKPDFView.mm */,
 				2D00065D1C1F58940088E6A7 /* WKPDFViewResizeCrash.mm */,
+				2D21FE581F04642800B58E7D /* WKPDFViewStablePresentationUpdateCallback.mm */,
 				5E4B1D2C1D404C6100053621 /* WKScrollViewDelegateCrash.mm */,
 				51C683DD1EA134DB00650183 /* WKURLSchemeHandler-1.mm */,
 				5CE354D81E70D9C300BEFE3B /* WKContentExtensionStore.mm */,
@@ -3041,6 +3044,7 @@
 				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 */,

Added: trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm (0 => 218917)


--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm	                        (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKPDFViewStablePresentationUpdateCallback.mm	2017-06-29 04:37:53 UTC (rev 218917)
@@ -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
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to