Use smaller tiles in windows that are not active to facilitate App Napping.
<rdar://problem/30358835>
Created attachment 300906 [details] WIP Patch Working on adding a test.
Comment on attachment 300906 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300906&action=review > Source/WebCore/page/FrameView.cpp:2790 > + // Use square tiles if the Window is not active to facilitate app napping. I think you must have meant something other than square?
Comment on attachment 300906 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300906&action=review >> Source/WebCore/page/FrameView.cpp:2790 >> + // Use square tiles if the Window is not active to facilitate app napping. > > I think you must have meant something other than square? smaller square tiles maybe? Will basically go back to 512x512, instead of (extra wide tiles or extra high tiles or one big tile).
Comment on attachment 300906 [details] WIP Patch Looks generally OK. Does this patch have any iOS impact?
(In reply to comment #5) > Comment on attachment 300906 [details] > WIP Patch > > Looks generally OK. Does this patch have any iOS impact? My patch may impact the tile sizes in the split screen case on iPad if you have 2 safari windows. I need to confirm. That said, we do not care about App Napping there so I do not need to impact iOS.
Created attachment 300932 [details] WIP patch
Comment on attachment 300932 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=300932&action=review > LayoutTests/compositing/tiling/non-active-window-tiles-size.html:18 > + setTimeout(function() { Simon: I need a better way to synchronize than this setTimeout(), any idea? a setTimeout(x, 0) does not suffice.
The tile size is adjusted on a: static const auto tileSizeUpdateDelay = std::chrono::milliseconds { 500 }; timer.
(In reply to comment #9) > The tile size is adjusted on a: > > static const auto tileSizeUpdateDelay = std::chrono::milliseconds { 500 }; > > timer. So you think, my setTimeout() is the way to go?
(In reply to comment #10) > (In reply to comment #9) > > The tile size is adjusted on a: > > > > static const auto tileSizeUpdateDelay = std::chrono::milliseconds { 500 }; > > > > timer. > > So you think, my setTimeout() is the way to go? Certainly a 500ms timer in a test is not the way to go. Maybe add an internals mechanism to force the update to happen immediately?
Comment on attachment 300932 [details] WIP patch Attachment 300932 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3026693 New failing tests: compositing/tiling/non-active-window-tiles-size.html
Created attachment 300938 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 300932 [details] WIP patch Attachment 300932 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3026720 New failing tests: compositing/tiling/non-active-window-tiles-size.html
Created attachment 300944 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 300932 [details] WIP patch Attachment 300932 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3026739 New failing tests: compositing/tiling/non-active-window-tiles-size.html
Created attachment 300948 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 300955 [details] WIP Patch
Created attachment 300963 [details] Patch
Comment on attachment 300963 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300963&action=review > Source/WebCore/page/Page.h:365 > + bool isWindowActive() const; Would read a bit better as "isWindowActive(), or maybe isInActiveWindow() Also, confusing with DOMWindow is likely, and do they have some notion of being "active"? Can we rename this to make it clear it's the native window that's active? Say frontmost? > LayoutTests/compositing/tiling/non-active-window-tiles-size.html:7 > +<div id="activeResult"></div> > +<p>Tiles when window is not active:</p> > +<div id="nonActiveResult"></div> Make the divs <pre> to preserve formatting.
Comment on attachment 300963 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300963&action=review >> Source/WebCore/page/Page.h:365 >> + bool isWindowActive() const; > > Would read a bit better as "isWindowActive(), or maybe isInActiveWindow() > > Also, confusing with DOMWindow is likely, and do they have some notion of being "active"? Can we rename this to make it clear it's the native window that's active? Say frontmost? I tried to match the name of our activity state: ActivityState::WindowIsActive. Also note that we already have a bool isVisibleAndActive() const; getter on Page which queries this state already. >> LayoutTests/compositing/tiling/non-active-window-tiles-size.html:7 >> +<div id="nonActiveResult"></div> > > Make the divs <pre> to preserve formatting. Will do
Created attachment 300972 [details] Patch
Comment on attachment 300972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300972&action=review > Source/WebCore/testing/Internals.cpp:398 > + backing->setTileSizeUpdateDelayDisabledForTesting(true); Shouldn't this say false???
Comment on attachment 300972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300972&action=review >> Source/WebCore/testing/Internals.cpp:398 >> + backing->setTileSizeUpdateDelayDisabledForTesting(true); > > Shouldn't this say false??? Oops :D
Created attachment 300973 [details] Patch
Comment on attachment 300972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300972&action=review > Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:165 > + [platformView() _updateActivityState]; This really isn't right. We already listen to windowDidBecomeKey and windowDidResignKey notifications, WKTR just isn't sending them because of the way it mocks out keyness. Seems like it would be better to fix that rather than expose bizarro internals as SPI.
Created attachment 300974 [details] Patch
Comment on attachment 300974 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300974&action=review > Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:168 > + if (m_windowIsKey) > + [m_window makeKeyWindow]; > + else > + [m_window resignKeyWindow]; Does this get reset between tests? > LayoutTests/platform/ios-simulator-wk2/compositing/tiling/non-active-window-tiles-size-expected.txt:7 > +(GraphicsLayer > +(anchor 0.00 0.00) > +(bounds 800.00 600.00) > +(visible rect 0.00, 0.00 800.00 x 600.00) > +(coverage rect 0.00, 0.00 800.00 x 600.00) I think you need to re-generate these results.
Comment on attachment 300974 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300974&action=review >> Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:168 >> + [m_window resignKeyWindow]; > > Does this get reset between tests? TestController::resetStateToConsistentValues() calls m_mainWebView->focus(); which calls setWindowIsKey(true); >> LayoutTests/platform/ios-simulator-wk2/compositing/tiling/non-active-window-tiles-size-expected.txt:7 >> +(coverage rect 0.00, 0.00 800.00 x 600.00) > > I think you need to re-generate these results. I know, I'll get them from EWS :P
Comment on attachment 300974 [details] Patch Attachment 300974 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3028159 New failing tests: compositing/tiling/non-active-window-tiles-size.html
Created attachment 300985 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Committed r211910: <http://trac.webkit.org/changeset/211910>