Bug 186909

Summary: https://hackernoon.com/ uses lots of layer backing store
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cdumez, commit-queue, ews-watchlist, graouts, realdawei, rniwa, ryanhaddad, simon.fraser, sondracake50, thorton, tsavell, 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=188655
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews100 for mac-sierra
none
Archive of layout-test-results from ews114 for mac-sierra
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Archive of layout-test-results from ews106 for mac-sierra-wk2
none
Patch
none
Patch for EWS
none
Patch none

Description Simon Fraser (smfr) 2018-06-21 21:29:36 PDT
https://hackernoon.com/ uses lots of layer backing store
Comment 1 Simon Fraser (smfr) 2018-06-21 21:34:03 PDT
Created attachment 343307 [details]
Patch
Comment 2 Simon Fraser (smfr) 2018-06-21 21:34:28 PDT
rdar://problem/40257540
Comment 3 Simon Fraser (smfr) 2018-06-21 21:35:17 PDT
I need to verify that the PlatformCALayerRemote part of this is correct. The test results seem wrong.
Comment 4 EWS Watchlist 2018-06-21 22:46:50 PDT
Comment on attachment 343307 [details]
Patch

Attachment 343307 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8286744

New failing tests:
fast/images/animated-gif-iframe-webkit-transform.html
Comment 5 EWS Watchlist 2018-06-21 22:46:52 PDT
Created attachment 343309 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 6 EWS Watchlist 2018-06-21 23:14:34 PDT
Comment on attachment 343307 [details]
Patch

Attachment 343307 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8286774

New failing tests:
fast/images/animated-gif-iframe-webkit-transform.html
Comment 7 EWS Watchlist 2018-06-21 23:14:36 PDT
Created attachment 343310 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 8 EWS Watchlist 2018-06-21 23:28:25 PDT
Comment on attachment 343307 [details]
Patch

Attachment 343307 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8286810

New failing tests:
compositing/backing/backing-store-attachment-outside-viewport.html
compositing/backing-store-attachment-1.html
Comment 9 EWS Watchlist 2018-06-21 23:28:27 PDT
Created attachment 343311 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 10 EWS Watchlist 2018-06-22 03:21:00 PDT
Comment on attachment 343307 [details]
Patch

Attachment 343307 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8288326

New failing tests:
fast/images/animated-gif-iframe-webkit-transform.html
Comment 11 EWS Watchlist 2018-06-22 03:21:02 PDT
Created attachment 343316 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 12 Brent Fulgham 2018-06-22 13:02:56 PDT
Comment on attachment 343307 [details]
Patch

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

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:-890
> -        return;

It's surprising that you want to do the rest of this method if there's no backing store. Do you need to signal DirtyRectsChanged or set a repaint rect to get your new behavior to work?

> Source/WebCore/platform/graphics/ca/cocoa/PlatformCALayerCocoa.mm:750
> +    return [m_layer contents];

Why didn't you use "!!m_layer.contents" here like you did in PlatformCALayerRemote::hasContents()?
Comment 13 Brent Fulgham 2018-06-22 13:04:18 PDT
performance-api/performance-observer-no-document-leak.html is flaky for reasons unrelated to this patch.
Comment 14 Brent Fulgham 2018-06-22 13:08:47 PDT
Comment on attachment 343307 [details]
Patch

This patch seems to contribute to flakiness on the test infrastructure.
Comment 15 Chris Dumez 2018-06-22 13:11:51 PDT
(In reply to Brent Fulgham from comment #14)
> Comment on attachment 343307 [details]
> Patch
> 
> This patch seems to contribute to flakiness on the test infrastructure.

If you're referring to performance-api/performance-observer-no-document-leak.html, then it is my test and has nothing to do with this patch. I am currently looking into why performance-api/performance-observer-no-document-leak.html is flaky (I added this test recently).
Comment 16 Simon Fraser (smfr) 2018-06-25 17:13:37 PDT
Looks like I broke some animated image tests; investigating.
Comment 17 Simon Fraser (smfr) 2018-06-26 19:36:01 PDT
Created attachment 343678 [details]
Patch
Comment 18 Simon Fraser (smfr) 2018-06-27 10:15:34 PDT
Created attachment 343719 [details]
Patch for EWS
Comment 19 Simon Fraser (smfr) 2018-06-27 10:19:43 PDT
Created attachment 343720 [details]
Patch
Comment 20 WebKit Commit Bot 2018-06-27 11:22:14 PDT
Comment on attachment 343678 [details]
Patch

Clearing flags on attachment: 343678

Committed r233268: <https://trac.webkit.org/changeset/233268>
Comment 21 WebKit Commit Bot 2018-06-27 11:22:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 22 Ryan Haddad 2018-06-27 12:52:44 PDT
This change broke the Windows build:
https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/10303

C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\win\PlatformCALayerWin.cpp(50): error C2259: 'WebCore::PlatformCALayerWin': cannot instantiate abstract class [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\win\PlatformCALayerWin.cpp(50): note: due to following members:
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\win\PlatformCALayerWin.cpp(50): note: 'bool WebCore::PlatformCALayer::hasContents(void) const': is abstract
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\PlatformCALayer.h(183): note: see declaration of 'WebCore::PlatformCALayer::hasContents'
C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\win\PlatformCALayerWin.cpp(55): error C2259: 'WebCore::PlatformCALayerWin': cannot instantiate abstract class [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\win\PlatformCALayerWin.cpp(55): note: due to following members:
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\win\PlatformCALayerWin.cpp(55): note: 'bool WebCore::PlatformCALayer::hasContents(void) const': is abstract
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\platform\graphics\ca\PlatformCALayer.h(183): note: see declaration of 'WebCore::PlatformCALayer::hasContents'
Comment 23 Simon Fraser (smfr) 2018-06-27 12:53:50 PDT
Will fix.
Comment 24 Simon Fraser (smfr) 2018-06-27 13:03:00 PDT
Windows build fix in https://trac.webkit.org/changeset/233274/webkit
Comment 25 Antoine Quint 2018-08-16 08:40:56 PDT
This caused a regression, see https://bugs.webkit.org/show_bug.cgi?id=188655.