Bug 126149 – Web Inspector: Remove canOverrideDeviceMetrics and setDeviceMetricsOverride from protocol

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Seokju Kwon]

These are not used anywhere in WebInspectorUI.
So, it removes unused Protocols and APIs.

I will remove other 'device metrics' codes as well.

[Radar WebKit Bug Importer]

<rdar://problem/15717425>

[Seokju Kwon]

Created attachment 219893 [details]
Patch

[Timothy Hatcher]

Comment on attachment 219893 [details]
Patch

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

> Source/WebCore/inspector/InspectorPageAgent.cpp:695
>  bool InspectorPageAgent::deviceMetricsChanged(int width, int height, double fontScaleFactor, bool fitWindow)

You can remove this function too and updateViewMetrics. I am sure there are other functions and code that could be removed in WebKit be removing this API.

[Seokju Kwon]

(In reply to comment #3)
> (From update of attachment 219893 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219893&action=review
> 
> > Source/WebCore/inspector/InspectorPageAgent.cpp:695
> >  bool InspectorPageAgent::deviceMetricsChanged(int width, int height, double fontScaleFactor, bool fitWindow)
> 
> You can remove this function too and updateViewMetrics. I am sure there are other functions and code that could be removed in WebKit be removing this API.

Sure. It seems like there are so many unused codes left!.
I file a master bug : Bug 126236, I will remove them on other bugs.

[Seokju Kwon]

Created attachment 220012 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 220012 [details]
Patch

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

> Source/WebCore/inspector/protocol/Page.json:-202
> -            "name": "canOverrideDeviceMetrics",
> -            "description": "Checks whether <code>setDeviceMetricsOverride</code> can be invoked.",
> -            "returns": [
> -                { "name": "result", "type": "boolean", "description": "If true, <code>setDeviceMetricsOverride</code> can safely be invoked on the agent." }
> -            ]
> -        },
> -        {
> -            "name": "setDeviceMetricsOverride",

You could also remove these from the legacy backend files:

    Source/WebInspectorUI/Versions/Inspector-iOS-6.0.json
    Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json

Though the backends supported it, we don't use it in the frontend. If we ever want to introduce something with the same name, then we would conflict with these old versions.

If you do remove them from these legacy files, regenerate the InspectorBackendCommands files by running Source/WebInspectorUI/Tools/update-InspectorBackendCommands.rb or something like that.

[Seokju Kwon]

(In reply to comment #6)
> (From update of attachment 220012 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220012&action=review
> 
> > Source/WebCore/inspector/protocol/Page.json:-202
> > -            "name": "canOverrideDeviceMetrics",
> > -            "description": "Checks whether <code>setDeviceMetricsOverride</code> can be invoked.",
> > -            "returns": [
> > -                { "name": "result", "type": "boolean", "description": "If true, <code>setDeviceMetricsOverride</code> can safely be invoked on the agent." }
> > -            ]
> > -        },
> > -        {
> > -            "name": "setDeviceMetricsOverride",
> 
> You could also remove these from the legacy backend files:
> 
>     Source/WebInspectorUI/Versions/Inspector-iOS-6.0.json
>     Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json
> 
> Though the backends supported it, we don't use it in the frontend. If we ever want to introduce something with the same name, then we would conflict with these old versions.
> 
> If you do remove them from these legacy files, regenerate the InspectorBackendCommands files by running Source/WebInspectorUI/Tools/update-InspectorBackendCommands.rb or something like that.

Sure. I will remove them from the legacy backend files as well. 
Patch is coming. :)

[Seokju Kwon]

Created attachment 220095 [details]
Patch

[Seokju Kwon]

@Joepeck
Could you please take another look at this?

[Joseph Pecoraro]

Comment on attachment 220095 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 220095 [details]
Patch

Clearing flags on attachment: 220095

Committed r161402: <http://trac.webkit.org/changeset/161402>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.