Expected Queueing Time Metric ignores tasks with nested message loops.

Expected Queueing Time Metric ignores tasks with nested message loops.

We're seeing cases where the Expected Queueing Time metric is reporting
values which are far too high. We hypothesize that this is due to
nested message loops, which currently cause us to report a task with
length equal to the duration of the nested message loop.

Exclude tasks containing nested message loops from this metric.

BUG=675060
Review-Url: https://codereview.chromium.org/2788013003
Cr-Commit-Position: refs/heads/master@{#461210}
Committed: https://chromium.googlesource.com/chromium/src/+/c4067e72471b45ccb8d775189e53c6bcd3cb2c91

[tdresser, 2017-03-31 15:27:59]

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

[commit-bot: I haz the power, 2017-03-31 15:28:17]

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

[commit-bot: I haz the power, 2017-03-31 16:58:48]

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

[commit-bot: I haz the power, 2017-03-31 16:58:49]

Dry run: This issue passed the CQ dry run.

[tdresser, 2017-03-31 17:16:21]

tdresser@chromium.org changed reviewers:
+ skyostil@chromium.org

[tdresser, 2017-03-31 17:16:22]

PTAL

I'll address the sleep case in another patch.

[Sami, 2017-03-31 18:05:35]

lgtm

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/PerformanceMonitor.h (right):

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/PerformanceMonitor.h:123: void
onBeginNestedMessageLoop() override{};
nit: Remove the ';' so "git cl format" will format this properly.

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator_unittest.cc
(right):

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator_unittest.cc:148:
// The main thread is considered unresponsive during a single long task, which
nit: I'm not really following how this comment relates to the test below since
the long task is effectively ignored?

[tdresser, 2017-03-31 18:43:20]

tdresser@chromium.org changed reviewers:
+ bokan@chromium.org

[tdresser, 2017-03-31 18:43:21]

+bokan@, for 1 line change to PerformanceMonitor.h.

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/PerformanceMonitor.h (right):

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/PerformanceMonitor.h:123: void
onBeginNestedMessageLoop() override{};
On 2017/03/31 18:05:34, Sami wrote:
> nit: Remove the ';' so "git cl format" will format this properly.

Done.

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator_unittest.cc
(right):

https://codereview.chromium.org/2788013003/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/queueing_time_estimator_unittest.cc:148:
// The main thread is considered unresponsive during a single long task, which
On 2017/03/31 18:05:34, Sami wrote:
> nit: I'm not really following how this comment relates to the test below since
> the long task is effectively ignored?

Whoops, that was some bad copy pasta.

Fixed.

[bokan, 2017-03-31 18:45:55]

PerformanceMonitor lgtm

[tdresser, 2017-03-31 18:48:54]

The CQ bit was checked by tdresser@chromium.org

[tdresser, 2017-03-31 18:48:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/2788013003/#ps20001
(title: "Address nits")

[commit-bot: I haz the power, 2017-03-31 18:49:20]

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

[commit-bot: I haz the power, 2017-03-31 20:35:03]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490986134956200,
"parent_rev": "ddc6c499fc6f05937be269de5c0daf54b0d298fb", "commit_rev":
"c4067e72471b45ccb8d775189e53c6bcd3cb2c91"}

[commit-bot: I haz the power, 2017-03-31 20:35:54]

Description was changed from

==========
Expected Queueing Time Metric ignores tasks with nested message loops.

We're seeing cases where the Expected Queueing Time metric is reporting
values which are far too high. We hypothesize that this is due to
nested message loops, which currently cause us to report a task with
length equal to the duration of the nested message loop.

Exclude tasks containing nested message loops from this metric.

BUG=675060
==========

to

==========
Expected Queueing Time Metric ignores tasks with nested message loops.

We're seeing cases where the Expected Queueing Time metric is reporting
values which are far too high. We hypothesize that this is due to
nested message loops, which currently cause us to report a task with
length equal to the duration of the nested message loop.

Exclude tasks containing nested message loops from this metric.

BUG=675060

Review-Url: https://codereview.chromium.org/2788013003
Cr-Commit-Position: refs/heads/master@{#461210}
Committed:
https://chromium.googlesource.com/chromium/src/+/c4067e72471b45ccb8d775189e53...
==========

[commit-bot: I haz the power, 2017-03-31 20:35:55]

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