[Webkit-unassigned] [Bug 99660] toggle FPS counter option

[bugzilla-daemon at webkit.org]

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





--- Comment #31 from egraether at chromium.org  2012-11-06 12:18:12 PST ---
(In reply to comment #28)
> (From update of attachment 171500 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171500&action=review
> 
> > Source/WebCore/inspector/Inspector.json:342
> > +                "name": "syncShowFPSCounter",
> > +                "description": "Synchronizes the showFPSCounter setting with the backend",
> 
> I think we're going to get rid of the about:flags flag for the FPS counter eventually, so let's not expose the complexity of syncing different values for the option into the inspector protocol. How about just ORing the values of the command line flag with the value of inspector option?

I removed the syncing.

> > Source/WebCore/inspector/Inspector.json:444
> > +                "name": "isForceCompositingModeEnabled",
> 
> I think the conditions of whether we can show FPS counter may vary per embedder, so hard-coding the limitations of chromium implementation into front-end and adding protocol methods to support this does not seem fair. Can we instead expose the capability, i.e. canShowFPSCounter() and ignore the fact that chromium can only do this while accelerated compositing is on, perhaps just add a warning text to the option in the UI?

I did the renaming and put a comment about this in the new patch.

> > Source/WebCore/inspector/InspectorClient.h:82
> > +    virtual bool isShowFPSCounterFlagSet() { return false; }
> 
> I suggest we drop this.

dropped, it was used in the syncing.

> > Source/WebKit/chromium/src/WebViewImpl.cpp:3988
> > +void WebViewImpl::loadFontAtlas()
> 
> nit: loadFontAtlasIfNecessary()
> 
> > Source/WebKit/chromium/src/WebViewImpl.cpp:3991
> > +    if (!m_isFontAtlasLoaded) {
> 
> nit: we prefer early bail-outs, i.e.
> if (m_isFontAtlasLoaded)
>     return;

both nits implemented

please have a look at the new patch, thanks

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