Bug 131184 – Web Inspector: remove unused metrics and commands from the Timeline agent

• Component: Web Inspector
• Status: RESOLVED FIXED

[Brian Burg]

This is a nontrivial amount of data (unsigned long per record) and we don't do anything with it. Any use of it would probably part of a more comprehensive feature.

[Radar WebKit Bug Importer]

<rdar://problem/16517540>

[Timothy Hatcher]

Sounds good.

[Brian Burg]

This includes:

 - used heap size for every timeline record
 - begin/cancel frame messages, which were broken
 - runtime querying of feature support for features nobody implements

[Brian Burg]

Created attachment 228662 [details]
the patch

[Timothy Hatcher]

Comment on attachment 228662 [details]
the patch

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

> Source/WebCore/inspector/InspectorController.h:129
>      void willComposite();

This too isn't used.

> Source/WebCore/inspector/InspectorController.h:130
>      void didComposite();

Ditto.

> Source/WebCore/inspector/protocol/Timeline.json:-19
> -                { "name": "usedHeapSize", "type": "integer", "optional": true, "description": "Current size of JS heap." }

These should be removed from iOS 7 protocol files too.

[Timothy Hatcher]

I do want to track rendered frames, but we can add it back at that time (likely with a better design.)

[Brian Burg]

(In reply to comment #6)
> I do want to track rendered frames, but we can add it back at that time (likely with a better design.)

Right. I kept the enum value in there because it would be useful to have this functionality. Just none of the existing plumbing works.

[Brian Burg]

(In reply to comment #5)
> (From update of attachment 228662 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228662&action=review
> 
> > Source/WebCore/inspector/InspectorController.h:129
> >      void willComposite();
> 
> This too isn't used.
> 
> > Source/WebCore/inspector/InspectorController.h:130
> >      void didComposite();
> 
> Ditto.
> 
> > Source/WebCore/inspector/protocol/Timeline.json:-19
> > -                { "name": "usedHeapSize", "type": "integer", "optional": true, "description": "Current size of JS heap." }
> 
> These should be removed from iOS 7 protocol files too.

What about counters and other heap- and frame-related  stuff in the protocol file that has never been used in the current inspector UI?

[Brian Burg]

Created attachment 228679 [details]
the patch, now with less didComposite

[Timothy Hatcher]

(In reply to comment #8)
> (In reply to comment #5)
> > (From update of attachment 228662 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=228662&action=review
> > 
> > > Source/WebCore/inspector/InspectorController.h:129
> > >      void willComposite();
> > 
> > This too isn't used.
> > 
> > > Source/WebCore/inspector/InspectorController.h:130
> > >      void didComposite();
> > 
> > Ditto.
> > 
> > > Source/WebCore/inspector/protocol/Timeline.json:-19
> > > -                { "name": "usedHeapSize", "type": "integer", "optional": true, "description": "Current size of JS heap." }
> > 
> > These should be removed from iOS 7 protocol files too.
> 
> What about counters and other heap- and frame-related  stuff in the protocol file that has never been used in the current inspector UI?

Yes, anything we have removed from TOT should be removed from the old protocols versions too. That way if we want to use those names in the future, we can feature check the protocol and only get the new version.

We likely have missed some thing during removal. I pulled some things out the other day that I missed when cleaning up the profiler.

[Timothy Hatcher]

(In reply to comment #7)
> (In reply to comment #6)
> > I do want to track rendered frames, but we can add it back at that time (likely with a better design.)
> 
> Right. I kept the enum value in there because it would be useful to have this functionality. Just none of the existing plumbing works.

We probably should remove the enums too — for completeness. Easy to add back later when needed.

[Timothy Hatcher]

Comment on attachment 228679 [details]
the patch, now with less didComposite

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

> Source/WebInspectorUI/UserInterface/Protocol/Legacy/7.0/InspectorWebBackendCommands.js:-238
> -InspectorBackend.registerCommand("Timeline.supportsFrameInstrumentation", [], ["result"]);
> -InspectorBackend.registerCommand("Timeline.canMonitorMainThread", [], ["result"]);

This should be removed from the .json file in WebInspectorUI/Versions too.

[Brian Burg]

Created attachment 228687 [details]
the patch, with old protocols fixed

[Brian Burg]

Committed r166846: <http://trac.webkit.org/changeset/166846>