Bug 207134 - Make HostWindow be the creator of the remote ImageBuffer
Summary: Make HostWindow be the creator of the remote ImageBuffer
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on: 207109
Blocks: 204955 207198
  Show dependency tree / graph
 
Reported: 2020-02-03 11:01 PST by Said Abou-Hallawa
Modified: 2020-02-27 12:13 PST (History)
27 users (show)

See Also:

Attachments
Patch (386.66 KB, patch)
2020-02-03 11:06 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (386.96 KB, patch)
2020-02-03 14:29 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (387.02 KB, patch)
2020-02-03 15:17 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch for review (19.63 KB, patch)
2020-02-03 15:47 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (19.32 KB, patch)
2020-02-21 17:14 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (19.57 KB, patch)
2020-02-21 17:35 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.10 KB, patch)
2020-02-24 16:11 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.11 KB, patch)
2020-02-24 17:32 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.13 KB, patch)
2020-02-24 17:59 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.14 KB, patch)
2020-02-25 08:56 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Patch (10.15 KB, patch)
2020-02-25 09:30 PST, Said Abou-Hallawa 		no flags Details | Formatted Diff | Diff
Show Obsolete (10) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2020-02-03 11:01:47 PST 
RenderingBackend will be responsible for creating the ImageBuffer. The plan is to have RemoteRenderingBackend which inherits RenderingBackend and lives in WebKit be responsible for creating the RemoteImageBuffer. The goal for RenderingBackend and RemoteRenderingBackend is to interact with RemoteImageBuffer seamlessly in the caller side.
Comment 1 Said Abou-Hallawa 2020-02-03 11:06:40 PST 
Created attachment 389540 [details]
Patch
Comment 2 Said Abou-Hallawa 2020-02-03 14:29:03 PST 
Created attachment 389570 [details]
Patch
Comment 3 Said Abou-Hallawa 2020-02-03 15:17:51 PST 
Created attachment 389579 [details]
Patch
Comment 4 Said Abou-Hallawa 2020-02-03 15:47:11 PST 
Created attachment 389584 [details]
Patch  for review
Comment 5 Said Abou-Hallawa 2020-02-21 17:14:49 PST 
Created attachment 391435 [details]
Patch
Comment 6 Said Abou-Hallawa 2020-02-21 17:35:36 PST 
Created attachment 391438 [details]
Patch
Comment 7 Sam Weinig 2020-02-21 18:02:03 PST 
Comment on attachment 391438 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +
> +        RenderingBackend will be responsible for creating the ImageBuffer. In a
> +        following patch RemoteRenderingBackend will be introduced. It'll inherit
> +        RenderingBackend and live in WebKit. It will be responsible for creating
> +        the RemoteImageBuffer. The goal for RenderingBackend and RemoteRenderingBackend
> +        is to interact with RemoteImageBuffer seamlessly in the caller side.

What is the added value of this additional interface over doing this via HostWindow?
Comment 8 Sam Weinig 2020-02-21 18:05:11 PST 
(In reply to Sam Weinig from comment #7)
> Comment on attachment 391438 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=391438&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +
> > +        RenderingBackend will be responsible for creating the ImageBuffer. In a
> > +        following patch RemoteRenderingBackend will be introduced. It'll inherit
> > +        RenderingBackend and live in WebKit. It will be responsible for creating
> > +        the RemoteImageBuffer. The goal for RenderingBackend and RemoteRenderingBackend
> > +        is to interact with RemoteImageBuffer seamlessly in the caller side.
> 
> What is the added value of this additional interface over doing this via
> HostWindow?

To be more specific, the alternative I imagine is adding the createImageBuffer() virtual functions to HostWindow, and having Chrome forward those to createImageBuffer() virtual functions on the ChromeClient.
Comment 9 Said Abou-Hallawa 2020-02-24 16:11:11 PST 
Created attachment 391593 [details]
Patch
Comment 10 Said Abou-Hallawa 2020-02-24 17:32:43 PST 
Created attachment 391615 [details]
Patch
Comment 11 Said Abou-Hallawa 2020-02-24 17:59:36 PST 
Created attachment 391616 [details]
Patch
Comment 12 Darin Adler 2020-02-24 22:08:16 PST 
Comment on attachment 391616 [details]
Patch

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

> Source/WebCore/platform/HostWindow.h:28
> +#include "ColorSpace.h"

Typically we can get away with a forward declaration of an enum, if it’s an enum class.

> Source/WebCore/platform/HostWindow.h:29
> +#include "RenderingMode.h"

Ditto.
Comment 13 Said Abou-Hallawa 2020-02-25 08:56:23 PST 
Created attachment 391656 [details]
Patch
Comment 14 Said Abou-Hallawa 2020-02-25 09:30:40 PST 
Created attachment 391662 [details]
Patch
Comment 15 WebKit Commit Bot 2020-02-25 10:49:07 PST 
Comment on attachment 391662 [details]
Patch

Clearing flags on attachment: 391662

Committed r257363: <https://trac.webkit.org/changeset/257363>
Comment 16 WebKit Commit Bot 2020-02-25 10:49:09 PST 
All reviewed patches have been landed.  Closing bug.
Comment 17 Radar WebKit Bug Importer 2020-02-25 10:50:17 PST 
<rdar://problem/59770658>
Comment 18 Said Abou-Hallawa 2020-02-25 17:47:44 PST 
*** Bug 204955 has been marked as a duplicate of this bug. ***
Comment 19 Said Abou-Hallawa 2020-02-25 17:51:29 PST 
*** Bug 207198 has been marked as a duplicate of this bug. ***
Comment 20 Said Abou-Hallawa 2020-02-26 17:22:56 PST 
Comment on attachment 391438 [details]
Patch

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

>>> Source/WebCore/ChangeLog:12
>>> +        is to interact with RemoteImageBuffer seamlessly in the caller side.
>> 
>> What is the added value of this additional interface over doing this via HostWindow?
> 
> To be more specific, the alternative I imagine is adding the createImageBuffer() virtual functions to HostWindow, and having Chrome forward those to createImageBuffer() virtual functions on the ChromeClient.

Why adding createImageBuffer() to HostWindow and not adding the RenderingBackend to PlatformStrategies?
Comment 21 Sam Weinig 2020-02-27 12:13:19 PST 
(In reply to Said Abou-Hallawa from comment #20)
> Comment on attachment 391438 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=391438&action=review
> 
> >>> Source/WebCore/ChangeLog:12
> >>> +        is to interact with RemoteImageBuffer seamlessly in the caller side.
> >> 
> >> What is the added value of this additional interface over doing this via HostWindow?
> > 
> > To be more specific, the alternative I imagine is adding the createImageBuffer() virtual functions to HostWindow, and having Chrome forward those to createImageBuffer() virtual functions on the ChromeClient.
> 
> Why adding createImageBuffer() to HostWindow and not adding the
> RenderingBackend to PlatformStrategies?

PlatformStrategies have not been a good approach due their creation of a singleton and reliance on processes not wanting multiple strategies at once. Use of the per-page HostWindow avoids that problem.