[webkit-reviews] review canceled: [Bug 61098] Web Inspector: Provide inspector extension API to access timeline data : [Attachment 94008] Proposed Patch

[bugzilla-daemon at webkit.org]

Michael Schneider <michschn at google.com> has canceled Michael Schneider
<michschn at google.com>'s request for review:
Bug 61098: Web Inspector: Provide inspector extension API to access timeline
data
https://bugs.webkit.org/show_bug.cgi?id=61098

Attachment 94008: Proposed Patch
https://bugs.webkit.org/attachment.cgi?id=94008&action=review

------- Additional Comments from Michael Schneider <michschn at google.com>
Looks like I overlooked this test failure and submitted the patch prematurely.

After looking at the failure, my initial analysis is as follows:
- The TimelinePanel was active all the time before, now it ignores the events
when the it is not activated.
- The TimelineTests just activate the instrumentation data collection using
TimelineAgent.start().
- The WebInspector.TimelinePanel.FormattedRecord constructor has a side effect,
which adds the now missing url property to the record.
- This function is not called anymore, as the TimelinePanel is in a disabled
state.
- The failing test explicitly checks for that side effect.

IMO, this side effect must either be keept local to the TimelinePanel or be
applied to all listeners, regardless if the the TimelinePanel is activated or
not.
- we could copy the record, then modify the test to enable the TimelinePanel.
- we could add the URL property in the TimelineManager already, so that every
listener receives the modified record

What do you think? I do not know what the best solution is here.