Bug 93077

Summary: [WK2] LayerTreeCoordinator can disallow tile updates in forceRepaint
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: REOPENED    
Severity: Normal CC: jturcotte, noam, ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 93121    
Bug Blocks:    
Attachments:
Description Flags
proposed fix none

Balazs Kelemen
Reported 2012-08-03 01:06:23 PDT
This has been revealed in connection with bug 90394 and it makes repaint tests failing.
Attachments
proposed fix (4.05 KB, patch)
2012-08-03 01:15 PDT, Balazs Kelemen 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Balazs Kelemen
Comment 1 2012-08-03 01:15:17 PDT
Created attachment 156283 [details] proposed fix This was suggested by Joselyn :) Actually I'm not 100% sure it's correct. It fixes the tests and don't introduce new fails. But is it ok to send tile updates when waiting for the ui process? I guess it works because when running WTR, the UI expects this behavior. As long as the only user of force repaint is wtr this makes no danger but still I wonder whether should we involve the UI side in fixing this.
Jocelyn Turcotte
Comment 2 2012-08-03 03:30:48 PDT
Comment on attachment 156283 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=156283&action=review Yep that's what I had in mind. I think it's ok to override m_waitingForUIProcess here since forceRepaint does the same for the rest of the layer sync operations by calling flushPendingLayerChanges directly instead of performScheduledLayerFlush. > Source/WebKit2/WebProcess/WebPage/LayerTreeCoordinator/LayerTreeCoordinator.cpp:179 > + WTF::TemporaryChange<bool> inForceRepaint(m_inForceRepaint, true); Wow, didn't know we had such a class.
Balazs Kelemen
Comment 3 2012-08-03 06:07:27 PDT
Comment on attachment 156283 [details] proposed fix Landed in http://trac.webkit.org/changeset/124603
Csaba Osztrogonác
Comment 4 2012-08-03 07:34:19 PDT
(In reply to comment #3) > (From update of attachment 156283 [details]) > Landed in http://trac.webkit.org/changeset/124603 It made 18 tests crash, here is the brand new bug report for you: https://bugs.webkit.org/show_bug.cgi?id=93121
Balazs Kelemen
Comment 5 2012-08-03 08:49:49 PDT
Reopening because of the crashes.
Balazs Kelemen
Comment 6 2012-08-03 09:08:29 PDT
Reverted r124603 for reason: Made a few tests crash under LayerTreeCoordinator::flushPendingLayerChanges Committed r124620: <http://trac.webkit.org/changeset/124620>
Note You need to log in before you can comment on or make changes to this bug.