Bug 126149

Summary: Web Inspector: Remove canOverrideDeviceMetrics and setDeviceMetricsOverride from protocol
Product: WebKit Reporter: Seokju Kwon <seokju>
Component: Web InspectorAssignee: Seokju Kwon <seokju>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 126236    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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.