[Webkit-unassigned] [Bug 69048] Separate compositor platform support from webkit's platform support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69048





--- Comment #13 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2011-09-29 11:02:42 PST ---
(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 109116 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=109116&action=review
> > 
> > > Source/WebKit/chromium/public/WebKit.h:58
> > > +WEBKIT_EXPORT void initializeWithoutV8AndCompositor(WebKitPlatformSupport*);
> > 
> > lol, when we just recently added initializeWithoutV8, there was a debate about
> > whether or not we should just add flags to initialize().  I argued that since
> > we only had the use case of running without V8, that we should just go for the
> > simpler form.
> > 
> > I'm pretty sure that initializeWithoutV8AndCompositor is getting out of hand
> > though ;-)
> > 
> > This might be better:
> > 
> >   struct WebKitInitializationOptions {
> >       bool initializeV8;
> >       bool initializeCompositor;
> >       // ... default ctor sets them to true ...
> >   };
> >   WEBKIT_EXPORT void initializeWithOptions(WebKitPlatformSupport*, const WebKitInitializationOptions&);
> 
> That works for me.
> 
> > > Source/WebKit/chromium/src/WebKit.cpp:61
> > > +class WebKitCompositorSupport : public WebCore::CompositorSupportDelegate {
> > 
> > I'm guessing there is some next patch that will make it possible for there to be
> > a CompositorSupportDelegate that doesn't just thunk to WebKitPlatformSupport?
> 
> Right, it's part of the compositor wenkit API, I'll upload it as soon as I figure out how to get webkit-patch to use the right base commit.
> 
> > Can you explain how that might look?
> 
> Nothing fancy, a separate compositor initialization function that takes a WebCompositorPlatformSupport that looks just like CompositorSupportDelegate.
> 
> > 
> > Also, it seems like the 4 functions on this interface could be defined as safe to
> > call from any thread in WebKit.  I'm not quite sure why they need to be factored
> > out like this.  It may already be the case that they are safe to call from background
> > threads.  I think webworkers can query the current time, and the trace and histogram
> > systems are threadsafe.  Am I missing something?
> 
> There's a few reasons:
> 1- the functions themselves may be thread-safe (I'd have to check), but the way to get to them doesn't look like it is (s_webKitPlatformSupport isn't protected by any lock).

WebKit has to be initialized before s_webKitPlatformSupport is used, and we should ensure that any background threads are gone before WebKit::shutdown returns.


> 2- when we want to extract the compositor out of WebKit, we'll need those anyway. This is part of this effort.

I see.  It seems like we could have WebCompositorPlatformSupport be a getter on WebKitPlatformSupport.  That would be the much more natural way to handle this kind of thing.  If you want to get that inside WebKit::initialize(), then you can certainly avoid any concerns about thread safety.


> 3- it makes it harder to add dependencies to the rest of webcore, including calling functions that wouldn't be thread safe.

I'm not sure I understand this issue.  Can you explain more?


> Note: in the browser process, the compositor will be called from a non-webkit thread, if that makes things clearer...

I'm concerned about calling more WebKit code in the browser process.  We are actually trying to call less of it.  Maybe the extraction of this code into a separate library should happen sooner?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.