Add destroyDecodedDataIfPossible to ScriptResource to stop ScriptStreaming.

Add destroyDecodedDataIfPossible to ScriptResource to stop ScriptStreaming.

BUG=635419

[tasak, 2016-08-17 05:10:34]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 05:10:48]

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-08-17 06:41:32]

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

[commit-bot: I haz the power, 2016-08-17 06:41:33]

Dry run: This issue passed the CQ dry run.

[tasak, 2016-08-17 08:11:15]

Description was changed from

==========
Add destroyDecodedDataIfPossible to ScriptResource to stop ScriptStreaming.

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

to

==========
Add destroyDecodedDataIfPossible to ScriptResource to stop ScriptStreaming.

BUG=635419
==========

[tasak, 2016-08-17 08:13:34]

tasak@google.com changed reviewers:
+ haraken@chromium.org, marja@chromium.org

[tasak, 2016-08-17 08:13:35]

Would you review this CL?

[haraken, 2016-08-17 08:29:19]

LGTM on my side but I want to have marja@ confirm the change (i.e., if it's safe
to cancel script streaming when the system is under high memory pressure).

Also please write a CL description so that others can follow what this CL is
doing.

[marja, 2016-08-17 08:37:40]

lgtm % a comment

https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ScriptResource.cpp (right):

https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ScriptResource.cpp:74: void
ScriptResource::destroyDecodedDataIfPossible()
... and especially here... destroyDecodedDataIfPossible does not in fact destroy
any data.

So maybe a less surprising name would be "notifyCancel" or
"notifyCancelToClients" (or whatever the Blink naming convention is, I don't
remember).

https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/fetch/ScriptResource.h (right):

https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/fetch/ScriptResource.h:102: void
destroyDecodedDataIfPossible() override;
I find this name surprising. It's not clear what's the connection between
cancelling <-> destroying decoded data.

[haraken, 2016-08-17 08:44:43]

On 2016/08/17 08:37:40, marja wrote:
> lgtm % a comment
> 
>
https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/fetch/ScriptResource.cpp (right):
> 
>
https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/fetch/ScriptResource.cpp:74: void
> ScriptResource::destroyDecodedDataIfPossible()
> ... and especially here... destroyDecodedDataIfPossible does not in fact
destroy
> any data.
> 
> So maybe a less surprising name would be "notifyCancel" or
> "notifyCancelToClients" (or whatever the Blink naming convention is, I don't
> remember).
> 
>
https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/fetch/ScriptResource.h (right):
> 
>
https://codereview.chromium.org/2251893002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/fetch/ScriptResource.h:102: void
> destroyDecodedDataIfPossible() override;
> I find this name surprising. It's not clear what's the connection between
> cancelling <-> destroying decoded data.

I'd suggest using purgeMemory() throughout the code base.

[marja, 2016-08-17 08:46:44]

How high / deep through the levels would you route the name "purgeMemory"? Would
ScriptStreamer have purgeMemory? Who'd call ScriptResource::purgeMemory?

[tasak, 2016-08-17 09:53:50]

Thank you, marja.

On 2016/08/17 08:46:44, marja wrote:
> How high / deep through the levels would you route the name "purgeMemory"?
Would
> ScriptStreamer have purgeMemory? Who'd call ScriptResource::purgeMemory?

I'm sorry.
I discussed with hiroshige@ and I found that I modified MemoryCache::prune to
force to invoke the prune (in this case,
call path is MemoryCache::prune => Resource::prune() =>
ScriptResource::destroyDecodedDataIfPossible())

However, the change of MemoryCache::prune is not allowable and there is no way
to invoke the notifyCancel and so on.

I will think about how to invoke this cancel method and will add unittests
(hiroshige@ requested).
So I will close this issue and will recreate another patch, which solves these
issues.