Bug 93077 - [WK2] LayerTreeCoordinator can disallow tile updates in forceRepaint
Summary: [WK2] LayerTreeCoordinator can disallow tile updates in forceRepaint
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 93121
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-03 01:06 PDT by Balazs Kelemen
Modified: 2012-08-03 09:08 PDT (History)
3 users (show)

See Also:


Attachments
proposed fix (4.05 KB, patch)
2012-08-03 01:15 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Balazs Kelemen 2012-08-03 01:06:23 PDT
This has been revealed in connection with bug 90394 and it makes repaint tests failing.
Comment 1 Balazs Kelemen 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.
Comment 2 Jocelyn Turcotte 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.
Comment 3 Balazs Kelemen 2012-08-03 06:07:27 PDT
Comment on attachment 156283 [details]
proposed fix

Landed in http://trac.webkit.org/changeset/124603
Comment 4 Csaba Osztrogonác 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
Comment 5 Balazs Kelemen 2012-08-03 08:49:49 PDT
Reopening because of the crashes.
Comment 6 Balazs Kelemen 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>