[scheduler] Report background timer throttling intervention.

[scheduler] Report background timer throttling intervention.

Use InterventionReporter to report throttling to the console if the
throttling delay is greater than 3 seconds. (It doesn't make sense
to report smaller throttling durations given that we align timers to
1Hz).

BUG=639852
Committed: https://crrev.com/74942b51aa27981ee18d9cbfe1425a250ab76abc
Cr-Commit-Position: refs/heads/master@{#430704}

[altimin, 2016-11-04 16:21:03]

altimin@chromium.org changed reviewers:
+ alexclarke@chromium.org, skyostil@chromium.org

[altimin, 2016-11-04 16:21:04]

PTAL

[Sami, 2016-11-04 17:51:49]

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h
(right):

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h:89:
// is throttled.
Please explain what value the callback is given too.

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc
(right):

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc:25:
constexpr base::TimeDelta kMinimalThrottlingDurationToReport =
kMinimalThrottlingDurationToReport since we report about other types of
throttling too.

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc:212:
"Background timers take too long to run on this page. "
To rephrase a bit:

"Timer tasks have used too much CPU time while the page was in the background.
As a result, they have been deferred by NN seconds. See ... for more details."

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc:218:
intervention_reporter_->ReportIntervention(WebString::fromUTF8(message));
Is this potentially spammy? I wonder if we should just print this once per
navigation. I don't think the developers need notification for each and every
instance.

[alex clarke (OOO till 29th), 2016-11-07 16:53:10]

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h
(right):

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h:89:
// is throttled.
On 2016/11/04 17:51:49, Sami wrote:
> Please explain what value the callback is given too.

+1 You could consider naming it here.

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h:149:
base::Optional<base::Callback<void(base::TimeDelta)>> reporting_callback_;
Do we really need base::Optional? base::Callback has a default constructor that
does nothing for which is_null returns true.

[altimin, 2016-11-07 17:25:30]

PTAL

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h
(right):

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.h:89:
// is throttled.
On 2016/11/07 16:53:10, alex clarke wrote:
> On 2016/11/04 17:51:49, Sami wrote:
> > Please explain what value the callback is given too.
> 
> +1 You could consider naming it here.

Done.

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc
(right):

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc:25:
constexpr base::TimeDelta kMinimalThrottlingDurationToReport =
On 2016/11/04 17:51:49, Sami wrote:
> kMinimalThrottlingDurationToReport since we report about other types of
> throttling too.

Did you mean kMinimalBackgroundThrottlingDurationToReport? :)

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc:212:
"Background timers take too long to run on this page. "
On 2016/11/04 17:51:49, Sami wrote:
> To rephrase a bit:
> 
> "Timer tasks have used too much CPU time while the page was in the background.
> As a result, they have been deferred by NN seconds. See ... for more details."

Done.

https://codereview.chromium.org/2476973002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc:218:
intervention_reporter_->ReportIntervention(WebString::fromUTF8(message));
On 2016/11/04 17:51:49, Sami wrote:
> Is this potentially spammy? I wonder if we should just print this once per
> navigation. I don't think the developers need notification for each and every
> instance.

Done.

[Sami, 2016-11-08 19:05:09]

Thanks for the changes, lgtm.

https://codereview.chromium.org/2476973002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc
(right):

https://codereview.chromium.org/2476973002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:199:
reporting_callback_.Run(-current_budget_level_ / cpu_percentage_);
nit: Is forgot if calling a null callback is a no-op or not? If it's a no-op, we
could just remove the null check since this is always gonna be there in
production code.

[altimin, 2016-11-08 19:08:39]

The CQ bit was checked by altimin@chromium.org

[altimin, 2016-11-08 19:08:40]

The CQ bit was unchecked by altimin@chromium.org

[altimin, 2016-11-08 19:08:40]

The CQ bit was checked by altimin@chromium.org

[altimin, 2016-11-08 19:08:47]

https://codereview.chromium.org/2476973002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc
(right):

https://codereview.chromium.org/2476973002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/renderer/task_queue_throttler.cc:199:
reporting_callback_.Run(-current_budget_level_ / cpu_percentage_);
On 2016/11/08 19:05:08, Sami wrote:
> nit: Is forgot if calling a null callback is a no-op or not? If it's a no-op,
we
> could just remove the null check since this is always gonna be there in
> production code.

It's a failure, unfortunately.

[commit-bot: I haz the power, 2016-11-08 19:09:16]

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, 2016-11-08 20:42:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-08 21:00:05]

Description was changed from

==========
[scheduler] Report background timer throttling intervention.

Use InterventionReporter to report throttling to the console if the
throttling delay is greater than 3 seconds. (It doesn't make sense
to report smaller throttling durations given that we align timers to
1Hz).

BUG=639852
==========

to

==========
[scheduler] Report background timer throttling intervention.

Use InterventionReporter to report throttling to the console if the
throttling delay is greater than 3 seconds. (It doesn't make sense
to report smaller throttling durations given that we align timers to
1Hz).

BUG=639852

Committed: https://crrev.com/74942b51aa27981ee18d9cbfe1425a250ab76abc
Cr-Commit-Position: refs/heads/master@{#430704}
==========

[commit-bot: I haz the power, 2016-11-08 21:00:07]

Patchset 3 (id:??) landed as
https://crrev.com/74942b51aa27981ee18d9cbfe1425a250ab76abc
Cr-Commit-Position: refs/heads/master@{#430704}