Issue 399043002: DevTools: switch Timeline frontend into buffered mode and remove the corresponding experiment.

Issue 399043002: DevTools: switch Timeline frontend into buffered mode and remove the corresponding experiment. (Closed)

Created:
6 years, 5 months ago by loislo

Modified:
6 years, 4 months ago

Reviewers:
caseq, alph, yurys, pfeldman

CC:
blink-reviews, shans, apavlov+blink_chromium.org, rjwright, Steve Block, caseq+blink_chromium.org, aandrey+blink_chromium.org, pfeldman+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, dstockwell, Timothy Loh, devtools-reviews_chromium.org, Eric Willigers, loislo+blink_chromium.org, lushnikov+blink_chromium.org, eustas+blink_chromium.org, paulirish+reviews_chromium.org, darktears, blink-reviews-animation_chromium.org, vsevik+blink_chromium.org, Mike Lawther (Google), sergeyv+blink_chromium.org

Base URL:
svn://svn.chromium.org/blink/trunk

Project:
blink

Visibility:
Public.

More Reviews

Description

DevTools: switch Timeline frontend into buffered mode and remove the corresponding experiment. All the tests were adjusted to the buffered mode. Console.timeline also records timeline in buffered mode. BUG=361045 R=yurys@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179113

Patch Set 1 #

Patch Set 2 : minor changes #

Total comments: 9

Patch Set 3 : rebaselined #

Patch Set 4 : rebaselined #

Patch Set 5 : timeline-decode-resize was fixed? #

Total comments: 8

Patch Set 6 : rebaselined #

Patch Set 7 : comments addressed #

Patch Set 8 : comments addressed #

Total comments: 1

Patch Set 9 : timeline-websocket-event rebaselined #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+220 lines, -391 lines)			Patch
M	LayoutTests/http/tests/inspector/timeline-test.js	View	1 2 3	3 chunks	+36 lines, -29 lines	0 comments	Download
M	LayoutTests/http/tests/inspector/timeline-xhr-event.html	View		2 chunks	+3 lines, -7 lines	0 comments	Download
M	LayoutTests/http/tests/inspector/timeline-xhr-response-type-blob-event.html	View	1	2 chunks	+10 lines, -8 lines	0 comments	Download
M	LayoutTests/http/tests/inspector/websocket/timeline-websocket-event.html	View	1	1 chunk	+5 lines, -8 lines	0 comments	Download
M	LayoutTests/http/tests/inspector/websocket/timeline-websocket-event-expected.txt	View	1 2 3 4 5 6 7 8	1 chunk	+1 line, -0 lines	0 comments	Download
D	LayoutTests/inspector-protocol/timeline/timeline-start-bufferEvents.html	View	1 2 3	1 chunk	+0 lines, -62 lines	0 comments	Download
D	LayoutTests/inspector-protocol/timeline/timeline-start-bufferEvents-expected.txt	View	1 2 3	1 chunk	+0 lines, -7 lines	0 comments	Download
M	LayoutTests/inspector/console/console-timeline.html	View	1 2 3 4 5 6	7 chunks	+49 lines, -65 lines	0 comments	Download
M	LayoutTests/inspector/console/console-timeline-expected.txt	View		3 chunks	+0 lines, -17 lines	0 comments	Download
M	LayoutTests/inspector/extensions/extensions-events.html	View	1 2 3 4 5 6	1 chunk	+0 lines, -18 lines	0 comments	Download
M	LayoutTests/inspector/extensions/extensions-events-expected.txt	View	1 2 3 4 5 6	1 chunk	+0 lines, -2 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-animation-frame.html	View		1 chunk	+3 lines, -6 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-bound-function.html	View		2 chunks	+3 lines, -4 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-bound-function-expected.txt	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-dom-content-loaded-event.html	View		1 chunk	+2 lines, -7 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-gc-event.html	View	1	1 chunk	+6 lines, -7 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-load-event.html	View	1	1 chunk	+2 lines, -8 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-network-received-data.html	View	1 2 3	1 chunk	+25 lines, -32 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-network-resource.html	View		2 chunks	+4 lines, -14 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-network-resource.js	View		1 chunk	+1 line, -1 line	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-paint.html	View		1 chunk	+11 lines, -17 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-receive-response-event.html	View	1 2 3	4 chunks	+4 lines, -18 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-script-id.html	View		2 chunks	+6 lines, -9 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-time.html	View	1 2 3	1 chunk	+1 line, -0 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-timer.html	View	1	2 chunks	+3 lines, -6 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-timer-fired-from-eval-call-site.html	View	1	2 chunks	+13 lines, -12 lines	0 comments	Download
M	LayoutTests/inspector/timeline/timeline-window-filter.html	View	1 2 3 4 5 6 7	1 chunk	+0 lines, -1 line	0 comments	Download
M	Source/core/inspector/InspectorTimelineAgent.cpp	View	1 2 3 4 5 6 7	1 chunk	+3 lines, -0 lines	0 comments	Download
M	Source/devtools/front_end/common/Settings.js	View	1 2 3 4 5 6 7 8	1 chunk	+0 lines, -1 line	0 comments	Download
M	Source/devtools/front_end/sdk/TimelineManager.js	View	1 2 3 4 5 6	6 chunks	+8 lines, -7 lines	0 comments	Download
M	Source/devtools/front_end/timeline/TimelineModelImpl.js	View	1 2 3 4 5 6 7	6 chunks	+17 lines, -8 lines	0 comments	Download
M	Source/devtools/front_end/timeline/TimelinePanel.js	View	1 2 3 4 5 6 7	5 chunks	+4 lines, -9 lines	0 comments	Download

