[Webkit-unassigned] [Bug 57418] [Qt] [Symbian] GraphicsLayer: support plugins on symbian

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Viatcheslav Ostapenko <ostapenko.viatcheslav at nokia.com>  2011-05-24 07:49:12 PST ---
(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #11)
> > > (From update of attachment 91306 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=91306&action=review
> > > 
> > > > Source/WebCore/plugins/PluginView.h:432
> > > > +#if USE(ACCELERATED_COMPOSITING) && (defined(XP_UNIX) || OS(SYMBIAN))
> > > 
> > > Why the unix here?
> > 
> > Because this patch adds AC plugin layer implementation for symbian to already existing unix implementation. Mac and windows still do not have plugin AC layer implementation for Qt. I just modified existing ifdef (added SYMBIAN).
> > It's a bit unclear, because I had to break previous #ifdef at line 429 (paintUsingXPixmap is not needed on symbian and other platforms).
> 
> Sounds like we might want to come up with a new define then.  Otherwise we're going to have to add defined(WIN) || everywhere when we add win, and then again fo mac, etc.

After mac and win AC plugin layers are implemented this condition can be removed because all 4 platforms will be supported.

> > > 
> > > > Source/WebCore/plugins/symbian/PluginViewSymbian.cpp:472
> > > > +        if (m_parentFrame->page()->chrome()->client()->allowsAcceleratedCompositing()
> > > > +            && m_parentFrame->page()->settings()
> > > > +            && m_parentFrame->page()->settings()->acceleratedCompositingEnabled()) {
> > > 
> > > I wonder if this shouldn't be a helper function.
> > 
> > What exactly? Big condition or creating of plugin layer? Or both together? ;)
> > In any case, I'd prefer to leave it as it is to make it similar to unix implementation to make later merging of other changes simpler.
> 
> Well, consider this example:
> 
> if (shouldUseAccelleratedComposititing(m_parentFrame))
> 
> Such is very clear at the call site what it does.  And the actual implementation of the check can use local variables and early return as well.
> 
> I'm not saying that splitting ifs off into functions is always a good choice.  But in this one, I think it might be, given all the m_parentFrame->paget()->chrome copy paste.

Yes, I've got it. Should I make it for linux part also? I'd like to keep it similar.

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