Don't force rendering in finishAllRendering
Created attachment 142816 [details] Patch
Attachment 142816 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 142816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142816&action=review > Source/WebCore/ChangeLog:7 > + Can you explain why here?
Comment on attachment 142816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142816&action=review Yes! >> Source/WebCore/ChangeLog:8 >> + No new tests. (OOPS!) > > You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] You'll have to do this - might be simple enough to write a CCLayerTreeHostTest that hits this case.
Created attachment 142840 [details] Patch
Comment on attachment 142840 [details] Patch We need to reconcile this with why this existed in the first place before we just go fixing the bug. This was added way back very specifically to prevent flash prevention when switching modes on Windows and OSX. You should check with @jbates to see if this will regress us in some way. EVen better, you could talk with John and figure out how to remove this flow entirely, since the only reason it exists is for flashes. I'm sure updateRect unification fixes this, as does a lot of the recent transport stuff. Let me know, but I'd like to understand the plan before we just remove this.
+jbates. As discussed, this only affects the thread, which we're not shipping on any platform where we don't have forced compositing, so this will not introduce any flash on any platform we ship. I'm happy to remove the finish altogether if we can, but otherwise we need a way to fix this correctness issue (using a texture after it's been destroyed)
(In reply to comment #7) > +jbates. > > As discussed, this only affects the thread, which we're not shipping on any platform where we don't have forced compositing, so this will not introduce any flash on any platform we ship. If you do land this, please add a blocking bug for http://crbug.com/128385 to fix the flash so that threaded compositing is not turned on for other platforms that may not have force compositing mode on.
Thanks Antoine for studying this. I know it was a pain to do. This seems sane to me, given the constraints.
Comment on attachment 142840 [details] Patch Yeah, I'm not sure whether we really have a flash in threaded mode or not but I think it's safer to assume that we don't until proven otherwise. The finishAllRendering() call was added well before we got threaded mode up and running.
Comment on attachment 142840 [details] Patch Clearing flags on attachment: 142840 Committed r117825: <http://trac.webkit.org/changeset/117825>
All reviewed patches have been landed. Closing bug.