Bug 99863 - [chromium] API to pass impl thread via WebLayerTreeView
Summary: [chromium] API to pass impl thread via WebLayerTreeView
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: Alexandre Elias
URL:
Keywords:
Depends on: 99891
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-19 12:35 PDT by Alexandre Elias
Modified: 2013-04-15 08:52 PDT (History)
7 users (show)

See Also:


Attachments
Patch (10.47 KB, patch)
2012-10-19 12:39 PDT, Alexandre Elias
no flags Details | Formatted Diff | Diff
Patch (10.42 KB, patch)
2012-10-19 13:10 PDT, Alexandre Elias
no flags Details | Formatted Diff | Diff
Patch (9.41 KB, patch)
2012-10-19 13:11 PDT, Alexandre Elias
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandre Elias 2012-10-19 12:35:32 PDT
[chromium] API to pass impl thread via WebLayerTreeView
Comment 1 Alexandre Elias 2012-10-19 12:39:15 PDT
Created attachment 169675 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-19 12:40:43 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-19 12:41:23 PDT
Hi James, this is the WebKit side of the change, carefully designed to land first without breaking anything.  After this, I'll make a big Chromium patch switching everything over, then go back to WebKit to delete the cruft.
Comment 4 James Robinson 2012-10-19 12:53:51 PDT
Comment on attachment 169675 [details]
Patch

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

This looks great.

> Source/Platform/chromium/public/WebLayerTreeView.h:75
> +    WEBKIT_EXPORT static WebLayerTreeView* create(WebLayerTreeViewClient*, const WebLayer& root, const Settings&, WebThread* implThread);
> +    // FIXME(aelias): Delete this version when the above is implemented.
>      WEBKIT_EXPORT static WebLayerTreeView* create(WebLayerTreeViewClient*, const WebLayer& root, const Settings&);

Actually both of these ::create()s are dead - in fact all of the ::create() functions for things that hang off of WebCompositorSupport are currently unimplemented and uncalled.  I still haven't deleted them because I suck :(.  Don't worry about patching this file
Comment 5 Alexandre Elias 2012-10-19 12:57:18 PDT
I noticed that, but I actually intend to use the new create() I added.  The reason is that I want to avoid calling compositorSupport() from the browser process.  As part of my Chromium-side change, I'll change the CompositorSupport implementation to use that create() as well to avoid the duplicate code.
Comment 6 James Robinson 2012-10-19 13:03:44 PDT
I don't think that's a good idea - I'm about to delete the whole ::create() family.  There's no way to implement them from inside WebKit.dll and no way to provide exported functions from outside WebKit.dll.  This way lies madness, I tried pretty hard.

What you should do is get an instance of a WebCompositorSupport implementation in your browser code from somewhere other than WebKit::Platform::current() and use the factory functions on it.
Comment 7 Alexandre Elias 2012-10-19 13:10:25 PDT
Created attachment 169681 [details]
Patch

Don't add a new WebLayerTreeView::create
Comment 8 Alexandre Elias 2012-10-19 13:11:50 PDT
Created attachment 169682 [details]
Patch

Remove accidentally added PageWidgetDelegate change
Comment 9 Alexandre Elias 2012-10-19 13:12:37 PDT
OK, makes sense.  Please r+ again.
Comment 10 WebKit Review Bot 2012-10-19 14:30:26 PDT
Comment on attachment 169682 [details]
Patch

Clearing flags on attachment: 169682

Committed r131944: <http://trac.webkit.org/changeset/131944>
Comment 11 WebKit Review Bot 2012-10-19 14:30:30 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 James Robinson 2012-10-19 14:46:43 PDT
On further thought, I actually think this is going in the wrong direction - we can't really support different thread modes on different views.