Bug 188693

Summary: [CoordGraphics] Switch to Nicosia::CompositionLayer state tracking
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, ews-watchlist, magomez, mcatanzaro, pgriffis, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=188838
https://bugs.webkit.org/show_bug.cgi?id=188839
https://bugs.webkit.org/show_bug.cgi?id=189224
https://bugs.webkit.org/show_bug.cgi?id=196190
Bug Depends on:    
Bug Blocks: 187385    
Attachments:
Description Flags
WIP
none
Archive of layout-test-results from ews205 for win-future
none
WIP
none
WIP
none
Archive of layout-test-results from ews200 for win-future
none
Patch
none
Archive of layout-test-results from ews202 for win-future
none
WIP
none
Patch
none
Archive of layout-test-results from ews202 for win-future
none
Patch for landing none

Zan Dobersek
Reported 2018-08-17 01:55:39 PDT
SSIA.
Attachments
WIP (60.21 KB, patch)
2018-08-17 01:59 PDT, Zan Dobersek
no flags
Archive of layout-test-results from ews205 for win-future (12.91 MB, application/zip)
2018-08-18 09:29 PDT, EWS Watchlist
no flags
WIP (60.83 KB, patch)
2018-08-19 23:56 PDT, Zan Dobersek
no flags
WIP (61.88 KB, patch)
2018-08-20 01:04 PDT, Zan Dobersek
no flags
Archive of layout-test-results from ews200 for win-future (12.90 MB, application/zip)
2018-08-20 06:56 PDT, EWS Watchlist
no flags
Patch (72.71 KB, patch)
2018-08-20 09:24 PDT, Zan Dobersek
no flags
Archive of layout-test-results from ews202 for win-future (12.69 MB, application/zip)
2018-08-20 11:17 PDT, EWS Watchlist
no flags
WIP (72.73 KB, patch)
2018-08-20 23:35 PDT, Zan Dobersek
no flags
Patch (72.70 KB, patch)
2018-08-21 04:38 PDT, Zan Dobersek
no flags
Archive of layout-test-results from ews202 for win-future (12.93 MB, application/zip)
2018-08-21 13:24 PDT, EWS Watchlist
no flags
Patch for landing (72.93 KB, patch)
2018-08-22 05:02 PDT, Zan Dobersek
no flags
Zan Dobersek
Comment 1 2018-08-17 01:59:13 PDT
EWS Watchlist
Comment 2 2018-08-17 02:01:34 PDT
Attachment 347355 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:319: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:1253: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: 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: 4 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 3 2018-08-18 09:28:54 PDT
Comment on attachment 347355 [details] WIP Attachment 347355 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8902038 New failing tests: imported/blink/transitions/unprefixed-transform.html legacy-animation-engine/imported/blink/transitions/unprefixed-transform.html
EWS Watchlist
Comment 4 2018-08-18 09:29:05 PDT
Created attachment 347444 [details] Archive of layout-test-results from ews205 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews205 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Zan Dobersek
Comment 5 2018-08-19 23:56:34 PDT
EWS Watchlist
Comment 6 2018-08-19 23:57:28 PDT
Attachment 347483 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:319: More than one command on the same line [whitespace/newline] [4] ERROR: 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: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Zan Dobersek
Comment 7 2018-08-20 01:04:41 PDT
EWS Watchlist
Comment 8 2018-08-20 01:05:55 PDT
Attachment 347487 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:331: More than one command on the same line [whitespace/newline] [4] ERROR: 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: 3 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 9 2018-08-20 06:56:38 PDT
Comment on attachment 347487 [details] WIP Attachment 347487 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8916565 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html
EWS Watchlist
Comment 10 2018-08-20 06:56:49 PDT
Created attachment 347494 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Zan Dobersek
Comment 11 2018-08-20 09:24:10 PDT
EWS Watchlist
Comment 12 2018-08-20 09:27:12 PDT
Attachment 347502 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:331: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 13 2018-08-20 11:16:49 PDT
Comment on attachment 347502 [details] Patch Attachment 347502 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8918569 New failing tests: http/tests/security/canvas-remote-read-remote-video-localhost.html http/tests/security/video-poster-cross-origin-crash2.html
EWS Watchlist
Comment 14 2018-08-20 11:17:00 PDT
Created attachment 347514 [details] Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Zan Dobersek
Comment 15 2018-08-20 23:35:30 PDT
EWS Watchlist
Comment 16 2018-08-20 23:37:01 PDT
Attachment 347616 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:331: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Zan Dobersek
Comment 17 2018-08-21 04:38:21 PDT
EWS Watchlist
Comment 18 2018-08-21 04:40:50 PDT
Attachment 347627 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:331: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
EWS Watchlist
Comment 19 2018-08-21 13:24:44 PDT
Comment on attachment 347627 [details] Patch Attachment 347627 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8933514 New failing tests: http/tests/security/canvas-remote-read-remote-video-localhost.html http/tests/security/video-poster-cross-origin-crash2.html
EWS Watchlist
Comment 20 2018-08-21 13:24:55 PDT
Created attachment 347678 [details] Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Carlos Garcia Campos
Comment 21 2018-08-22 00:34:43 PDT
Comment on attachment 347627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347627&action=review I would file bugs for all the FIXMEs to ensure we don't forget about it. > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:453 > + m_nicosia.contentLayer = contentLayer; Should we do this inside the previous if? > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:831 > + } else { > + if (m_nicosia.backingStore) { else if
Zan Dobersek
Comment 22 2018-08-22 03:13:36 PDT
Comment on attachment 347627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347627&action=review >> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:453 >> + m_nicosia.contentLayer = contentLayer; > > Should we do this inside the previous if? We can. >> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:831 >> + if (m_nicosia.backingStore) { > > else if OK.
Zan Dobersek
Comment 23 2018-08-22 05:02:26 PDT
Created attachment 347779 [details] Patch for landing
EWS Watchlist
Comment 24 2018-08-22 05:04:37 PDT
Attachment 347779 [details] did not pass style-queue: ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:113: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp:331: More than one command on the same line [whitespace/newline] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Zan Dobersek
Comment 25 2018-08-22 05:14:16 PDT
Comment on attachment 347779 [details] Patch for landing Clearing flags on attachment: 347779 Committed r235165: <https://trac.webkit.org/changeset/235165>
Zan Dobersek
Comment 26 2018-08-22 05:14:20 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 27 2018-08-22 05:16:08 PDT
Michael Catanzaro
Comment 28 2018-09-01 20:38:49 PDT
Hi Zan, this commit caused a *very major* video playback performance regression. It should be obvious playing any video on youtube.com, or http://download.blender.org/peach/bigbuckbunny_movies/big_buck_bunny_720p_stereo.ogg.
Zan Dobersek
Comment 29 2018-10-02 04:28:42 PDT
(In reply to Michael Catanzaro from comment #28) > Hi Zan, this commit caused a *very major* video playback performance > regression. It should be obvious playing any video on youtube.com, or > http://download.blender.org/peach/bigbuckbunny_movies/ > big_buck_bunny_720p_stereo.ogg. This was addressed in bug #189224.
Note You need to log in before you can comment on or make changes to this bug.