[Webkit-unassigned] [Bug 99660] checkbox to toggle FPS counter in the inspector's settings
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Nov 9 06:10:43 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=99660
Pavel Feldman <pfeldman at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #173158|review? |review+
Flag| |
--- Comment #42 from Pavel Feldman <pfeldman at chromium.org> 2012-11-09 06:12:19 PST ---
(From update of attachment 173158)
View in context: https://bugs.webkit.org/attachment.cgi?id=173158&action=review
Please fix nits prior to landing.
> Source/WebCore/inspector/Inspector.json:351
> + { "name": "result", "type": "boolean", "description": "True for showing the FPS counter" }
I do understand that you copied the one above, but we should not call input parameter "result", it should be "enabled" or "show".
>> Source/WebCore/inspector/InspectorClient.h:83
>> + virtual bool canShowFPSCounter() { return false; }
>
> Made false the default.
I would follow the pattern above and reverse the order (canShow, then show). You should also move these above to after overrideDeviceMetrics
>> Source/WebCore/inspector/InspectorPageAgent.h:113
>> + virtual void canShowFPSCounter(ErrorString*, bool*);
>
> Dropped the outParam.
Also reverse order.
> Source/WebKit/chromium/src/InspectorClientImpl.h:86
> + virtual bool canShowFPSCounter();
reverse
> Source/WebKit/chromium/src/WebViewImpl.cpp:4030
> +void WebViewImpl::loadFontAtlasIfNecessary()
The order of declaration should match the order of definition.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list