Messages

Total messages: 17 (0 generated)

Expand Messages | Collapse Messages

loislo

On 2014/07/17 09:06:55, caseq wrote: > +Libo & Bryan in case this may affect PageSpeed. ...

6 years, 5 months ago (2014-07-17 10:42:11 UTC) #4

yurys

As agreed offline let's revert backend change and update only front-end and tests. https://codereview.chromium.org/399043002/diff/20001/LayoutTests/http/tests/inspector/timeline-test.js File ...

6 years, 5 months ago (2014-07-17 10:42:25 UTC) #5

blink-reviews

Thanks for letting us know about this change. We are not using the timeline data ...

6 years, 5 months ago (2014-07-17 14:22:32 UTC) #6

Thanks for letting us know about this change. We are not using the timeline
data any more, so this will net affect us. Thanks, -Libo


On Thu, Jul 17, 2014 at 6:42 AM, <yurys@chromium.org> wrote:

> As agreed offline let's revert backend change and update only front-end and
> tests.
>
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/http/tests/inspector/timeline-test.js
> File LayoutTests/http/tests/inspector/timeline-test.js (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/http/tests/inspector/timeline-test.js#newcode136
> LayoutTests/http/tests/inspector/timeline-test.js:136: var
> recordTypeName = (typeof records[i].type === "string") ? records[i].type
> : records[i].type();
> This must always be type(), can we fix this?
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/console/console-timeline.html
> File LayoutTests/inspector/console/console-timeline.html (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/console/console-timeline.html#newcode96
> LayoutTests/inspector/console/console-timeline.html:96:
> InspectorTest.addSniffer(panel._model._timelineManager,
> "_processBufferedEvents", next);
> Can we use an event for this?
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-network-received-data.html
> File LayoutTests/inspector/timeline/timeline-network-received-data.html
> (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-network-
> received-data.html#newcode18
> LayoutTests/inspector/timeline/timeline-network-received-data.html:18:
> window._scriptEvaluated = true;
> may be use callback counter?
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-network-
> received-data.html#newcode40
> LayoutTests/inspector/timeline/timeline-network-received-data.html:40:
> var filteredRecords =
> InspectorTest.filterTimelineRecords("ResourceReceivedData");
> filterTimelineRecords("ResourceReceivedData", true);
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-network-resource.html
> File LayoutTests/inspector/timeline/timeline-network-resource.html
> (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-network-resource.html#newcode11
> LayoutTests/inspector/timeline/timeline-network-resource.html:11:
> window.timelineNetworkResourceEvaluated = callback;
> Add a comment that it is called from timeline-network-resource.js ?
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-receive-response-event.html
> File LayoutTests/inspector/timeline/timeline-receive-response-event.html
> (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-receive-
> response-event.html#newcode15
> LayoutTests/inspector/timeline/timeline-receive-response-event.html:15:
> image.onload = function(event) { callback(); }  // do not pass event
> argument to the callback.
> Should be possible to do image.onload = callback;
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-timer-fired-
> from-eval-call-site.html
> File
> LayoutTests/inspector/timeline/timeline-timer-fired-
> from-eval-call-site.html
> (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-timer-fired-
> from-eval-call-site.html#newcode13
> LayoutTests/inspector/timeline/timeline-timer-fired-
> from-eval-call-site.html:13:
> "    window.callWhenDone();" +
> I'd either revert this or add \n.
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-window-filter-expected.txt
> File LayoutTests/inspector/timeline/timeline-window-filter-expected.txt
> (right):
>
> https://codereview.chromium.org/399043002/diff/20001/
> LayoutTests/inspector/timeline/timeline-window-
> filter-expected.txt#newcode36
> LayoutTests/inspector/timeline/timeline-window-filter-expected.txt:36:
> time range = 1390988669365.8047 - 1390988669866.5461
> Why did this change?
>
> https://codereview.chromium.org/399043002/diff/20001/
> Source/devtools/front_end/timeline/TimelineModelImpl.js
> File Source/devtools/front_end/timeline/TimelineModelImpl.js (left):
>
> https://codereview.chromium.org/399043002/diff/20001/
> Source/devtools/front_end/timeline/TimelineModelImpl.js#oldcode89
> Source/devtools/front_end/timeline/TimelineModelImpl.js:89: if
> (this._collectionEnabled)
> Without this check we will display events if timeline recording was
> initiated by an extension. @caseq, do you think it is OK?
>
> https://codereview.chromium.org/399043002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

yurys

https://codereview.chromium.org/399043002/diff/70001/LayoutTests/inspector/console/console-timeline.html File LayoutTests/inspector/console/console-timeline.html (right): https://codereview.chromium.org/399043002/diff/70001/LayoutTests/inspector/console/console-timeline.html#newcode154 LayoutTests/inspector/console/console-timeline.html:154: InspectorTest.addSniffer(panel._model._currentTarget.timelineManager, "_processBufferedEvents", next); Consider introducing event for this. https://codereview.chromium.org/399043002/diff/70001/LayoutTests/inspector/timeline/timeline-event-dispatch.html ...

6 years, 5 months ago (2014-07-24 16:58:37 UTC) #7

loislo

6 years, 4 months ago (2014-07-28 14:00:06 UTC) #8

yurys

lgtm https://codereview.chromium.org/399043002/diff/130001/Source/core/inspector/InspectorTimelineAgent.cpp File Source/core/inspector/InspectorTimelineAgent.cpp (right): https://codereview.chromium.org/399043002/diff/130001/Source/core/inspector/InspectorTimelineAgent.cpp#newcode830 Source/core/inspector/InspectorTimelineAgent.cpp:830: m_state->setBoolean(TimelineAgentState::bufferEvents, true); Please mention this change in the ...

6 years, 4 months ago (2014-07-29 07:19:21 UTC) #9

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/loislo@chromium.org/399043002/130001

6 years, 4 months ago (2014-07-29 07:22:53 UTC) #13

commit-bot: I haz the power

FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink ...

6 years, 4 months ago (2014-07-29 08:30:38 UTC) #14

commit-bot: I haz the power

The CQ bit was unchecked by commit-bot@chromium.org

6 years, 4 months ago (2014-07-29 08:57:20 UTC) #15

commit-bot: I haz the power

Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/18043)

6 years, 4 months ago (2014-07-29 08:57:21 UTC) #16

loislo

6 years, 4 months ago (2014-07-29 09:27:13 UTC) #17

Message was sent while issue was closed.

Committed patchset #9 manually as r179113 (presubmit successful).

Expand Messages | Collapse Messages