Reland: Refactor Script(Loader|Runner): don't access Resources all over the place...

Reland: Refactor Script(Loader|Runner): don't access Resources all over the place...

... but use proper layering instead.

PendingScript holds a Resource, and many places in the code are using both the
PendingScript and the underlying Resource. For example, ScriptRunner holds a
collection of PendingScripts, and ScriptLoaders then listen to the underlying
Resources. This is confusing and error prone.

The Resource - ScriptLoader - PendingScript - Element love quadrangle is messy
in general.  Both ScriptLoader and PendingScript refer to the Resource. The way
to get ScriptLoader from PendingScript is via Element which PendingScript also
holds. But there's no way to get PendingScript from ScriptLoader.

This CL makes ScriptRunner access the ScriptLoaders directly and moves the
PendingScript to ScriptLoader. In addition, it makes ScriptRunner not access the
Resource of the ScriptLoader directly.

This CL also cancels the hack from https://bugs.webkit.org/show_bug.cgi?id=92211
(r139942) where it was speculated that a crash can be caused by ScriptLoader
getting the notifyFinished notification twice because it doesn't unsubscribe yet
when it gets the first one. The speculative fix was to made it nullify
ScriptLoader::m_resource and check for the nullity later. This is pretty
surprising, since the obvious solution would've been to just unsubscribe early
enough (which is done by this CL).

Note a behavioral change: It's OK to call Resource::addClient even if the
Resource is finished, and in that case, the client will be notified
immediately. But PendingScript used to ASSERT that the Resource is not
finished. This CL removes that restriction (the underlying Resource already
handles adding clients to finished resources + made ScriptStreamer handle it
too).

First version: https://codereview.chromium.org/656113002/

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183984

[marja, 2014-10-20 11:54:51]

marja@chromium.org changed reviewers:
+ haraken@chromium.org, jochen@chromium.org

[marja, 2014-10-20 11:54:51]

marja@chromium.org changed required reviewers:
+ jochen@chromium.org

[marja, 2014-10-20 11:54:51]

jochen, ptal at this fixed version. (haraken@, FYI)

This is a reincarnation of https://codereview.chromium.org/656113002/

To ease the review, the first patch set is the original version, and the fixes
are in the second patch set.

The fixes are:

1) PendingScript::watchForLoad(), set m_watchingForLoad earlier, because
addClient() can result in calling notifyFinished() and further
stopWatchingForLoad().

2) If the loading fails, we need to detach the ScriptLoader from the
PendingScript, so that the Element can get deleted too. <<< this one caused the
leaks! Before this CL, we used to delete the corresponding PendingScript in
ScriptRunner::notifyScriptLoadError, but in the buggy version of this CL, we
didn't.

3) PendingScript::stopWatchingForLoad(), unifying the Resource and
ScriptStreamer behaviors; here we should just stop watching, not cancel
anything.

[jochen (gone - plz use gerrit), 2014-10-20 11:59:17]

jochen@chromium.org changed required reviewers:
- jochen@chromium.org

[jochen (gone - plz use gerrit), 2014-10-20 11:59:18]

lgtm

[marja, 2014-10-20 12:00:02]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2014-10-20 12:01:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/669603002/20001

[marja, 2014-10-20 12:03:08]

The CQ bit was unchecked by marja@chromium.org

[marja, 2014-10-20 12:16:00]

(I added a tiny fix: since PendingScript::stopWatchingForLoad no longer cancels
the streaming, I made HTMLScriptRunner::detach() call
PendingScript::releaseElementAndClear which does. This is also consistent with
the idea of detaching.)

[marja, 2014-10-20 12:16:42]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2014-10-20 12:16:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/669603002/40001

[commit-bot: I haz the power, 2014-10-20 12:55:09]

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

[commit-bot: I haz the power, 2014-10-20 12:55:11]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

[marja, 2014-10-20 13:04:49]

The CQ bit was checked by marja@chromium.org

[commit-bot: I haz the power, 2014-10-20 13:06:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/669603002/60001

[haraken, 2014-10-20 13:52:31]

LGTM

[commit-bot: I haz the power, 2014-10-20 14:24:03]

Committed patchset #4 (id:60001) as 183984