Create PendingScriptClient as a separate client interface for PendingScript.

Create PendingScriptClient as a separate client interface for PendingScript.

This is aimed toward making PendingScript's relationship with its client
more clear and consistent.

Currently, its client (a ScriptResourceClient) may or may not be added as
a client of the ScriptResource, depending on whether or not a streamer is
used. If a streamer is not used, then it's a direct client (which means
that in this scenario watchForLoad will call notifyFinished if the script
is already loaded); if it is, then PendingScript emits the callbacks
instead of the resource (when the streamer finishes), but it doesn't
perfectly emulate ScriptResource (it doesn't call the finished method when
the client is added if the resource is already finished, and it doesn't
call other methods like debugName and notifyAppendData).

This patch makes a narrower PendingScriptClient interface for clients of
the PendingScript to use, and PendingScript handles dispatching to the
client, rather than sometimes delegating this responsibility to the
resource itself.

This also simplifies some code which was using the Resource* to look up
the PendingScript*, which can now be provided directly to the client
method.

Committed: https://crrev.com/fda40a4d70c58633f45979b58efb23ee213f7802
Cr-Commit-Position: refs/heads/master@{#438433}

[jbroman, 2016-12-07 23:26:45]

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

[commit-bot: I haz the power, 2016-12-07 23:27:14]

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-08 00:53:32]

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

[commit-bot: I haz the power, 2016-12-08 00:53:33]

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

[jbroman, 2016-12-09 01:14:35]

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

[commit-bot: I haz the power, 2016-12-09 01:15:07]

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

[jbroman, 2016-12-09 01:32:50]

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

[commit-bot: I haz the power, 2016-12-09 01:33:32]

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-09 03:50:41]

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

[commit-bot: I haz the power, 2016-12-09 03:50:42]

Dry run: This issue passed the CQ dry run.

[jbroman, 2016-12-09 15:57:12]

Description was changed from

==========
Create PendingScriptClient as a separate client interface for PendingScript.

TODO(jbroman): More CL description.

BUG=
==========

to

==========
Create PendingScriptClient as a separate client interface for PendingScript.

This is aimed toward making PendingScript's relationship with its client
more clear and consistent.

Currently, its client (a ScriptResourceClient) may or may not be added as
a client of the ScriptResource, depending on whether or not a streamer is
used. If a streamer is not used, then it's a direct client (which means
that in this scenario watchForLoad will call notifyFinished if the script
is already loaded); if it is, then PendingScript emits the callbacks
instead of the resource (when the streamer finishes), but it doesn't
perfectly emulate ScriptResource (it doesn't call the finished method when
the client is added if the resource is already finished, and it doesn't
call other methods like debugName and notifyAppendData).

This patch makes a narrower PendingScriptClient interface for clients of
the PendingScript to use, and PendingScript handles dispatching to the
client, rather than sometimes delegating this responsibility to the
resource itself.

This also simplifies some code which was using the Resource* to look up
the PendingScript*, which can now be provided directly to the client
method.
==========

[jbroman, 2016-12-09 15:59:34]

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

[jbroman, 2016-12-09 15:59:35]

I found this division in who calls the ScriptResourceClient of a PendingScript
confusing; to me this seems simpler. WDYT?

[kouhei (in TOK), 2016-12-12 01:37:06]

lgtm

This is a lot simpler.

https://codereview.chromium.org/2549143009/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/PendingScript.cpp (right):

https://codereview.chromium.org/2549143009/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/PendingScript.cpp:93:
m_client->pendingScriptFinished(this);
Wow. Thanks for this change. Old code abusing ScriptResourceClient interface
looks very scary.

[jbroman, 2016-12-13 15:42:17]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2016-12-13 15:42:34]

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-13 15:51:08]

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

[commit-bot: I haz the power, 2016-12-13 15:51:09]

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

[jbroman, 2016-12-13 15:56:18]

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

[jbroman, 2016-12-13 15:56:18]

+haraken for ScriptStreamerTest.cpp

Just a replace of TestScriptResourceClient with TestPendingScriptClient.

[haraken, 2016-12-14 02:15:07]

On 2016/12/13 15:56:18, jbroman wrote:
> +haraken for ScriptStreamerTest.cpp
> 
> Just a replace of TestScriptResourceClient with TestPendingScriptClient.

LGTM

