Move ThreadableLoaderClientWrapper to Oilpan heap

Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader.

BUG=587663
Committed: https://crrev.com/ce88c0f90d6859b34255d6992aace413f84f1d1f
Cr-Commit-Position: refs/heads/master@{#407058}

[yhirano, 2016-07-14 08:14:07]

Patchset #3 (id:40001) has been deleted

[yhirano, 2016-07-14 08:14:15]

Patchset #2 (id:20001) has been deleted

[yhirano, 2016-07-14 08:14:24]

Patchset #3 (id:80001) has been deleted

[yhirano, 2016-07-14 08:14:28]

Patchset #2 (id:60001) has been deleted

[yhirano, 2016-07-14 08:16:42]

Description was changed from

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

BUG=587663
==========

to

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient.

BUG=587663
==========

[yhirano, 2016-07-14 08:18:59]

yhirano@chromium.org changed reviewers:
+ hiroshige@chromium.org, oilpan-reviews@chromium.org, tyoshino@chromium.org

[yhirano, 2016-07-14 08:18:59]

+tyoshino@ for ResourceTimingClient
+hiroshige@ for threading / post-tasking
+oilpan-reviews for oilpanization

[haraken, 2016-07-14 08:33:17]

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

[haraken, 2016-07-14 08:33:17]

https://codereview.chromium.org/2146333002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h
(right):

https://codereview.chromium.org/2146333002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h:140:
ThreadableLoaderClient* m_client = nullptr;

Is it guaranteed that clearClient() is explicitly called when the client is
gone? (i.e., is it guaranteed that m_client doesn't become a stale pointer?)

[yhirano, 2016-07-14 08:55:41]

https://codereview.chromium.org/2146333002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h
(right):

https://codereview.chromium.org/2146333002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h:140:
ThreadableLoaderClient* m_client = nullptr;
On 2016/07/14 08:33:17, haraken wrote:
> 
> Is it guaranteed that clearClient() is explicitly called when the client is
> gone? (i.e., is it guaranteed that m_client doesn't become a stale pointer?)

This class is inistantiated only in WorkerThreadableLoader constructor and its
|client| argument is passed to this class's constructor.

WorkerThreadableLoader's assumption is |*client| is valid as long as the
WorkerThreadableLoader instance is valid. As |clearClient| is called in
~WorkerThreadableLoader (via MainThreadBridgeBase::destroy), I think it's OK.

[haraken, 2016-07-14 09:05:32]

LGTM

[tyoshino (SeeGerritForStatus), 2016-07-15 04:54:56]

lgtm

[hiroshige, 2016-07-19 10:06:32]

hiroshige@chromium.org changed reviewers:
+ ksakamoto@chromium.org

[hiroshige, 2016-07-19 10:06:33]

lgtm.

Could you mention in the CL description that this CL makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
WorkerThreadableLoader?
+ksakamoto@ FYI.

[yhirano, 2016-07-19 11:22:40]

Description was changed from

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient.

BUG=587663
==========

to

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader?

BUG=587663
==========

[yhirano, 2016-07-19 11:23:04]

Description was changed from

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader?

BUG=587663
==========

to

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader.

BUG=587663
==========

[yhirano, 2016-07-19 11:23:14]

On 2016/07/19 10:06:33, hiroshige wrote:
> lgtm.
> 
> Could you mention in the CL description that this CL makes
> ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
> WorkerThreadableLoader?
Done.

[yhirano, 2016-07-20 01:20:48]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2016-07-20 01:21:15]

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

[yhirano, 2016-07-20 01:55:57]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2016-07-21 12:38:56]

ksakamoto@: ping

[Kunihiko Sakamoto, 2016-07-22 02:24:57]

Sorry for the delay, I didn't realize I'm in the R field.

https://codereview.chromium.org/2146333002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h
(right):

https://codereview.chromium.org/2146333002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h:129:
m_client->didReceiveResourceTiming(*info);
I think this is always no-op. Did you add it by some necessity, or just for
consistency with other client calls?

[yhirano, 2016-07-22 02:28:12]

https://codereview.chromium.org/2146333002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h
(right):

https://codereview.chromium.org/2146333002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/loader/ThreadableLoaderClientWrapper.h:129:
m_client->didReceiveResourceTiming(*info);
On 2016/07/22 02:24:57, Kunihiko Sakamoto wrote:
> I think this is always no-op. Did you add it by some necessity, or just for
> consistency with other client calls?

Just for consistency. Unittests become cleaner with this change :)

[Kunihiko Sakamoto, 2016-07-22 02:31:19]

lgtm

[yhirano, 2016-07-22 02:57:47]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2016-07-22 02:58:20]

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-22 04:12:03]

Description was changed from

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader.

BUG=587663
==========

to

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader.

BUG=587663
==========

[commit-bot: I haz the power, 2016-07-22 04:12:04]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2016-07-22 04:13:50]

Description was changed from

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader.

BUG=587663
==========

to

==========
Move ThreadableLoaderClientWrapper to Oilpan heap

As preparation for moving ThreadableLoader to Oilpan heap, this CL moves
ThreadableLoaderClientWrapper to Oilpan heap.

This CL also removes ThreadableLoaderClientWrapper::ResourceTimingClient and
makes
ThreadableLoaderClient::didReceiveResourceTiming() to be called also for
ThreadableLoaderClient associated with WorkerThreadableLoader.

BUG=587663

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

[commit-bot: I haz the power, 2016-07-22 04:13:51]

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