Make Resource MemoryCoordinatorClient.

Make Resource MemoryCoordinatorClient:

- implement prepareToSuspend for background renderer's purge + suspend. prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJvI/edit?usp=sharing

BUG=635419
Committed: https://crrev.com/17888b419ca845a6762aae5287b449424997cc88
Cr-Commit-Position: refs/heads/master@{#414425}

[tasak, 2016-08-17 04:47:56]

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

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

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:38:49]

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

[commit-bot: I haz the power, 2016-08-17 06:38:51]

Dry run: This issue passed the CQ dry run.

[tasak, 2016-08-17 08:08:58]

Description was changed from

==========
Prune CachedMetadata when Resource is pruned.

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

to

==========
Prune CachedMetadata when Resource is pruned.

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

[tasak, 2016-08-17 08:10:51]

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

[tasak, 2016-08-17 08:10:51]

Would you review this CL?

[haraken, 2016-08-17 08:15:54]

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

[haraken, 2016-08-17 08:15:55]

LGTM on my side.

yhirano@: Does it look good?

[yhirano, 2016-08-17 09:00:11]

I guess clients may assume a cached metadata will not be cleared if they retain
the resource and other clients don't clear the metadata. Resource::unlock()
rejects to unlock |m_data| if there are clients, don't you need similar logic?

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

Thank you.

On 2016/08/17 09:00:11, yhirano wrote:
> I guess clients may assume a cached metadata will not be cleared if they
retain
> the resource and other clients don't clear the metadata. Resource::unlock()
> rejects to unlock |m_data| if there are clients, don't you need similar logic?

I see. I agree with you.
I will refactor V8ScriptRunner and ScriptSteamer to lock CachedMetadata while
the classes are using CachedMetadata.
I will also add unit tests (to check that reallly CachedMetadata is purged and
really CachedMetadata is created again).

[tasak, 2016-08-18 09:44:36]

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

[commit-bot: I haz the power, 2016-08-18 09:44:49]

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-18 11:15:02]

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

[commit-bot: I haz the power, 2016-08-18 11:15:03]

Dry run: This issue passed the CQ dry run.

[tasak, 2016-08-23 05:02:04]

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

[commit-bot: I haz the power, 2016-08-23 05:02:28]

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

[tasak, 2016-08-23 05:02:37]

Patchset #2 (id:20001) has been deleted

[tasak, 2016-08-23 06:03:01]

Description was changed from

==========
Prune CachedMetadata when Resource is pruned.

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

to

==========
Make Resource MemoryCoordinatorClient.

- implemented purgeToSuspend to clear CachedMetadata before suspend a renderer.

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

[tasak, 2016-08-23 06:03:43]

Description was changed from

==========
Make Resource MemoryCoordinatorClient.

- implemented purgeToSuspend to clear CachedMetadata before suspend a renderer.

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

to

==========
Make Resource MemoryCoordinatorClient.

- implemented purgeToSuspend to clear CachedMetadata before suspend a renderer.
- protected CachedMetadata while V8ScriptRunner is using the cache.

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

[tasak, 2016-08-23 06:03:59]

Description was changed from

==========
Make Resource MemoryCoordinatorClient.

- implemented purgeToSuspend to clear CachedMetadata before suspend a renderer.
- protected CachedMetadata while V8ScriptRunner is using the cache.

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

to

==========
Make Resource MemoryCoordinatorClient:

- implement purgeToSuspend to clear CachedMetadata before suspend a renderer.
- protect CachedMetadata while V8ScriptRunner is using the cache.

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

[tasak, 2016-08-23 06:05:21]

Patchset #2 (id:40001) has been deleted

[tasak, 2016-08-23 06:05:32]

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

[commit-bot: I haz the power, 2016-08-23 06:05:53]

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

[tasak, 2016-08-23 07:31:44]

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

[commit-bot: I haz the power, 2016-08-23 07:31:57]

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-23 09:33:52]

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

[commit-bot: I haz the power, 2016-08-23 09:33:52]

Dry run: This issue passed the CQ dry run.

[tasak, 2016-08-23 11:12:30]

