Bug 98833 - [chromium] Add entrypoints to set frame contents and recycle resources in WebDelegatedRendererLayer
Summary: [chromium] Add entrypoints to set frame contents and recycle resources in Web...
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-10-09 16:28 PDT by Antoine Labour
Modified: 2012-10-10 19:33 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.82 KB, patch)
2012-10-09 16:31 PDT, Antoine Labour
no flags Details | Formatted Diff | Diff
Patch (3.90 KB, patch)
2012-10-09 17:51 PDT, Antoine Labour
no flags Details | Formatted Diff | Diff
Patch (3.90 KB, patch)
2012-10-10 15:45 PDT, 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-10-09 16:28:42 PDT
[chromium] Add entrypoints to set frame contents and recycle resources in WebDelegatedRendererLayer
Comment 1 Antoine Labour 2012-10-09 16:31:17 PDT
Created attachment 167874 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-09 16:35:33 PDT
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 Alexandre Elias 2012-10-09 17:02:34 PDT
Comment on attachment 167874 [details]
Patch

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

> Source/Platform/chromium/public/WebDelegatedRendererLayer.h:31
> +namespace cc {

We can't have a "namespace cc" here.  It looks like TransferableResourceList will have to be declared in WebKit namespace and put in Source/Platform/chromium/public/ .
Comment 4 Antoine Labour 2012-10-09 17:13:33 PDT
We should find a way to do this.

If you pull the thread, you'll end up moving all of cc back into webkit.
We need a way to have opaque data flow untouched through the webkit API...

Also, it would become a nightmare to have TransferedResource in a different repository as we add more resource types and transport mechanism (think shared memory etc.)
Comment 5 Adam Barth 2012-10-09 17:14:52 PDT
Comment on attachment 167874 [details]
Patch

Indeed.
Comment 6 Adam Barth 2012-10-09 17:16:19 PDT
> If you pull the thread, you'll end up moving all of cc back into webkit.
> We need a way to have opaque data flow untouched through the webkit API...

Typically we do this by exposing a type and then having the embedder subclass that type.  See ExtraData in a number of the networking APIs.
Comment 7 Antoine Labour 2012-10-09 17:51:43 PDT
Created attachment 167884 [details]
Patch
Comment 8 Antoine Labour 2012-10-09 17:52:48 PDT
FTR, I think it's sad to lose static typing for perceived purity of the API (who are we kidding really). But whatever you say.
Comment 9 Adam Barth 2012-10-09 18:01:46 PDT
(In reply to comment #8)
> FTR, I think it's sad to lose static typing for perceived purity of the API (who are we kidding really). But whatever you say.

jamesr might have a better idea, but that's the pattern we've used elsewhere.  It does have the unfortunate issue of making the embedder static_cast, but at least it has no runtime cost.
Comment 10 WebKit Review Bot 2012-10-09 18:26:53 PDT
Comment on attachment 167884 [details]
Patch

Attachment 167884 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14223936
Comment 11 Peter Beverloo (cr-android ews) 2012-10-09 18:49:28 PDT
Comment on attachment 167884 [details]
Patch

Attachment 167884 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14252009
Comment 12 Antoine Labour 2012-10-10 15:45:04 PDT
Created attachment 168080 [details]
Patch
Comment 13 Antoine Labour 2012-10-10 15:46:20 PDT
patch with non-pure-virtual methods to allow landing. Will remove in follow up when chrome side has landed.
Comment 14 WebKit Review Bot 2012-10-10 19:33:47 PDT
Comment on attachment 168080 [details]
Patch

Clearing flags on attachment: 168080

Committed r130998: <http://trac.webkit.org/changeset/130998>
Comment 15 WebKit Review Bot 2012-10-10 19:33:51 PDT
All reviewed patches have been landed.  Closing bug.