Bug 104875 – [chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTextureId/setBackingIOSurfaceId

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Antoine Labour]

[chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTextureId/setBackingIOSurfaceId

[Antoine Labour]

Created attachment 179191 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Antoine Labour]

The intent is to remove setBackingTextureId, setBackingIOSurfaceId, commitBackingTexture and setOpaque once the embedder creates and updates the layer.
We should be able to remove WebIOSurfaceLayer after that, and it will be easier to remove the setTextureId paths in TextureLayer (favoring the TextureLayerClient path), as well as adding support for DelegatedRendererLayer eventually.

[James Robinson]

Comment on attachment 179191 [details]
Patch

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

Awesome! I wanna do this for video too, if I ever get 'round to it.

Could you update Tools/DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp ? it's supposed to provide layout test coverage for WebPluginContainer.. compositing (it's currently marked IMAGE PASS on linux since it went flaky :( )

> Source/WebKit/chromium/ChangeLog:3
> +        [chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTextureId/setBackingIOSurfaceId

typo superseed -> supersede

[Antoine Labour]

Created attachment 179311 [details]
Patch

[Antoine Labour]

(In reply to comment #4)
> (From update of attachment 179191 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179191&action=review
> 
> Awesome! I wanna do this for video too, if I ever get 'round to it.
> 
> Could you update Tools/DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp ? it's supposed to provide layout test coverage for WebPluginContainer.. compositing (it's currently marked IMAGE PASS on linux since it went flaky :( )

Good idea, done.

> 
> > Source/WebKit/chromium/ChangeLog:3
> > +        [chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTextureId/setBackingIOSurfaceId
> 
> typo superseed -> supersede

Done.

[James Robinson]

Comment on attachment 179311 [details]
Patch

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

R=me.  If you don't mind, run platform/chromium/compositing/plugins/ with --verbose to make sure they still path (since they're marked IMAGE PASS the cr-linux EWS won't tell us).

> Tools/DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp:304
> +    m_layer = WTF::adoptPtr(webKitPlatformSupport()->compositorSupport()->createExternalTextureLayer(this));

nit: don't need the WTF::, just adoptPtr() should work

[Antoine Labour]

Created attachment 179322 [details]
Patch for landing

[Antoine Labour]

(In reply to comment #7)
> (From update of attachment 179311 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179311&action=review
> 
> R=me.  If you don't mind, run platform/chromium/compositing/plugins/ with --verbose to make sure they still path (since they're marked IMAGE PASS the cr-linux EWS won't tell us).

Yep, they pass (I verified that if I don't setWebLayer they fail).

> 
> > Tools/DumpRenderTree/chromium/TestRunner/src/WebTestPlugin.cpp:304
> > +    m_layer = WTF::adoptPtr(webKitPlatformSupport()->compositorSupport()->createExternalTextureLayer(this));
> 
> nit: don't need the WTF::, just adoptPtr() should work

Yeah, my bad. If you just include OwnPtr.h it's in WTF:: but if you include PassOwnPtr.h it gets promoted. I hadn't done the latter originally hence needed to add this. Fixed in the latest patch.

[WebKit Review Bot]

Comment on attachment 179322 [details]
Patch for landing

Clearing flags on attachment: 179322

Committed r137653: <http://trac.webkit.org/changeset/137653>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Antoine Labour]

Reopening to attach new patch.

[Antoine Labour]

Created attachment 179332 [details]
Patch for landing