Bug 207134 – Make HostWindow be the creator of the remote ImageBuffer

• Component: Canvas
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

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.

[Said Abou-Hallawa]

Created attachment 389540 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389570 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389579 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389584 [details]
Patch  for review

[Said Abou-Hallawa]

Created attachment 391435 [details]
Patch

[Said Abou-Hallawa]

Created attachment 391438 [details]
Patch

[Sam Weinig]

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?

[Sam Weinig]

(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.

[Said Abou-Hallawa]

Created attachment 391593 [details]
Patch

[Said Abou-Hallawa]

Created attachment 391615 [details]
Patch

[Said Abou-Hallawa]

Created attachment 391616 [details]
Patch

[Darin Adler]

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.

[Said Abou-Hallawa]

Created attachment 391656 [details]
Patch

[Said Abou-Hallawa]

Created attachment 391662 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 391662 [details]
Patch

Clearing flags on attachment: 391662

Committed r257363: <https://trac.webkit.org/changeset/257363>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59770658>

[Said Abou-Hallawa]

*** Bug 204955 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

*** Bug 207198 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

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?

[Sam Weinig]

(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.