Title: [224896] trunk
Revision
224896
Author
achristen...@apple.com
Date
2017-11-15 13:28:15 -0800 (Wed, 15 Nov 2017)

Log Message

WebViews scheduled in custom run loop modes should be able to do more than 50ms of work at a time
https://bugs.webkit.org/show_bug.cgi?id=179742
<rdar://problem/35519421>

Reviewed by Jer Noble.
Source/WTF:

        
In r224687 I fixed loading from scheduled WebViews with custom run loop modes, but in 
dispatchFunctionsFromMainThread we have an optimization to yield the run loop if we have already
done more than 50ms of work on the main thread in this run loop iteration.  When this happens
and we are running in a custom run loop mode, we disable this responsiveness optimization.
We are calling CFRunLoopRunInMode or [NSRunLoop acceptInputForMode:beforeDate:] in a while loop anyways,
so we would not benefit from a responsiveness optimization.  We definitely don't want to reschedule
on the main thread in the common run loop mode in this case.

* wtf/MainThread.cpp:
(WTF::dispatchFunctionsFromMainThread):
* wtf/MainThread.h:
* wtf/generic/MainThreadGeneric.cpp:
(WTF::currentRunLoopInCommonMode):
* wtf/mac/MainThreadMac.mm:
(WTF::currentRunLoopInCommonMode):
* wtf/win/MainThreadWin.cpp:
(WTF::currentRunLoopInCommonMode):

Tools:


* TestWebKitAPI/Tests/mac/WebViewScheduleInRunLoop.mm:
(-[ScheduleInRunLoopDelegate webView:didFinishLoadForFrame:]):
(TestWebKitAPI::TEST):
Load more than one scheduled WebView to test work that typically takes more than 50 ms.

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (224895 => 224896)


--- trunk/Source/WTF/ChangeLog	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Source/WTF/ChangeLog	2017-11-15 21:28:15 UTC (rev 224896)
@@ -1,3 +1,29 @@
+2017-11-15  Alex Christensen  <achristen...@webkit.org>
+
+        WebViews scheduled in custom run loop modes should be able to do more than 50ms of work at a time
+        https://bugs.webkit.org/show_bug.cgi?id=179742
+        <rdar://problem/35519421>
+
+        Reviewed by Jer Noble.
+        
+        In r224687 I fixed loading from scheduled WebViews with custom run loop modes, but in 
+        dispatchFunctionsFromMainThread we have an optimization to yield the run loop if we have already
+        done more than 50ms of work on the main thread in this run loop iteration.  When this happens
+        and we are running in a custom run loop mode, we disable this responsiveness optimization.
+        We are calling CFRunLoopRunInMode or [NSRunLoop acceptInputForMode:beforeDate:] in a while loop anyways,
+        so we would not benefit from a responsiveness optimization.  We definitely don't want to reschedule
+        on the main thread in the common run loop mode in this case.
+
+        * wtf/MainThread.cpp:
+        (WTF::dispatchFunctionsFromMainThread):
+        * wtf/MainThread.h:
+        * wtf/generic/MainThreadGeneric.cpp:
+        (WTF::currentRunLoopInCommonMode):
+        * wtf/mac/MainThreadMac.mm:
+        (WTF::currentRunLoopInCommonMode):
+        * wtf/win/MainThreadWin.cpp:
+        (WTF::currentRunLoopInCommonMode):
+
 2017-11-15  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r224863.

Modified: trunk/Source/WTF/wtf/MainThread.cpp (224895 => 224896)


--- trunk/Source/WTF/wtf/MainThread.cpp	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Source/WTF/wtf/MainThread.cpp	2017-11-15 21:28:15 UTC (rev 224896)
@@ -135,7 +135,7 @@
         // yield so the user input can be processed. Otherwise user may not be able to even close the window.
         // This code has effect only in case the scheduleDispatchFunctionsOnMainThread() is implemented in a way that
         // allows input events to be processed before we are back here.
