Bug 90494 – Implement device metrics for blackberry

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Hanna]

enabling remotely changing the size of the viewport on device

https://przilla.ott.qnx.com/bugzilla/show_bug.cgi?id=149876

[Hanna]

Created attachment 150792 [details]
Patch

[Konrad Piascik]

Comment on attachment 150792 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150792&action=review

> Source/WebKit/blackberry/WebCoreSupport/InspectorClientBlackBerry.h:65
> +    OwnPtr<DeviceMetricsSupport> m_deviceMetrics;

New member variables should be declared last and initialized in the order they are declared in the header.  You're initializing in the correct order in your constructor but not declaring in the right order.

> Source/WebKit/blackberry/WebCoreSupport/InspectorDeviceMetrics.h:1
> +/*

This is a small class that's entirely declared in the header.  This is not good and should be moved into the InspectorClientBlackberry class we don't need a separate class for this functionality.

[Hanna]

Created attachment 150808 [details]
Patch

[Konrad Piascik]

Comment on attachment 150808 [details]
Patch

Looks good.

[Rob Buis]

Comment on attachment 150808 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150808&action=review

LGTM.

> Source/WebKit/blackberry/WebCoreSupport/InspectorClientBlackBerry.cpp:93
> +void InspectorClientBlackBerry::overrideDeviceMetrics(int width, int height, float fontScaleFactor, bool fitWindow)

fitWindow not used, so you can remove the name.

[Hanna]

(In reply to comment #5)
> (From update of attachment 150808 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150808&action=review
> 
> LGTM.
> 
> > Source/WebKit/blackberry/WebCoreSupport/InspectorClientBlackBerry.cpp:93
> > +void InspectorClientBlackBerry::overrideDeviceMetrics(int width, int height, float fontScaleFactor, bool fitWindow)
> 
> fitWindow not used, so you can remove the name.

This function is called from the front end, I do not know if I remove a parameter it would still recognize the function

[Hanna]

Created attachment 150829 [details]
Patch

[Rob Buis]

Comment on attachment 150829 [details]
Patch

Looks good.

[WebKit Review Bot]

Comment on attachment 150829 [details]
Patch

Clearing flags on attachment: 150829

Committed r121872: <http://trac.webkit.org/changeset/121872>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Antonio Gomes]

Comment on attachment 150829 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=150829&action=review

> Source/WebKit/blackberry/WebCoreSupport/InspectorClientBlackBerry.cpp:88
> +bool InspectorClientBlackBerry::canOverrideDeviceMetrics()

not const?

> Source/WebKit/blackberry/WebCoreSupport/InspectorClientBlackBerry.cpp:100
> +bool InspectorClientBlackBerry::supportsFrameInstrumentation()

ditto