When Mobile Safari is brought to the foreground, it may have a snapshot from the page before it went to sleep. An immediate update is needed to refresh the page in case the contents have changed. In this case, we need to draw all the images synchronously even if the tile is painted for the first time. This will prevent unwanted flashing in case the page contents were not changed. To do that: -- SetActivityState message will include an activityStateChangeID member. -- LayerTreeTransaction will include an activityStateChangeID member. -- UIProcess sends an activityStateChangeID with the SetActivityState message if FlushImmediately paint is needed. Otherwise it sends zero. UIProcess waits for the LayerTreeTransaction whose activityStateChangeID is greater than or equal to the one it sent. If it receives the desired LayerTreeTransaction, the tree layer will be committed otherwise it will be ignored. -- WebProcess will read the activityStateChangeID from the SetActivityState message and will pass it in LayerTreeTransaction. Otherwise it will pass zero.
Created attachment 344387 [details] Patch
<rdar://problem/41249545>
Comment on attachment 344387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344387&action=review > Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:264 > + uint64_t activityStateChangeID() const { return m_activityStateChangeID; } 3...2...1... until Simon shows up and asks for a typedef here. > Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:451 > + while (m_webPageProxy.process().connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_webPageProxy.pageID(), activityStateUpdateTimeout, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives)) { You've completely defeated the timeout here. > Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:488 > -void RemoteLayerTreeDrawingArea::activityStateDidChange(ActivityState::Flags, bool wantsDidUpdateActivityState, const Vector<CallbackID>&) > +void RemoteLayerTreeDrawingArea::activityStateDidChange(ActivityState::Flags, uint64_t activityStateChangeID, const Vector<CallbackID>&) This is a little odd (and also breaking the build). It's weird that activityStateChangeID isn't for every activityStateChange, only ones that want a reply.
Comment on attachment 344387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344387&action=review >> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:264 >> + uint64_t activityStateChangeID() const { return m_activityStateChangeID; } > > 3...2...1... until Simon shows up and asks for a typedef here. Yes please. > Source/WebKit/UIProcess/DrawingAreaProxy.h:87 > + virtual void waitForDidUpdateActivityState(uint64_t) { } That would make this more readable. > Source/WebKit/UIProcess/WebPageProxy.h:1130 > + uint64_t takeNextActivityStateChangeID() { return ++m_currentActivityStateChangeID; } I'd like a comment that tells me what an "activity state" is in this context.
Comment on attachment 344387 [details] Patch Attachment 344387 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8451512 New failing tests: http/tests/security/contentSecurityPolicy/video-redirect-allowed.html
Created attachment 344395 [details] Archive of layout-test-results from ews201 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews201 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 344427 [details] Patch
Comment on attachment 344387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344387&action=review >>> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreeTransaction.h:264 >>> + uint64_t activityStateChangeID() const { return m_activityStateChangeID; } >> >> 3...2...1... until Simon shows up and asks for a typedef here. > > Yes please. A typedef for ActivityStateChangeID was added. >> Source/WebKit/UIProcess/DrawingAreaProxy.h:87 >> + virtual void waitForDidUpdateActivityState(uint64_t) { } > > That would make this more readable. Done. >> Source/WebKit/UIProcess/WebPageProxy.h:1130 >> + uint64_t takeNextActivityStateChangeID() { return ++m_currentActivityStateChangeID; } > > I'd like a comment that tells me what an "activity state" is in this context. A comment was added to explain the when the ActivityStateChangeID is sent and what it is used for. >> Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:451 >> + while (m_webPageProxy.process().connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_webPageProxy.pageID(), activityStateUpdateTimeout, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives)) { > > You've completely defeated the timeout here. Yes you are right. I missed this point in the last patch. I will update it. >> Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:488 >> +void RemoteLayerTreeDrawingArea::activityStateDidChange(ActivityState::Flags, uint64_t activityStateChangeID, const Vector<CallbackID>&) > > This is a little odd (and also breaking the build). It's weird that activityStateChangeID isn't for every activityStateChange, only ones that want a reply. I think passing wantsDidUpdateActivityState and activityStateChangeID will be redundant. If wantsDidUpdateActivityState = false, activityStateChangeID has to be = 0. And if wantsDidUpdateActivityState = true, activityStateChangeID has be > 0.
Created attachment 344430 [details] Patch
Comment on attachment 344387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344387&action=review >>> Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:451 >>> + while (m_webPageProxy.process().connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_webPageProxy.pageID(), activityStateUpdateTimeout, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives)) { >> >> You've completely defeated the timeout here. > > Yes you are right. I missed this point in the last patch. I will update it. Done.
Comment on attachment 344430 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344430&action=review > Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:454 > + while (m_webPageProxy.process().connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_webPageProxy.pageID(), activityStateUpdateTimeout - (MonotonicTime::now() - startTime), IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives)) { Is this OK if it timeout - delta goes negative? > Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:166 > + ActivityStateChangeID m_activityStateChangeID { ActivityStateChangeAsynchronous }; IDs with “special values” that actually mean something are a little weird :P
Comment on attachment 344430 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344430&action=review >> Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:454 >> + while (m_webPageProxy.process().connection()->waitForAndDispatchImmediately<Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTree>(m_webPageProxy.pageID(), activityStateUpdateTimeout - (MonotonicTime::now() - startTime), IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives)) { > > Is this OK if it timeout - delta goes negative? Yes. Connection::waitForAndDispatchImmediately() calls Connection::waitForMessage() timeout < 0. Connection::waitForMessage() calculates absoluteTimeout = MonotonicTime::now() + timeout. Connection::waitForMessage() calls Condition::waitUntil() with timeout < MonotonicTime::now(). Condition::waitUntil() will return false because it checks timeout < timeout.nowWithSameClock() at the beginning which will be true in this case. >> Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:166 >> + ActivityStateChangeID m_activityStateChangeID { ActivityStateChangeAsynchronous }; > > IDs with “special values” that actually mean something are a little weird :P is this better? ActivityStateChangeID m_activityStateChangeID { ActivityStateChangeIDNil };
Created attachment 344854 [details] Patch
Comment on attachment 344854 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344854&action=review > Source/WebKit/ChangeLog:3 > + [iOS] When asking for FlushImmediately, the UIProcess should ignore all the layer tree transactions which are sent before this request is received by the WebProcess The title could use some love. “When asking for FlushImmediately” means nothing to me unless I already have context. Also, we’re not /ignoring/ them (that would be disastrous), we’re just synchronously applying a bunch in a row. > Source/WebKit/Shared/DrawingAreaInfo.h:42 > + ActivityStateChangeIDNil = 0 Not really better. Go with the previous one.
Comment on attachment 344854 [details] Patch Attachment 344854 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8517587 New failing tests: http/tests/preload/onload_event.html
Created attachment 344869 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Created attachment 344879 [details] Patch
Comment on attachment 344879 [details] Patch Clearing flags on attachment: 344879 Committed r233781: <https://trac.webkit.org/changeset/233781>
All reviewed patches have been landed. Closing bug.