Bug 126149 - Web Inspector: Remove canOverrideDeviceMetrics and setDeviceMetricsOverride from protocol
Summary: Web Inspector: Remove canOverrideDeviceMetrics and setDeviceMetricsOverride f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Seokju Kwon
URL:
Keywords: InRadar
Depends on:
Blocks: 126236
  Show dependency treegraph
 
Reported: 2013-12-22 23:41 PST by Seokju Kwon
Modified: 2014-01-06 21:15 PST (History)
5 users (show)

See Also:


Attachments
Patch (11.55 KB, patch)
2013-12-22 23:45 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (14.02 KB, patch)
2013-12-25 16:20 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (22.37 KB, patch)
2013-12-29 16:38 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Seokju Kwon 2013-12-22 23:41:56 PST
These are not used anywhere in WebInspectorUI.
So, it removes unused Protocols and APIs.

I will remove other 'device metrics' codes as well.
Comment 1 Radar WebKit Bug Importer 2013-12-22 23:42:08 PST
<rdar://problem/15717425>
Comment 2 Seokju Kwon 2013-12-22 23:45:28 PST
Created attachment 219893 [details]
Patch
Comment 3 Timothy Hatcher 2013-12-23 14:59:32 PST
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.
Comment 4 Seokju Kwon 2013-12-25 15:44:56 PST
(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.
Comment 5 Seokju Kwon 2013-12-25 16:20:39 PST
Created attachment 220012 [details]
Patch
Comment 6 Joseph Pecoraro 2013-12-27 09:18:01 PST
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.
Comment 7 Seokju Kwon 2013-12-29 16:35:55 PST
(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. :)
Comment 8 Seokju Kwon 2013-12-29 16:38:16 PST
Created attachment 220095 [details]
Patch
Comment 9 Seokju Kwon 2014-01-06 16:51:26 PST
@Joepeck
Could you please take another look at this?
Comment 10 Joseph Pecoraro 2014-01-06 20:46:48 PST
Comment on attachment 220095 [details]
Patch

r=me
Comment 11 WebKit Commit Bot 2014-01-06 21:15:44 PST
Comment on attachment 220095 [details]
Patch

Clearing flags on attachment: 220095

Committed r161402: <http://trac.webkit.org/changeset/161402>
Comment 12 WebKit Commit Bot 2014-01-06 21:15:46 PST
All reviewed patches have been landed.  Closing bug.