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

[bugzilla-daemon at webkit.org]

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





--- Comment #28 from Andrey Kosyakov <caseq at chromium.org>  2012-11-06 06:34:43 PST ---
(From update of attachment 171500)
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?

> 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?

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

I suggest we drop this.

> 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;

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