Bug 140155

Summary: Web Inspector: Remove or use TimelineAgent Resource related event types
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: burg, commit-queue, graouts, japhet, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2015-01-06 16:07:00 PST
* SUMMARY
The TimelineAgent backend sends Network/Resource loading events that the frontend currently ignores:

    ScheduleResourceRequest
    ResourceSendRequest
    ResourceReceiveResponse
    ResourceReceivedData
    ResourceFinish

Instead of using these events, the frontend uses Network agent events (WebInspector.Resource and the TimestampsDidChange event).

Since these events are unused by the frontend they are just wasted messages. Either we should modify / adopt them, or just get rid of them.
Comment 1 Radar WebKit Bug Importer 2015-01-06 16:07:27 PST
<rdar://problem/19392397>
Comment 2 Timothy Hatcher 2015-01-06 16:15:36 PST
I vote for removal. The only advantage I see it ScheduleResourceRequest / ResourceSendRequest would potentially show interesting ancestor records for XHRs. We flatten the timeline records now, but we could use / show ancestor info if useful.

We use NetworkAgent because it is always reporting, not just when recording.
Comment 3 Joseph Pecoraro 2015-01-08 18:16:39 PST
Created attachment 244312 [details]
[PATCH] Proposed Fix
Comment 4 Timothy Hatcher 2015-01-08 19:49:47 PST
Comment on attachment 244312 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/inspector/protocol/Timeline.json:-9
> -            "enum": ["EventDispatch", "ScheduleStyleRecalculation", "RecalculateStyles", "InvalidateLayout", "Layout", "Paint", "ScrollLayer", "ResizeImage", "ParseHTML", "TimerInstall", "TimerRemove", "TimerFire", "EvaluateScript", "MarkLoad", "MarkDOMContent", "TimeStamp", "Time", "TimeEnd", "ScheduleResourceRequest", "ResourceSendRequest", "ResourceReceiveResponse", "ResourceReceivedData", "ResourceFinish", "XHRReadyStateChange", "XHRLoad", "FunctionCall", "ProbeSample", "ConsoleProfile", "GCEvent", "RequestAnimationFrame", "CancelAnimationFrame", "FireAnimationFrame", "WebSocketCreate", "WebSocketSendHandshakeRequest", "WebSocketReceiveHandshakeResponse", "WebSocketDestroy"],

Remove from Legacy protocol versions too?
Comment 5 Joseph Pecoraro 2015-01-09 13:44:50 PST
(In reply to comment #4)
> Comment on attachment 244312 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=244312&action=review
> 
> > Source/JavaScriptCore/inspector/protocol/Timeline.json:-9
> > -            "enum": ["EventDispatch", "ScheduleStyleRecalculation", "RecalculateStyles", "InvalidateLayout", "Layout", "Paint", "ScrollLayer", "ResizeImage", "ParseHTML", "TimerInstall", "TimerRemove", "TimerFire", "EvaluateScript", "MarkLoad", "MarkDOMContent", "TimeStamp", "Time", "TimeEnd", "ScheduleResourceRequest", "ResourceSendRequest", "ResourceReceiveResponse", "ResourceReceivedData", "ResourceFinish", "XHRReadyStateChange", "XHRLoad", "FunctionCall", "ProbeSample", "ConsoleProfile", "GCEvent", "RequestAnimationFrame", "CancelAnimationFrame", "FireAnimationFrame", "WebSocketCreate", "WebSocketSendHandshakeRequest", "WebSocketReceiveHandshakeResponse", "WebSocketDestroy"],
> 
> Remove from Legacy protocol versions too?

I figured I would leave them in so that we remember they existed. I'm on the fence still about when we should leave something in for historical purposes or not. In the past we were removing unsupported features, because they wouldn't have worked at all. This however, was working so I feel leaving it in is okay.
Comment 6 Brian Burg 2015-01-09 13:49:43 PST
(In reply to comment #5)
> (In reply to comment #4)
> > Comment on attachment 244312 [details]
> > [PATCH] Proposed Fix
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=244312&action=review
> > 
> > > Source/JavaScriptCore/inspector/protocol/Timeline.json:-9
> > > -            "enum": ["EventDispatch", "ScheduleStyleRecalculation", "RecalculateStyles", "InvalidateLayout", "Layout", "Paint", "ScrollLayer", "ResizeImage", "ParseHTML", "TimerInstall", "TimerRemove", "TimerFire", "EvaluateScript", "MarkLoad", "MarkDOMContent", "TimeStamp", "Time", "TimeEnd", "ScheduleResourceRequest", "ResourceSendRequest", "ResourceReceiveResponse", "ResourceReceivedData", "ResourceFinish", "XHRReadyStateChange", "XHRLoad", "FunctionCall", "ProbeSample", "ConsoleProfile", "GCEvent", "RequestAnimationFrame", "CancelAnimationFrame", "FireAnimationFrame", "WebSocketCreate", "WebSocketSendHandshakeRequest", "WebSocketReceiveHandshakeResponse", "WebSocketDestroy"],
> > 
> > Remove from Legacy protocol versions too?
> 
> I figured I would leave them in so that we remember they existed. I'm on the
> fence still about when we should leave something in for historical purposes
> or not. In the past we were removing unsupported features, because they
> wouldn't have worked at all. This however, was working so I feel leaving it
> in is okay.

I would like us to eventually have well-typed payloads for timelines and other places we use InspectorObjects, because it would make it easier to properly construct them and write tests. This would require some changes to the code generator and protocol specification format. This legacy cruft type is unlikely to have much in common with any feature along these lines.
Comment 7 WebKit Commit Bot 2015-01-09 14:26:31 PST
Comment on attachment 244312 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 244312

Committed r178201: <http://trac.webkit.org/changeset/178201>
Comment 8 WebKit Commit Bot 2015-01-09 14:26:34 PST
All reviewed patches have been landed.  Closing bug.