[webkit-reviews] review requested: [Bug 88384] [chromium] Certain settings in CCSettings could be global : [Attachment 146086] Patch

[bugzilla-daemon at webkit.org]

vollick at chromium.org has asked	for review:
Bug 88384: [chromium] Certain settings in CCSettings could be global
https://bugs.webkit.org/show_bug.cgi?id=88384

Attachment 146086: Patch
https://bugs.webkit.org/attachment.cgi?id=146086&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #4)
> (From update of attachment 145911 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=145911&action=review
>
> > Source/WebCore/platform/graphics/chromium/cc/CCSettings.cpp:53
> > +static const IntSize defaultTileSizeDefault = IntSize(256, 256);
> > +static IntSize s_defaultTileSize = defaultTileSizeDefault;
>
> This smells like a static initializer - is it?  If so that's a no-no. 
Perhaps we could use a lazy singleton pattern instead?
*blush* Fixed.
>
> > Source/WebKit/chromium/src/WebViewImpl.cpp:3483
> > +	    
CCSettings::setAcceleratedPaintingEnabled(page()->settings()->acceleratedDrawin
gEnabled());
>
> I think this is (slightly) wrong - we shouldn't have every view clobbering
the global settings.  Since this is exposed through WebKit API the embedder
(chromium) should be making these decisions based on command line flags or
whatnot before we even get to here.
Fixed.