-        if (MonotonicTime::now() - startTime > maxRunLoopSuspensionTime) {
+        if (MonotonicTime::now() - startTime > maxRunLoopSuspensionTime && currentRunLoopInCommonMode()) {
             scheduleDispatchFunctionsOnMainThread();
             break;
         }

Modified: trunk/Source/WTF/wtf/MainThread.h (224895 => 224896)


--- trunk/Source/WTF/wtf/MainThread.h	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Source/WTF/wtf/MainThread.h	2017-11-15 21:28:15 UTC (rev 224896)
@@ -87,6 +87,7 @@
 void initializeMainThreadPlatform();
 void scheduleDispatchFunctionsOnMainThread(SchedulePairHashSet* = nullptr);
 void dispatchFunctionsFromMainThread();
+bool currentRunLoopInCommonMode();
 
 #if OS(DARWIN) && !USE(GLIB)
 #if !USE(WEB_THREAD)

Modified: trunk/Source/WTF/wtf/generic/MainThreadGeneric.cpp (224895 => 224896)


--- trunk/Source/WTF/wtf/generic/MainThreadGeneric.cpp	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Source/WTF/wtf/generic/MainThreadGeneric.cpp	2017-11-15 21:28:15 UTC (rev 224896)
@@ -67,6 +67,11 @@
 {
 }
 
+bool currentRunLoopInCommonMode()
+{
+    return true;
+}
+
 void scheduleDispatchFunctionsOnMainThread(SchedulePairHashSet*)
 {
     // Use a RunLoop::Timer instead of RunLoop::dispatch() to be able to use a different priority and

Modified: trunk/Source/WTF/wtf/mac/MainThreadMac.mm (224895 => 224896)


--- trunk/Source/WTF/wtf/mac/MainThreadMac.mm	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Source/WTF/wtf/mac/MainThreadMac.mm	2017-11-15 21:28:15 UTC (rev 224896)
@@ -122,6 +122,11 @@
     CFRunLoopAddTimer(CFRunLoopGetCurrent(), CFRunLoopTimerCreate(0, 0, 0, 0, 0, timerFired, 0), kCFRunLoopCommonModes);
 }
 
+bool currentRunLoopInCommonMode()
+{
+    return [[NSRunLoop currentRunLoop] currentMode] == (NSString *)kCFRunLoopCommonModes;
+}
+
 void scheduleDispatchFunctionsOnMainThread(SchedulePairHashSet* pairs)
 {
     ASSERT(staticMainThreadCaller);

Modified: trunk/Source/WTF/wtf/win/MainThreadWin.cpp (224895 => 224896)


--- trunk/Source/WTF/wtf/win/MainThreadWin.cpp	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Source/WTF/wtf/win/MainThreadWin.cpp	2017-11-15 21:28:15 UTC (rev 224896)
@@ -49,6 +49,11 @@
     return 0;
 }
 
+bool currentRunLoopInCommonMode()
+{
+    return true;
+}
+
 void initializeMainThreadPlatform()
 {
     if (threadingWindowHandle)

Modified: trunk/Tools/ChangeLog (224895 => 224896)


--- trunk/Tools/ChangeLog	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Tools/ChangeLog	2017-11-15 21:28:15 UTC (rev 224896)
@@ -1,3 +1,16 @@
+2017-11-15  Alex Christensen  <achristen...@webkit.org>
+
+        WebViews scheduled in custom run loop modes should be able to do more than 50ms of work at a time
+        https://bugs.webkit.org/show_bug.cgi?id=179742
+        <rdar://problem/35519421>
+
+        Reviewed by Jer Noble.
+
+        * TestWebKitAPI/Tests/mac/WebViewScheduleInRunLoop.mm:
+        (-[ScheduleInRunLoopDelegate webView:didFinishLoadForFrame:]):
+        (TestWebKitAPI::TEST):
+        Load more than one scheduled WebView to test work that typically takes more than 50 ms.
+
 2017-11-15  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r223781.

Modified: trunk/Tools/TestWebKitAPI/Tests/mac/WebViewScheduleInRunLoop.mm (224895 => 224896)


--- trunk/Tools/TestWebKitAPI/Tests/mac/WebViewScheduleInRunLoop.mm	2017-11-15 21:18:08 UTC (rev 224895)
+++ trunk/Tools/TestWebKitAPI/Tests/mac/WebViewScheduleInRunLoop.mm	2017-11-15 21:28:15 UTC (rev 224896)
@@ -29,18 +29,19 @@
 #import <WebKit/WebFrameLoadDelegate.h>
 #import <WebKit/WebViewPrivate.h>
 #import <wtf/RetainPtr.h>
+#import <wtf/Vector.h>
 
 @interface ScheduleInRunLoopDelegate : NSObject <WebFrameLoadDelegate> {
 }
 @end
 
-static bool didFinishLoad;
+static size_t loadsFinished;
 
 @implementation ScheduleInRunLoopDelegate
 
 - (void)webView:(WebView *)sender didFinishLoadForFrame:(WebFrame *)frame
 {
-    didFinishLoad = true;
+    loadsFinished++;
 }
 
 @end
@@ -49,14 +50,19 @@
 
 TEST(WebKitLegacy, ScheduleInRunLoop)
 {
-    auto webView = adoptNS([[WebView alloc] init]);
+    const size_t webViewCount = 50;
+    Vector<RetainPtr<WebView>> webViews;
     auto delegate = adoptNS([[ScheduleInRunLoopDelegate alloc] init]);
-    [webView setFrameLoadDelegate:delegate.get()];
-    [webView unscheduleFromRunLoop:[NSRunLoop currentRunLoop] forMode:(NSString *)kCFRunLoopCommonModes];
-    [webView scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:@"TestRunLoopMode"];
-    [[webView mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+    for (size_t i = 0; i < webViewCount; ++i) {
+        auto webView = adoptNS([[WebView alloc] init]);
+        [webView setFrameLoadDelegate:delegate.get()];
+        [webView unscheduleFromRunLoop:[NSRunLoop currentRunLoop] forMode:(NSString *)kCFRunLoopCommonModes];
+        [webView scheduleInRunLoop:[NSRunLoop currentRunLoop] forMode:@"TestRunLoopMode"];
+        [[webView mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+        webViews.append(WTFMove(webView));
+    }
 
-    while (!didFinishLoad)
+    while (loadsFinished < webViewCount)
         CFRunLoopRunInMode(CFSTR("TestRunLoopMode"), std::numeric_limits<double>::max(), true);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to