[jbroman, 2016-12-14 04:04:11]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2016-12-14 04:04:28]

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-14 04:50:04]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481688251688000,
"parent_rev": "d5a5d840f4b6d7f84a852d9fd740c657bdb55da3", "commit_rev":
"f671d9d18600d54c922796e43a875560a1e43a50"}

[commit-bot: I haz the power, 2016-12-14 04:50:31]

Description was changed from

==========
Create PendingScriptClient as a separate client interface for PendingScript.

This is aimed toward making PendingScript's relationship with its client
more clear and consistent.

Currently, its client (a ScriptResourceClient) may or may not be added as
a client of the ScriptResource, depending on whether or not a streamer is
used. If a streamer is not used, then it's a direct client (which means
that in this scenario watchForLoad will call notifyFinished if the script
is already loaded); if it is, then PendingScript emits the callbacks
instead of the resource (when the streamer finishes), but it doesn't
perfectly emulate ScriptResource (it doesn't call the finished method when
the client is added if the resource is already finished, and it doesn't
call other methods like debugName and notifyAppendData).

This patch makes a narrower PendingScriptClient interface for clients of
the PendingScript to use, and PendingScript handles dispatching to the
client, rather than sometimes delegating this responsibility to the
resource itself.

This also simplifies some code which was using the Resource* to look up
the PendingScript*, which can now be provided directly to the client
method.
==========

to

==========
Create PendingScriptClient as a separate client interface for PendingScript.

This is aimed toward making PendingScript's relationship with its client
more clear and consistent.

Currently, its client (a ScriptResourceClient) may or may not be added as
a client of the ScriptResource, depending on whether or not a streamer is
used. If a streamer is not used, then it's a direct client (which means
that in this scenario watchForLoad will call notifyFinished if the script
is already loaded); if it is, then PendingScript emits the callbacks
instead of the resource (when the streamer finishes), but it doesn't
perfectly emulate ScriptResource (it doesn't call the finished method when
the client is added if the resource is already finished, and it doesn't
call other methods like debugName and notifyAppendData).

This patch makes a narrower PendingScriptClient interface for clients of
the PendingScript to use, and PendingScript handles dispatching to the
client, rather than sometimes delegating this responsibility to the
resource itself.

This also simplifies some code which was using the Resource* to look up
the PendingScript*, which can now be provided directly to the client
method.

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

[commit-bot: I haz the power, 2016-12-14 04:50:33]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-14 04:52:34]

Description was changed from

==========
Create PendingScriptClient as a separate client interface for PendingScript.

This is aimed toward making PendingScript's relationship with its client
more clear and consistent.

Currently, its client (a ScriptResourceClient) may or may not be added as
a client of the ScriptResource, depending on whether or not a streamer is
used. If a streamer is not used, then it's a direct client (which means
that in this scenario watchForLoad will call notifyFinished if the script
is already loaded); if it is, then PendingScript emits the callbacks
instead of the resource (when the streamer finishes), but it doesn't
perfectly emulate ScriptResource (it doesn't call the finished method when
the client is added if the resource is already finished, and it doesn't
call other methods like debugName and notifyAppendData).

This patch makes a narrower PendingScriptClient interface for clients of
the PendingScript to use, and PendingScript handles dispatching to the
client, rather than sometimes delegating this responsibility to the
resource itself.

This also simplifies some code which was using the Resource* to look up
the PendingScript*, which can now be provided directly to the client
method.

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

to

==========
Create PendingScriptClient as a separate client interface for PendingScript.

This is aimed toward making PendingScript's relationship with its client
more clear and consistent.

Currently, its client (a ScriptResourceClient) may or may not be added as
a client of the ScriptResource, depending on whether or not a streamer is
used. If a streamer is not used, then it's a direct client (which means
that in this scenario watchForLoad will call notifyFinished if the script
is already loaded); if it is, then PendingScript emits the callbacks
instead of the resource (when the streamer finishes), but it doesn't
perfectly emulate ScriptResource (it doesn't call the finished method when
the client is added if the resource is already finished, and it doesn't
call other methods like debugName and notifyAppendData).

This patch makes a narrower PendingScriptClient interface for clients of
the PendingScript to use, and PendingScript handles dispatching to the
client, rather than sometimes delegating this responsibility to the
resource itself.

This also simplifies some code which was using the Resource* to look up
the PendingScript*, which can now be provided directly to the client
method.

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

[commit-bot: I haz the power, 2016-12-14 04:52:36]

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