I updated the patch to protect CachedMetadata, i.e. RefPtr<CachedMetadata>
codeCache(); ....

So would you review the newest patch?

[tasak, 2016-08-24 11:47:37]

Description was changed from

==========
Make Resource MemoryCoordinatorClient:

- implement purgeToSuspend to clear CachedMetadata before suspend a renderer.
- protect CachedMetadata while V8ScriptRunner is using the cache.

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

to

==========
Make Resource MemoryCoordinatorClient:


- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

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

[tasak, 2016-08-24 11:47:46]

Description was changed from

==========
Make Resource MemoryCoordinatorClient:


- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

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

to

==========
Make Resource MemoryCoordinatorClient:

- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

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

[tasak, 2016-08-24 11:53:13]

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

[commit-bot: I haz the power, 2016-08-24 11:53:37]

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

[tasak, 2016-08-24 11:53:47]

tasak@google.com changed reviewers:
+ hiroshige@chromium.org

[tasak, 2016-08-24 11:53:48]

hiroshige@, would you review this CL?

[commit-bot: I haz the power, 2016-08-24 13:28:40]

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

[commit-bot: I haz the power, 2016-08-24 13:28:41]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-08-25 01:25:12]

lgtm

https://codereview.chromium.org/2249373002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/CachedMetadataHandler.h (right):

https://codereview.chromium.org/2249373002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/CachedMetadataHandler.h:30: // Returns
cached metadata of the given type associated with this resource.
Please add a comment saying that this metadata can be pruned at any time.

[tasak, 2016-08-25 10:32:52]

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

[commit-bot: I haz the power, 2016-08-25 10:33:06]

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

[tasak, 2016-08-25 10:33:50]

Thank you.

https://codereview.chromium.org/2249373002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/CachedMetadataHandler.h (right):

https://codereview.chromium.org/2249373002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/CachedMetadataHandler.h:30: // Returns
cached metadata of the given type associated with this resource.
On 2016/08/25 01:25:12, yhirano wrote:
> Please add a comment saying that this metadata can be pruned at any time.

Done.

[tasak, 2016-08-25 10:34:07]

The CQ bit was unchecked by tasak@google.com

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

The CQ bit was checked by tasak@google.com

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

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2249373002/#ps120001
(title: "Added comment")

[commit-bot: I haz the power, 2016-08-25 10:34:40]

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

[hiroshige, 2016-08-25 12:48:11]

Can we add tests to prepareToSuspend()?

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp (right):

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/ScriptStreamer.cpp:293: if
(codeCache.get())  {
We can remove .get() here.

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp (right):

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp:182: return
codeCache.get()
We can remove .get() here.

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/Resource.cpp:156: return
m_cachedMetadata.get();
We can remove .get() here.

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerScriptCachedMetadataHandler.cpp
(right):

https://codereview.chromium.org/2249373002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerScriptCachedMetadataHandler.cpp:52:
return m_cachedMetadata.get();
We can remove .get() here.

[commit-bot: I haz the power, 2016-08-25 14:00:15]

Description was changed from

==========
Make Resource MemoryCoordinatorClient:

- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

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

to

==========
Make Resource MemoryCoordinatorClient:

- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

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

[commit-bot: I haz the power, 2016-08-25 14:00:16]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-08-25 14:02:00]

Description was changed from

==========
Make Resource MemoryCoordinatorClient:

- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

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

to

==========
Make Resource MemoryCoordinatorClient:

- implement prepareToSuspend for background renderer's purge + suspend.
prepareToSuspend is invoked before a background renderer is suspended.
- intent-to-implement-and-ship of background renderer's purge + suspend is
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...

BUG=635419

Committed: https://crrev.com/17888b419ca845a6762aae5287b449424997cc88
Cr-Commit-Position: refs/heads/master@{#414425}
==========

[commit-bot: I haz the power, 2016-08-25 14:02:01]

Patchset 5 (id:??) landed as
https://crrev.com/17888b419ca845a6762aae5287b449424997cc88
Cr-Commit-Position: refs/heads/master@{#414425}