WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alexandre Elias
Comment 1
2012-10-19 12:39:15 PDT
Created
attachment 169675
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug