Bug 198791 - [Nicosia] Add SceneIntegration
Summary: [Nicosia] Add SceneIntegration
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords: InRadar
Depends on:
Blocks: 198475
  Show dependency treegraph
 
Reported: 2019-06-12 09:11 PDT by Zan Dobersek
Modified: 2019-06-13 07:06 PDT (History)
6 users (show)

See Also:


Attachments
WIP (18.58 KB, patch)
2019-06-12 09:13 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (19.87 KB, patch)
2019-06-12 13:47 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews211 for win-future (13.98 MB, application/zip)
2019-06-12 17:31 PDT, Build Bot
no flags Details
Patch for landing (20.05 KB, patch)
2019-06-13 06:57 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2019-06-12 09:11:40 PDT
[Nicosia] Add SceneIntegration
Comment 1 Zan Dobersek 2019-06-12 09:13:46 PDT
Created attachment 371965 [details]
WIP
Comment 2 Build Bot 2019-06-12 09:17:40 PDT
Attachment 371965 [details] did not pass style-queue:


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: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Zan Dobersek 2019-06-12 13:47:44 PDT
Created attachment 371983 [details]
Patch
Comment 4 Build Bot 2019-06-12 17:31:02 PDT
Comment on attachment 371983 [details]
Patch

Attachment 371983 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12458738

New failing tests:
imported/blink/fast/canvas/canvas-clip-stack-persistence.html
svg/text/textpath-reference-update.html
fast/shadow-dom/svg-thref-href-change-in-shadow-tree.html
css3/filters/blur-various-radii.html
imported/blink/fast/canvas/bug382588.html
storage/indexeddb/modern/get-keyrange.html
Comment 5 Build Bot 2019-06-12 17:31:06 PDT
Created attachment 372007 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment 6 Carlos Garcia Campos 2019-06-13 04:53:20 PDT
Comment on attachment 371983 [details]
Patch

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

> Source/WebCore/platform/graphics/nicosia/NicosiaSceneIntegration.h:47
> +    SceneIntegration();

Why is this needed?

> Source/WebCore/platform/graphics/nicosia/NicosiaSceneIntegration.h:48
> +    SceneIntegration(Client&);

explicit

> Source/WebCore/platform/graphics/nicosia/NicosiaSceneIntegration.h:59
> +        UpdateScope(Ref<SceneIntegration>&&);

explicit

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/LayerTreeHost.cpp:56
> +    , m_sceneIntegration(*new Nicosia::SceneIntegration(*this))

I would add a Nicosia::SceneIntegration::create method returning a Ref

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/LayerTreeHost.h:44
> +#if USE(COORDINATED_GRAPHICS)
> +#include <WebCore/NicosiaSceneIntegration.h>
> +#endif

If NicosiaSceneIntegration is specific to COORDINATED_GRAPHICS I would move the guard to the header instead (and the cpp)
Comment 7 Zan Dobersek 2019-06-13 06:04:28 PDT
Comment on attachment 371983 [details]
Patch

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

>> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/LayerTreeHost.h:44
>> +#endif
> 
> If NicosiaSceneIntegration is specific to COORDINATED_GRAPHICS I would move the guard to the header instead (and the cpp)

It's not, but it is only used in this class if USE(COORDINATED_GRAPHICS) is enabled.
Comment 8 Zan Dobersek 2019-06-13 06:57:39 PDT
Created attachment 372048 [details]
Patch for landing
Comment 9 Zan Dobersek 2019-06-13 07:05:41 PDT
Comment on attachment 372048 [details]
Patch for landing

Clearing flags on attachment: 372048

Committed r246400: <https://trac.webkit.org/changeset/246400>
Comment 10 Zan Dobersek 2019-06-13 07:05:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2019-06-13 07:06:22 PDT
<rdar://problem/51706849>