scheduler: Make DOM window event tasks unthrottled

scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG=675372
Review-Url: https://codereview.chromium.org/2651933002
Cr-Commit-Position: refs/heads/master@{#446000}
Committed: https://chromium.googlesource.com/chromium/src/+/05a13309259dd16f38eef41adc7cce55e3d15a05

[Sami, 2017-01-24 11:44:05]

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

[commit-bot: I haz the power, 2017-01-24 11:44:17]

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

[Sami, 2017-01-24 12:15:35]

Description was changed from

==========
scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG=675372
==========

to

==========
scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG=675372
==========

[Sami, 2017-01-24 12:15:35]

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

[Sami, 2017-01-24 12:16:16]

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

[commit-bot: I haz the power, 2017-01-24 12:16:54]

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

[commit-bot: I haz the power, 2017-01-24 12:16:54]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Sami, 2017-01-24 12:16:55]

PTAL, thanks!

FYI, decided against adding a layout test because we don't have a good way to
work around the 10s throttling grace period. There's a manual test here:
https://jsfiddle.net/mhan190b/show/light/

[altimin, 2017-01-24 12:19:13]

lgtm

[Sami, 2017-01-24 16:05:00]

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

[Sami, 2017-01-24 16:05:24]

Ah, I think Kentaro is on a plane right now. Daniel, could you take a look at
this P1 patch please?

[dcheng, 2017-01-24 16:15:51]

LGTM

[Sami, 2017-01-24 16:22:33]

The CQ bit was checked by skyostil@chromium.org

[commit-bot: I haz the power, 2017-01-24 16:22:44]

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-01-24 16:30:47]

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

[commit-bot: I haz the power, 2017-01-24 16:30:48]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[dcheng, 2017-01-24 16:31:33]

On 2017/01/24 16:22:44, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

Incidentally, it feels a little unfortunate to make the entire DOM window event
queue unthrottled, but perhaps we have no choice here. I wonder if it'd be
possible to tweak this in a followup so we only use the unthrottled task queue
where there are indexed db event in the queue.

[haraken, 2017-01-24 21:53:29]

LGTM

Just to confirm: There is no option to unthrottle only Indexed DB events 
because the Indexed DB events are speced to be scheduled in a FIFO order with
other events in DOMWindowEventQueue, right?

https://codereview.chromium.org/2651933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp (right):

https://codereview.chromium.org/2651933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp:46: :
SuspendableTimer(context, TaskType::Unthrottled),

Add a comment and mention why this needs to be unthrottled.

[Sami, 2017-01-25 10:29:30]

On 2017/01/24 21:53:29, haraken wrote:
> LGTM
> 
> Just to confirm: There is no option to unthrottle only Indexed DB events 
> because the Indexed DB events are speced to be scheduled in a FIFO order with
> other events in DOMWindowEventQueue, right?

Glancing through the spec, it mentions a "database access task source" but it's
not clear which tasks are meant to go there. I suspect we might be able to
introduce a new task queue for these tasks, but right now the code isn't really
set up to do that and I'd like to get this fix into M56 too.

Another potential future improvement we talked about was avoiding throttling
tabs that have active IndexedDB transactions.

https://codereview.chromium.org/2651933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp (right):

https://codereview.chromium.org/2651933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/DOMWindowEventQueue.cpp:46: :
SuspendableTimer(context, TaskType::Unthrottled),
On 2017/01/24 21:53:29, haraken wrote:
> 
> Add a comment and mention why this needs to be unthrottled.

Done.

[Sami, 2017-01-25 10:29:39]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2017-01-25 10:29:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, altimin@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2651933002/#ps20001
(title: "Added a comment.")

[commit-bot: I haz the power, 2017-01-25 10:29:58]

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-01-25 12:09:02]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485340179620630,
"parent_rev": "dacb8febda9ad9ac9c7724e4fe1aa3cc0bef3cfd", "commit_rev":
"05a13309259dd16f38eef41adc7cce55e3d15a05"}

[commit-bot: I haz the power, 2017-01-25 12:09:34]

Description was changed from

==========
scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG=675372
==========

to

==========
scheduler: Make DOM window event tasks unthrottled

This patch avoids throttling events that are routed via
DOMWindowEventQueue. This affects a handful of window-related events,
the most important ones being IndexedDB events. It turns out many web
pages (e.g., Google Docs) rely on IndexedDB transactions to be
relatively fast and start encountering errors if they are throttled too
aggressively while a tab is in the background.

BUG=675372

Review-Url: https://codereview.chromium.org/2651933002
Cr-Commit-Position: refs/heads/master@{#446000}
Committed:
https://chromium.googlesource.com/chromium/src/+/05a13309259dd16f38eef41adc7c...
==========

[commit-bot: I haz the power, 2017-01-25 12:09:36]

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