Move PerformanceBase to TaskRunnerTimer.

Move PerformanceBase to TaskRunnerTimer.

BUG=624694
Review-Url: https://codereview.chromium.org/2646933002
Cr-Commit-Position: refs/heads/master@{#444983}
Committed: https://chromium.googlesource.com/chromium/src/+/1174657e747a79abfc910fdd863552562ccfc8da

[jbroman, 2017-01-20 02:12:40]

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

[commit-bot: I haz the power, 2017-01-20 02:13:00]

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

[jbroman, 2017-01-20 02:14:20]

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

[haraken, 2017-01-20 02:21:51]

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceBase.cpp (right):

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceBase.cpp:77:
std::move(taskRunner),

What is making it hard to unconditionally use
TaskRunnerHelper::get(TaskType::PerformanceTimeline) here?

[jbroman, 2017-01-20 02:27:05]

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceBase.cpp (right):

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceBase.cpp:77:
std::move(taskRunner),
On 2017/01/20 at 02:21:51, haraken wrote:
> What is making it hard to unconditionally use
TaskRunnerHelper::get(TaskType::PerformanceTimeline) here?

PerformanceBase doesn't have any object to pass to TaskRunnerHelper. And it's
the base class also of WorkerPerformance, so LocalFrame and Document don't
apply.

ExecutionContext could possibly be used, but we don't actually need the whole
ExecutionContext, and currently some of the test implementations return nullptr
from their override of getExecutionContext.
It seems better, then, to have the derived class be responsible for choosing the
arguments to pass to TaskRunnerHelper. The only subclass for which that's
currently very meaningful is Performance (the main-thread version).

[haraken, 2017-01-20 02:30:25]

LGTM

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp (right):

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp:27:
Platform::current()->currentThread()->getWebTaskRunner()) {}

I'd prefer using TaskType::PerformanceTimeline (just in case).

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceObserverTest.cpp (right):

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceObserverTest.cpp:24:
Platform::current()->currentThread()->getWebTaskRunner()) {}

Ditto.

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/WorkerPerformance.cpp (right):

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/WorkerPerformance.cpp:46:
Platform::current()->currentThread()->scheduler()->timerTaskRunner()),

Yeah, we don't yet have a good concept of TaskTypes in worker threads. So
directly using the timer task runner makes sense.

[jbroman, 2017-01-20 02:33:35]

Just asking for clarification on how you'd like me to deal with the test impls.

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp (right):

https://codereview.chromium.org/2646933002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp:27:
Platform::current()->currentThread()->getWebTaskRunner()) {}
On 2017/01/20 at 02:30:25, haraken wrote:
> I'd prefer using TaskType::PerformanceTimeline (just in case).

With which LocalFrame/Document/ExecutionContext/ScriptState? I could pass in the
ScriptState from PerformanceBaseTest::initialize here (though it seems odd to
simultaneously have TestPerformanceBase::getExecutionContext return nullptr).

[jbroman, 2017-01-20 02:40:49]

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

[commit-bot: I haz the power, 2017-01-20 02:41:16]

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

[jbroman, 2017-01-20 02:42:10]

Is this what you had in mind? Or did you also want the getExecutionContext
overrides changed?

(Or did you want PerformanceBase to require an ExecutionContext as a constructor
argument, instead? If so, did you want the getExecutionContext overrides
changed?)

[jbroman, 2017-01-20 02:43:26]

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

[commit-bot: I haz the power, 2017-01-20 02:43:38]

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-01-20 04:25:20]

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

[commit-bot: I haz the power, 2017-01-20 04:25:20]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-01-20 04:26:08]

Patch Set 3 LGTM :)

[jbroman, 2017-01-20 04:31:31]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2017-01-20 04:31:42]

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-20 04:36:09]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484886691253000,
"parent_rev": "ac7bf105a4b432d23c18f0e4a042b960c2a9af87", "commit_rev":
"1174657e747a79abfc910fdd863552562ccfc8da"}

[commit-bot: I haz the power, 2017-01-20 04:36:43]

Description was changed from

==========
Move PerformanceBase to TaskRunnerTimer.

BUG=624694
==========

to

==========
Move PerformanceBase to TaskRunnerTimer.

BUG=624694

Review-Url: https://codereview.chromium.org/2646933002
Cr-Commit-Position: refs/heads/master@{#444983}
Committed:
https://chromium.googlesource.com/chromium/src/+/1174657e747a79abfc910fdd8635...
==========

[commit-bot: I haz the power, 2017-01-20 04:36:44]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1174657e747a79abfc910fdd8635...