[blink] Split TaskType::Internal into InternalTimer and InternalLoading.

[blink] Split TaskType::Internal into UnspecedTimer and UnspecedLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696
R=skyostil@chromium.org,haraken@chromium.org

Committed: https://crrev.com/7b7cf3177c6221fec581acb28dbd3a4f3c3ca412
Cr-Commit-Position: refs/heads/master@{#439869}

[altimin, 2016-12-08 15:53:14]

PTAL.

I used my best judgement to choose between InternalTimer and InternalLoading
task types, but please double check this.

[Sami, 2016-12-08 17:05:08]

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:4357:
TaskRunnerHelper::get(TaskType::InternalLoading, this)
InternalTimer IMO

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/TaskRunnerHelper.h (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/TaskRunnerHelper.h:42: // regressions in page
load time when throttled in the background.
I think this is worded a bit too negatively. Maybe just say for tasks that are
associated with loading page content?

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameView.cpp:164:
TaskRunnerHelper::get(TaskType::InternalLoading, &frame),
I would classify both of these as timers since they have more to do with
rendering than loading.

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:357:
->get(TaskType::InternalLoading)
Let's keep this as a timer to avoid reordering with the other worker-related
messages.

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp:32:
->get(TaskType::InternalLoading)
Ditto.

[altimin, 2016-12-08 17:47:41]

PTAL

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.cpp:4357:
TaskRunnerHelper::get(TaskType::InternalLoading, this)
On 2016/12/08 17:05:08, Sami wrote:
> InternalTimer IMO

I'm inclined to make unthrottled (because sensitive input visibility sounds
scary and we might want to run it without throttling due to privacy/security
reasons).

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/TaskRunnerHelper.h (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/TaskRunnerHelper.h:42: // regressions in page
load time when throttled in the background.
On 2016/12/08 17:05:08, Sami wrote:
> I think this is worded a bit too negatively. Maybe just say for tasks that are
> associated with loading page content?

Done.

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2556993005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameView.cpp:164:
TaskRunnerHelper::get(TaskType::InternalLoading, &frame),
On 2016/12/08 17:05:08, Sami wrote:
> I would classify both of these as timers since they have more to do with
> rendering than loading.

Done.

[haraken, 2016-12-09 00:04:27]

LGTM

https://codereview.chromium.org/2556993005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/TaskRunnerHelper.h (right):

https://codereview.chromium.org/2556993005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/TaskRunnerHelper.h:44: InternalLoading,

Shall we rename these to UnspecedTimerTask and UnspecedLoadingTask for clarity?

[altimin, 2016-12-16 16:55:05]

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

[altimin, 2016-12-16 16:55:20]

Description was changed from

==========
[blink] Split TaskType::Internal into InternalTimer and InternalLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696

R=skyostil@chromium.org,haraken@chromium.org
==========

to

==========
[blink] Split TaskType::Internal into UnspecedTimer and UnspecedLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696

R=skyostil@chromium.org,haraken@chromium.org
==========

[commit-bot: I haz the power, 2016-12-16 16:55:26]

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

[altimin, 2016-12-16 16:55:34]

Sami, do you want to take another look?

[commit-bot: I haz the power, 2016-12-16 17:16:42]

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

[commit-bot: I haz the power, 2016-12-16 17:16:43]

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

[Sami, 2016-12-16 18:10:33]

Sorry, lost track of this. lgtm.

[altimin, 2016-12-20 15:19:59]

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

[commit-bot: I haz the power, 2016-12-20 15:20:23]

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, 2016-12-20 15:35:59]

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

[commit-bot: I haz the power, 2016-12-20 15:36:00]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[altimin, 2016-12-20 15:43:25]

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

[commit-bot: I haz the power, 2016-12-20 15:43:49]

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, 2016-12-20 19:03:07]

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

[commit-bot: I haz the power, 2016-12-20 19:03:08]

Dry run: This issue passed the CQ dry run.

[altimin, 2016-12-20 19:09:58]

The CQ bit was checked by altimin@chromium.org

[altimin, 2016-12-20 19:10:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/2556993005/#ps80001
(title: "One more fix")

[commit-bot: I haz the power, 2016-12-20 19:10:41]

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-12-20 19:37:00]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1482260998459870,
"parent_rev": "f0ec9e0f1ae0553125c706490c81346e97e4c4af", "commit_rev":
"f9460b097c26c35972feb3d29214703d652c05b3"}

[commit-bot: I haz the power, 2016-12-20 19:37:30]

Description was changed from

==========
[blink] Split TaskType::Internal into UnspecedTimer and UnspecedLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696

R=skyostil@chromium.org,haraken@chromium.org
==========

to

==========
[blink] Split TaskType::Internal into UnspecedTimer and UnspecedLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696

R=skyostil@chromium.org,haraken@chromium.org

Review-Url: https://codereview.chromium.org/2556993005
==========

[commit-bot: I haz the power, 2016-12-20 19:37:31]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-20 19:42:04]

Description was changed from

==========
[blink] Split TaskType::Internal into UnspecedTimer and UnspecedLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696

R=skyostil@chromium.org,haraken@chromium.org

Review-Url: https://codereview.chromium.org/2556993005
==========

to

==========
[blink] Split TaskType::Internal into UnspecedTimer and UnspecedLoading.

Differentiate between loading and non-loading internal blink tasks and
post them to appropriate task queues.

BUG=624696

R=skyostil@chromium.org,haraken@chromium.org

Committed: https://crrev.com/7b7cf3177c6221fec581acb28dbd3a4f3c3ca412
Cr-Commit-Position: refs/heads/master@{#439869}
==========

[commit-bot: I haz the power, 2016-12-20 19:42:06]

Patchset 5 (id:??) landed as
https://crrev.com/7b7cf3177c6221fec581acb28dbd3a4f3c3ca412
Cr-Commit-Position: refs/heads/master@{#439869}

[whywhat, 2017-02-23 20:35:12]

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

[whywhat, 2017-02-23 20:35:13]

Note that "unspeced" doesn't seem to be a word (neither is unspecced although I
imagine it'd have two c's or be spelled with an '). With only 36 places in the
codebase it might not be too late to change the name to Unspecified :)

[Sami, 2017-02-24 17:56:45]

On 2017/02/23 20:35:13, whywhat wrote:
> Note that "unspeced" doesn't seem to be a word (neither is unspecced although
I
> imagine it'd have two c's or be spelled with an '). With only 36 places in the
> codebase it might not be too late to change the name to Unspecified :)

Agreed that "unspeced" is a little weird. Google shows some hits for unspecced
with the meaning we're looking for, but I for one would be happy with
unspecified too.

[haraken, 2017-02-24 23:59:42]

On 2017/02/24 17:56:45, Sami wrote:
> On 2017/02/23 20:35:13, whywhat wrote:
> > Note that "unspeced" doesn't seem to be a word (neither is unspecced
although
> I
> > imagine it'd have two c's or be spelled with an '). With only 36 places in
the
> > codebase it might not be too late to change the name to Unspecified :)
> 
> Agreed that "unspeced" is a little weird. Google shows some hits for unspecced
> with the meaning we're looking for, but I for one would be happy with
> unspecified too.

Feel free to rename it :)