Bug 175528

Summary: [CoordGraphics] Simplify CoordinatedGraphicsScene state updates
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cgarcia, magomez, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Zan Dobersek 2017-08-14 04:23:22 PDT
[CoordGraphics] Simplify CoordinatedGraphicsScene state updates
Comment 1 Zan Dobersek 2017-08-14 04:57:16 PDT
Created attachment 318030 [details]
Patch
Comment 2 Build Bot 2017-08-14 04:58:43 PDT
Attachment 318030 [details] did not pass style-queue:


ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:234:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Carlos Garcia Campos 2017-08-14 05:10:26 PDT
Comment on attachment 318030 [details]
Patch

I never liked that render queue thing \o/
Comment 4 Zan Dobersek 2017-08-14 09:41:21 PDT
Comment on attachment 318030 [details]
Patch

Clearing flags on attachment: 318030

Committed r220700: <http://trac.webkit.org/changeset/220700>
Comment 5 Zan Dobersek 2017-08-14 09:41:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Radar WebKit Bug Importer 2017-08-14 09:42:26 PDT
<rdar://problem/33876795>
Comment 7 Carlos Garcia Campos 2017-08-14 23:21:03 PDT
Debug bot is exiting early after this change, hitting an assert:

ASSERTION FAILED: m_surfaces.contains(atlasID)
/home/slave/webkitgtk/gtk-linux-64-debug/build/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp(449) : void WebKit::CoordinatedGraphicsScene::removeUpdateAtlas(uint32_t)
Comment 8 Michael Catanzaro 2017-08-15 07:16:42 PDT
Reverted r220700 for reason:

Broke debug bot

Committed r220742: <http://trac.webkit.org/changeset/220742>
Comment 9 Zan Dobersek 2017-08-15 23:59:09 PDT
Created attachment 318240 [details]
Patch
Comment 10 Build Bot 2017-08-16 00:00:57 PDT
Attachment 318240 [details] did not pass style-queue:


ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:234:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Zan Dobersek 2017-08-16 00:01:03 PDT
Created attachment 318241 [details]
Patch
Comment 12 Zan Dobersek 2017-08-16 00:01:31 PDT
Comment on attachment 318241 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=318241&action=review

> Source/WebKit/ChangeLog:28
> +        Outside of CoordinatedGraphicsScene and ThreadedCompositor classes, the
> +        CompositingCoordinator class now passes the atlases-to-remove Vector by
> +        a const lvalue reference down to ThreadedCompositor, and then manually
> +        clears the Vector. Before the Vector was passed as an rvalue reference,
> +        depending on the ThreadedCompositor code to clear out the original Vector
> +        object by moving its resources into the functor object. This doesn't occur
> +        anymore because the Vector object is now appended to another Vector.

This describes the problem that was causing asserts.
Comment 13 Build Bot 2017-08-16 00:04:07 PDT
Attachment 318241 [details] did not pass style-queue:


ERROR: Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:234:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Zan Dobersek 2017-08-16 04:21:46 PDT
Comment on attachment 318241 [details]
Patch

Clearing flags on attachment: 318241

Committed r220792: <http://trac.webkit.org/changeset/220792>
Comment 15 Zan Dobersek 2017-08-16 04:21:50 PDT
All reviewed patches have been landed.  Closing bug.