A window with a hung tab waits 5s before becoming active
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Nov 2015 23:10:57 +0000 (23:10 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Nov 2015 23:10:57 +0000 (23:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151426

Reviewed by Sam Weinig.

This patch adds an optimization to skip the synchronous web process
message to check for a legacy scrollbar when we know that legacy
scrollbars are not enabled.

(Note that legacy scrollbars don't work quite right, due to
<rdar://problem/23585420> and <rdar://problem/23605296>. Still, I verified
with logging that we do the synchronous message when legacy scrollbars
are enabled.)

A consequence of this change is that we will no longer support
click-to-scroll-while-inactive behavior for scrollbars with custom looks
on systems with modern scrollbars. I spoke with Beth and Dan, and they
agreed that this is a reasonable change to make, since we don't support
click-to-scroll-while-inactive behavior for fully custom scrollbars either,
and since systems with modern scrollbars typically use swipe to scroll.

* UIProcess/Cocoa/WebViewImpl.h: Added some helper functions to explain
the behaviors we're checking for.

* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::WebViewImpl::mightBeginDragWhileInactive): Factored out from
shouldDelayWindowOrderingForEvent.

(WebKit::WebViewImpl::mightBeginScrollWhileInactive): New function.

(WebKit::WebViewImpl::acceptsFirstMouse): Moved this function next to
shouldDelayWindowOrderingForEvent because their responsibilities are
very similar. Added a fast path check for when we know that we will
not accept first mouse because we can't start a drag or scroll by
clicking while inactive.

(WebKit::WebViewImpl::shouldDelayWindowOrderingForEvent): Refactored
to use the helper function. Behavior unchanged.

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/Cocoa/WebViewImpl.h
Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm

index 892640b241d6f89f8d0cee5f52a7749c34910a9f..a9740ac8aadcf83506c61178c162795996222157 100644 (file)
@@ -1,3 +1,44 @@
+2015-11-20  Geoffrey Garen  <ggaren@apple.com>
+
+        A window with a hung tab waits 5s before becoming active
+        https://bugs.webkit.org/show_bug.cgi?id=151426
+
+        Reviewed by Sam Weinig.
+
+        This patch adds an optimization to skip the synchronous web process
+        message to check for a legacy scrollbar when we know that legacy
+        scrollbars are not enabled.
+
+        (Note that legacy scrollbars don't work quite right, due to
+        <rdar://problem/23585420> and <rdar://problem/23605296>. Still, I verified
+        with logging that we do the synchronous message when legacy scrollbars
+        are enabled.)
+
+        A consequence of this change is that we will no longer support
+        click-to-scroll-while-inactive behavior for scrollbars with custom looks
+        on systems with modern scrollbars. I spoke with Beth and Dan, and they
+        agreed that this is a reasonable change to make, since we don't support
+        click-to-scroll-while-inactive behavior for fully custom scrollbars either,
+        and since systems with modern scrollbars typically use swipe to scroll.
+
+        * UIProcess/Cocoa/WebViewImpl.h: Added some helper functions to explain
+        the behaviors we're checking for.
+
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (WebKit::WebViewImpl::mightBeginDragWhileInactive): Factored out from
+        shouldDelayWindowOrderingForEvent.
+
+        (WebKit::WebViewImpl::mightBeginScrollWhileInactive): New function.
+
+        (WebKit::WebViewImpl::acceptsFirstMouse): Moved this function next to
+        shouldDelayWindowOrderingForEvent because their responsibilities are
+        very similar. Added a fast path check for when we know that we will
+        not accept first mouse because we can't start a drag or scroll by
+        clicking while inactive.
+
+        (WebKit::WebViewImpl::shouldDelayWindowOrderingForEvent): Refactored
+        to use the helper function. Behavior unchanged.
+
 2015-11-20  Geoffrey Garen  <ggaren@apple.com>
 
         A hung webpage pretends to be responsive if you scroll
index 0c18d7bb0c36a0d077f61e6e5d9991875d691640..c31cc56e4c85a290c35a9712153f8448720d542b 100644 (file)
@@ -504,6 +504,9 @@ private:
     void mouseUpInternal(NSEvent *);
     void mouseDraggedInternal(NSEvent *);
 
+    bool mightBeginDragWhileInactive();
+    bool mightBeginScrollWhileInactive();
+
     NSView <WebViewImplDelegate> *m_view;
     std::unique_ptr<PageClient> m_pageClient;
     Ref<WebPageProxy> m_page;
index d1bf5dea1b6794ddf7d95a0e918e6c3fc1631a34..e035a9d0cbc2203a30aef71c7fb80d521756db36 100644 (file)
@@ -555,22 +555,6 @@ bool WebViewImpl::isOpaque() const
     return m_page->drawsBackground();
 }
 
-bool WebViewImpl::acceptsFirstMouse(NSEvent *event)
-{
-    // There's a chance that responding to this event will run a nested event loop, and
-    // fetching a new event might release the old one. Retaining and then autoreleasing
-    // the current event prevents that from causing a problem inside WebKit or AppKit code.
-    [[event retain] autorelease];
-
-    if (![m_view hitTest:event.locationInWindow])
-        return false;
-
-    setLastMouseDownEvent(event);
-    bool result = m_page->acceptsFirstMouse(event.eventNumber, WebEventFactory::createWebMouseEvent(event, m_lastPressureEvent.get(), m_view));
-    setLastMouseDownEvent(nil);
-    return result;
-}
-
 bool WebViewImpl::acceptsFirstResponder()
 {
     return true;
@@ -1134,11 +1118,48 @@ void WebViewImpl::windowDidChangeOcclusionState()
     m_page->viewStateDidChange(WebCore::ViewState::IsVisible);
 }
 
+bool WebViewImpl::mightBeginDragWhileInactive()
+{
+    if (m_view.window.isKeyWindow)
+        return false;
+
+    if (m_page->editorState().selectionIsNone || !m_page->editorState().selectionIsRange)
+        return false;
+
+    return true;
+}
+
+bool WebViewImpl::mightBeginScrollWhileInactive()
+{
+    // Legacy style scrollbars have design details that rely on tracking the mouse all the time.
+    if (WKRecommendedScrollerStyle() == NSScrollerStyleLegacy)
+        return true;
+
+    return false;
+}
+
+bool WebViewImpl::acceptsFirstMouse(NSEvent *event)
+{
+    if (!mightBeginDragWhileInactive() && !mightBeginScrollWhileInactive())
+        return false;
+
+    // There's a chance that responding to this event will run a nested event loop, and
+    // fetching a new event might release the old one. Retaining and then autoreleasing
+    // the current event prevents that from causing a problem inside WebKit or AppKit code.
+    [[event retain] autorelease];
+
+    if (![m_view hitTest:event.locationInWindow])
+        return false;
+
+    setLastMouseDownEvent(event);
+    bool result = m_page->acceptsFirstMouse(event.eventNumber, WebEventFactory::createWebMouseEvent(event, m_lastPressureEvent.get(), m_view));
+    setLastMouseDownEvent(nil);
+    return result;
+}
+
 bool WebViewImpl::shouldDelayWindowOrderingForEvent(NSEvent *event)
 {
-    // If this is the active window or we don't have a range selection, there is no need to perform additional checks
-    // and we can avoid making a synchronous call to the WebProcess.
-    if (m_view.window.isKeyWindow || m_page->editorState().selectionIsNone || !m_page->editorState().selectionIsRange)
+    if (!mightBeginDragWhileInactive())
         return false;
 
     // There's a chance that responding to this event will run a nested event loop, and