[DevTools] Turn starting tracing into promise.

[DevTools] Turn starting tracing into promise.

This allows to handle suspend/resume reload logic in a single place.
Also removed all kinds of now unnecessary started notifications.

BUG=none

Review-Url: https://codereview.chromium.org/2717393003
Cr-Commit-Position: refs/heads/master@{#453740}
Committed: https://chromium.googlesource.com/chromium/src/+/760f26d93cae268813692570a21601edb223aa9e

[dgozman, 2017-02-27 22:52:02]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[dgozman, 2017-02-27 22:52:02]

Could you please take a look?

[dgozman, 2017-02-27 22:52:07]

The CQ bit was checked by dgozman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 22:52:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[caseq, 2017-02-27 23:13:38]

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js
(right):

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:702: var
resourceTreeModel = SDK.ResourceTreeModel.fromTarget(this._target);
nit: move down to actual usage.

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js
(right):

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js:185:
tracingManager.start.bind(tracingManager, this, categories, options,
undefined));
Looks like we need to defer this to when tracingManager.start() asynchronously
completes.

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js:191:
tracingStarted() {
Now that startRecording returns a promise, you can get rid of this one perhaps.

[commit-bot: I haz the power, 2017-02-28 00:38:38]

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

[commit-bot: I haz the power, 2017-02-28 00:38:39]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-02-28 01:57:21]

Description was changed from

==========
[DevTools] Turn starting tracing into promise.

This allows to handle suspend/resume reload logic in a single place.

BUG=none
==========

to

==========
[DevTools] Turn starting tracing into promise.

This allows to handle suspend/resume reload logic in a single place.
Also removed all kinds of now unnecessary started notifications.

BUG=none
==========

[dgozman, 2017-02-28 01:58:17]

PTAL

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js
(right):

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:702: var
resourceTreeModel = SDK.ResourceTreeModel.fromTarget(this._target);
On 2017/02/27 23:13:38, caseq wrote:
> nit: move down to actual usage.

this._target is reset in next line.

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js
(right):

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js:185:
tracingManager.start.bind(tracingManager, this, categories, options,
undefined));
On 2017/02/27 23:13:38, caseq wrote:
> Looks like we need to defer this to when tracingManager.start() asynchronously
> completes.

Good catch! Done.

https://codereview.chromium.org/2717393003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js:191:
tracingStarted() {
On 2017/02/27 23:13:38, caseq wrote:
> Now that startRecording returns a promise, you can get rid of this one
perhaps.

Done.

[alph, 2017-02-28 03:11:24]

alph@chromium.org changed reviewers:
+ alph@chromium.org

[alph, 2017-02-28 03:11:25]

lgtm

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/timeline-test.js
(right):

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/timeline-test.js:93:
timelineController._startRecordingWithCategories(categories,
enableJSSampling).then(tracingStarted);
await

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/inspector/tracing.html (right):

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/tracing.html:52:
tracingManager.start(tracingClient, "", "").then(() => {
await

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js
(right):

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineController.js:184:
return profilingStartedPromise.then(() => tracingManager.start(this, categories,
options));
nit: await

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:569:
return this._controller.startRecording(recordingOptions, enabledTraceProviders)
nit: await

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:704:
'Click the reload button %s or hit %s to record and evaluate the page load.',
revert?

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:709:
'Then, zoom and pan the timeline with the mousewheel or %s keys.\n%s',
ditto

[dgozman, 2017-02-28 08:59:01]

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/timeline-test.js
(right):

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/timeline-test.js:93:
timelineController._startRecordingWithCategories(categories,
enableJSSampling).then(tracingStarted);
On 2017/02/28 03:11:24, alph wrote:
> await

No, thanks :-)

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js
(right):

https://codereview.chromium.org/2717393003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelinePanel.js:704:
'Click the reload button %s or hit %s to record and evaluate the page load.',
On 2017/02/28 03:11:25, alph wrote:
> revert?

That's what formatter does for me.

[dgozman, 2017-02-28 19:47:15]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2017-02-28 19:47:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[caseq, 2017-02-28 21:41:36]

lgtm

[commit-bot: I haz the power, 2017-02-28 23:27:39]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488311235119430,
"parent_rev": "eab82c830edc973964782a440d878ed077e6a5be", "commit_rev":
"760f26d93cae268813692570a21601edb223aa9e"}

[commit-bot: I haz the power, 2017-02-28 23:28:17]

Description was changed from

==========
[DevTools] Turn starting tracing into promise.

This allows to handle suspend/resume reload logic in a single place.
Also removed all kinds of now unnecessary started notifications.

BUG=none
==========

to

==========
[DevTools] Turn starting tracing into promise.

This allows to handle suspend/resume reload logic in a single place.
Also removed all kinds of now unnecessary started notifications.

BUG=none

Review-Url: https://codereview.chromium.org/2717393003
Cr-Commit-Position: refs/heads/master@{#453740}
Committed:
https://chromium.googlesource.com/chromium/src/+/760f26d93cae268813692570a216...
==========

[commit-bot: I haz the power, 2017-02-28 23:28:18]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/760f26d93cae268813692570a216...