RESOLVED FIXED 99863
[chromium] API to pass impl thread via WebLayerTreeView
https://bugs.webkit.org/show_bug.cgi?id=99863
Summary [chromium] API to pass impl thread via WebLayerTreeView
Alexandre Elias
Reported 2012-10-19 12:35:32 PDT
[chromium] API to pass impl thread via WebLayerTreeView
Attachments
Patch (10.47 KB, patch)
2012-10-19 12:39 PDT, Alexandre Elias
no flags
Patch (10.42 KB, patch)
2012-10-19 13:10 PDT, Alexandre Elias
no flags
Patch (9.41 KB, patch)
2012-10-19 13:11 PDT, Alexandre Elias
no flags
Alexandre Elias
Comment 1 2012-10-19 12:39:15 PDT
WebKit Review Bot
Comment 2 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.
Alexandre Elias
Comment 3 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.
James Robinson
Comment 4 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
Alexandre Elias
Comment 5 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.
James Robinson
Comment 6 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.
Alexandre Elias
Comment 7 2012-10-19 13:10:25 PDT
Created attachment 169681 [details] Patch Don't add a new WebLayerTreeView::create
Alexandre Elias
Comment 8 2012-10-19 13:11:50 PDT
Created attachment 169682 [details] Patch Remove accidentally added PageWidgetDelegate change
Alexandre Elias
Comment 9 2012-10-19 13:12:37 PDT
OK, makes sense. Please r+ again.
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2012-10-19 14:30:30 PDT
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 12 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.
Note You need to log in before you can comment on or make changes to this bug.