Use TaskRunnerHelper in CanvasAsyncBlobCreator

Use TaskRunnerHelper in CanvasAsyncBlobCreator

BUG=624696
Committed: https://crrev.com/adbe3f34c435039528111c045a832f74e6a93e8f
Cr-Commit-Position: refs/heads/master@{#405388}

[haraken, 2016-07-13 07:32:29]

haraken@chromium.org changed reviewers:
+ dcheng@chromium.org, hiroshige@chromium.org, tzik@chromium.org

[haraken, 2016-07-13 07:32:29]

PTAL

[hiroshige, 2016-07-13 07:38:05]

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

[dcheng, 2016-07-13 07:38:25]

https://codereview.chromium.org/2143353002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
(right):

https://codereview.chromium.org/2143353002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp:221:
TaskRunnerHelper::getUnthrottledTaskRunner(m_document)->postTask(BLINK_FROM_HERE,
WTF::bind(&CanvasAsyncBlobCreator::createBlobAndInvokeCallback,
wrapPersistent(this)));
Not related to this CL, but this name is quite long. Also, I wonder... should
getUnthrottledTaskRunner pass by reference instead of pointer? In this CL and in
https://codereview.chromium.org/2141873002/, it seems like the supplied argument
should never be null anyway.

[commit-bot: I haz the power, 2016-07-13 07:38:26]

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

[haraken, 2016-07-13 07:48:17]

On 2016/07/13 07:38:25, dcheng wrote:
>
https://codereview.chromium.org/2143353002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
> (right):
> 
>
https://codereview.chromium.org/2143353002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp:221:
>
TaskRunnerHelper::getUnthrottledTaskRunner(m_document)->postTask(BLINK_FROM_HERE,
> WTF::bind(&CanvasAsyncBlobCreator::createBlobAndInvokeCallback,
> wrapPersistent(this)));
> Not related to this CL, but this name is quite long. Also, I wonder... should
> getUnthrottledTaskRunner pass by reference instead of pointer? In this CL and
in
> https://codereview.chromium.org/2141873002/, it seems like the supplied
argument
> should never be null anyway.

We can drop the "TaskRunner" suffix if you like it :)

I'm intentionally passing a pointer because it can be null if document->frame()
returns null etc (then TaskRunnerHelper falls back to the default task runner).
This CL also passes a null document in CanvasAsyncBlobCreatorTest (we can avoid
this though).

[commit-bot: I haz the power, 2016-07-13 08:47:16]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-13 08:47:17]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-07-13 14:24:29]

On 2016/07/13 07:48:17, haraken wrote:
> On 2016/07/13 07:38:25, dcheng wrote:
> >
>
https://codereview.chromium.org/2143353002/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
> > (right):
> > 
> >
>
https://codereview.chromium.org/2143353002/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp:221:
> >
>
TaskRunnerHelper::getUnthrottledTaskRunner(m_document)->postTask(BLINK_FROM_HERE,
> > WTF::bind(&CanvasAsyncBlobCreator::createBlobAndInvokeCallback,
> > wrapPersistent(this)));
> > Not related to this CL, but this name is quite long. Also, I wonder...
should
> > getUnthrottledTaskRunner pass by reference instead of pointer? In this CL
and
> in
> > https://codereview.chromium.org/2141873002/, it seems like the supplied
> argument
> > should never be null anyway.
> 
> We can drop the "TaskRunner" suffix if you like it :)

Perhaps! For now, it's not much of a problem, but when Blink is reformatted to
80 columns, it might be a bit more annoying.

> 
> I'm intentionally passing a pointer because it can be null if
document->frame()
> returns null etc (then TaskRunnerHelper falls back to the default task
runner).
> This CL also passes a null document in CanvasAsyncBlobCreatorTest (we can
avoid
> this though).

This CL LGTM. I wonder what the best default behavior is though: I'm guessing
there are going to be some things where we just don't want to post the task if
document->frame() is gone. This is easy enough to revisit later though.

[haraken, 2016-07-14 01:25:38]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2016-07-14 01:26:36]

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-07-14 01:34:01]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-14 01:34:05]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 01:35:42]

Description was changed from

==========
Use TaskRunnerHelper in CanvasAsyncBlobCreator

BUG=624696
==========

to

==========
Use TaskRunnerHelper in CanvasAsyncBlobCreator

BUG=624696

Committed: https://crrev.com/adbe3f34c435039528111c045a832f74e6a93e8f
Cr-Commit-Position: refs/heads/master@{#405388}
==========

[commit-bot: I haz the power, 2016-07-14 01:35:43]

Patchset 1 (id:??) landed as
https://crrev.com/adbe3f34c435039528111c045a832f74e6a93e8f
Cr-Commit-Position: refs/heads/master@{#405388}