Bug 104875 - [chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTextureId/setBackingIOSurfaceId
Summary: [chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTexture...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Labour
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-12 21:00 PST by Antoine Labour
Modified: 2012-12-13 14:20 PST (History)
7 users (show)

See Also:


Attachments
Patch (7.90 KB, patch)
2012-12-12 21:02 PST, Antoine Labour
no flags Details | Formatted Diff | Diff
Patch (10.45 KB, patch)
2012-12-13 11:59 PST, Antoine Labour
no flags Details | Formatted Diff | Diff
Patch for landing (10.44 KB, patch)
2012-12-13 13:44 PST, Antoine Labour
no flags Details | Formatted Diff | Diff
Patch for landing (11.13 KB, patch)
2012-12-13 14:19 PST, Antoine Labour
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Labour 2012-12-12 21:00:41 PST
[chromium] Add WebPluginContainer::setWebLayer to superseed setBackingTextureId/setBackingIOSurfaceId
Comment 1 Antoine Labour 2012-12-12 21:02:55 PST
Created attachment 179191 [details]
Patch
Comment 2 WebKit Review Bot 2012-12-12 21:06:10 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Antoine Labour 2012-12-12 21:07:15 PST
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.
Comment 4 James Robinson 2012-12-13 10:56:06 PST
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
Comment 5 Antoine Labour 2012-12-13 11:59:42 PST
Created attachment 179311 [details]
Patch
Comment 6 Antoine Labour 2012-12-13 12:00:40 PST
(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.
Comment 7 James Robinson 2012-12-13 12:43:57 PST
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
Comment 8 Antoine Labour 2012-12-13 13:44:04 PST
Created attachment 179322 [details]
Patch for landing
Comment 9 Antoine Labour 2012-12-13 13:46:25 PST
(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.
Comment 10 WebKit Review Bot 2012-12-13 14:17:29 PST
Comment on attachment 179322 [details]
Patch for landing

Clearing flags on attachment: 179322

Committed r137653: <http://trac.webkit.org/changeset/137653>
Comment 11 WebKit Review Bot 2012-12-13 14:17:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Antoine Labour 2012-12-13 14:19:31 PST
Reopening to attach new patch.
Comment 13 Antoine Labour 2012-12-13 14:19:33 PST
Created attachment 179332 [details]
Patch for landing