This will be necessary to move control of process suppression to the WebProcess.
Created attachment 220178 [details] Fix
Comment on attachment 220178 [details] Fix Attachment 220178 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/5880595670368256
Created attachment 220182 [details] GTK fix
Comment on attachment 220182 [details] GTK fix View in context: https://bugs.webkit.org/attachment.cgi?id=220182&action=review > Source/WebKit2/UIProcess/WebContext.h:304 > +#if PLATFORM(MAC) && !PLATFORM(IOS) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1090 > + void windowModificationsStateChanged(); > + static WindowModificationsState windowModificationsState(); > +#else > + static WindowModificationsState windowModificationsState() { return WindowModificationsStarted; } > +#endif I don't get why something called WindowModificationsState would be on the WebContext (or static). "Window" level concepts should really a PageClient thing if possible. > Source/WebKit2/UIProcess/mac/WebContextMac.mm:137 > + for (auto *context : contexts) { * on the wrong side. > Source/WebKit2/UIProcess/mac/WebProcessProxyMac.mm:85 > +void WebProcessProxy::windowModificationsStateChanged() Even though this isn't currently used by other platforms, there isn't anything really mac specific about it. Can this just go in WebProcessProxy.cpp? > Source/WebKit2/UIProcess/mac/WebProcessProxyMac.mm:88 > + for (WebPageProxyMap::iterator it = m_pageMap.begin(), end = m_pageMap.end(); it != end; ++it) > + it->value->viewStateDidChange(ViewState::IsModifying); This can be written as: for (const auto& page : m_pageMap.values()) page->viewStateDidChange(ViewState::IsModifying);
(In reply to comment #4) > (From update of attachment 220182 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220182&action=review > > > Source/WebKit2/UIProcess/WebContext.h:304 > > +#if PLATFORM(MAC) && !PLATFORM(IOS) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1090 > > + void windowModificationsStateChanged(); > > + static WindowModificationsState windowModificationsState(); > > +#else > > + static WindowModificationsState windowModificationsState() { return WindowModificationsStarted; } > > +#endif > > I don't get why something called WindowModificationsState would be on the WebContext (or static). "Window" level concepts should really a PageClient thing if possible. The WindowModifications state is not per window, as you might expect from the name – it is per WindowServer connection (i.e. process wide). The code registers a single handler, and then broadcasts the message out to all WebPageProxies. The alternative would be to redundantly register multiple handlers which would all be receiving exactly the same message, which seemed inefficient. It looked like the best way to find all WebPageProxies in existence was from the WebContext – the WebContext statically know all WebContexts in existence, which in turn can find all WebPageProxies, which in turn can find all WebPageProxies. Did I miss a more direct route? > * on the wrong side. > > Even though this isn't currently used by other platforms, there isn't anything really mac specific about it. Can this just go in WebProcessProxy.cpp? > > This can be written as: Yep, yep, yep - will fix.
Created attachment 220269 [details] WIP
Created attachment 220275 [details] Fix
Committed revision 161246.
Comment on attachment 220275 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=220275&action=review > Source/WebKit2/ChangeLog:11 > + WindowServer connection basis, and as such may produce (safe) false positives. Did you mean "safe false negatives"? I don't think it's ever safe to falsely say that something is not visible to the user when in reality it is.
(In reply to comment #9) > (From update of attachment 220275 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220275&action=review > > > Source/WebKit2/ChangeLog:11 > > + WindowServer connection basis, and as such may produce (safe) false positives. > > Did you mean "safe false negatives"? I don't think it's ever safe to falsely say that something is not visible to the user when in reality it is. Ugh, yes you're right, this is now "safe false negatives" – the comment reflects the initial implementation. The flag was inverted – in the former version of the patch the value was called "IsModifying", which meant !IsVisuallyIdle – and as such we would sometimes produce safe false positives.