Changeset 187471 in webkit

Timestamp:

Jul 27, 2015 5:43:04 PM (9 years ago)

Author:

timothy_horton@apple.com

Message:

First in-window viewStateChange synchronously blocks despite not previously being in-window
​https://bugs.webkit.org/show_bug.cgi?id=147344
<rdar://problem/22021772>

Reviewed by Simon Fraser.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::updateViewState):
(WebKit::WebPageProxy::dispatchViewStateChange):
The whole point of m_viewWasEverInWindow was so that we would not
synchronously wait when a view was added to a window for the first time,
only all subsequent times.

However, since m_viewWasEverInWindow was being set *before* being
checked in dispatchViewStateChange, we were always blocking. This is
a huge waste of main-thread time, because there's no reason to wait
for the first paint if you've never seen the view before (and shouldn't
expect it to have content).

Instead, set the flag after dispatching a view state change, so it becomes
"have we ever sent a view state with IsInWindow set" instead.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (3 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-07-27  Tim Horton  <timothy_horton@apple.com>
+
+        First in-window viewStateChange synchronously blocks despite not previously being in-window
+        https://bugs.webkit.org/show_bug.cgi?id=147344
+        <rdar://problem/22021772>
+
+        Reviewed by Simon Fraser.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::updateViewState):
+        (WebKit::WebPageProxy::dispatchViewStateChange):
+        The whole point of m_viewWasEverInWindow was so that we would not
+        synchronously wait when a view was added to a window for the first time,
+        only all subsequent times.
+
+        However, since m_viewWasEverInWindow was being set *before* being
+        checked in dispatchViewStateChange, we were always blocking. This is
+        a huge waste of main-thread time, because there's no reason to wait
+        for the first paint if you've never seen the view before (and shouldn't
+        expect it to have content).
+
+        Instead, set the flag after dispatching a view state change, so it becomes
+        "have we ever sent a view state with IsInWindow set" instead.
+
 2015-07-27  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp

@@ -1331 +1331 @@
     if (flagsToUpdate & ViewState::IsVisibleOrOccluded && m_pageClient.isViewVisibleOrOccluded())
         m_viewState |= ViewState::IsVisibleOrOccluded;
-    if (flagsToUpdate & ViewState::IsInWindow && m_pageClient.isViewInWindow()) {
+    if (flagsToUpdate & ViewState::IsInWindow && m_pageClient.isViewInWindow())
         m_viewState |= ViewState::IsInWindow;
-        m_viewWasEverInWindow = true;
-    }
     if (flagsToUpdate & ViewState::IsVisuallyIdle && m_pageClient.isVisuallyIdle())
         m_viewState |= ViewState::IsVisuallyIdle;
@@ -1402 +1400 @@
     ViewState::Flags changed = m_viewState ^ previousViewState;
 
+    bool isNowInWindow = (changed & ViewState::IsInWindow) && isInWindow();
     // We always want to wait for the Web process to reply if we've been in-window before and are coming back in-window.
-    if (m_viewWasEverInWindow && (changed & ViewState::IsInWindow) && isInWindow() && m_drawingArea->hasVisibleContent())
+    if (m_viewWasEverInWindow && isNowInWindow && m_drawingArea->hasVisibleContent())
         m_viewStateChangeWantsSynchronousReply = true;
 
@@ -1438 +1437 @@
     m_potentiallyChangedViewStateFlags = ViewState::NoFlags;
     m_viewStateChangeWantsSynchronousReply = false;
+    m_viewWasEverInWindow |= isNowInWindow;
 }