[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
Committed: https://crrev.com/e46449c9825bf0891d9b2ba9d0bcaa6ce6cc959f
Cr-Commit-Position: refs/heads/master@{#397312}

[hiroshige, 2016-04-26 09:34:35]

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

[commit-bot: I haz the power, 2016-04-26 09:34:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/1

[hiroshige, 2016-04-26 09:36:33]

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

[commit-bot: I haz the power, 2016-04-26 09:36:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/20001

[commit-bot: I haz the power, 2016-04-26 11:00:35]

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

[commit-bot: I haz the power, 2016-04-26 11:00:36]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-04-26 12:59:00]

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

[hiroshige, 2016-04-26 12:59:01]

This is WIP CL for doing a Finch experiment to make references from MemoryCache
to Resource weak.
|gWeak| controls whether we should use WeakMember or not, and will be replaced
with runtime flag governed by the experiment.

Do you think we can land this to beta (or stable) and doing Finch experiments
there?

[hiroshige, 2016-04-26 13:03:48]

On 2016/04/26 12:59:01, hiroshige wrote:
> This is WIP CL for doing a Finch experiment to make references from
MemoryCache
> to Resource weak.
> |gWeak| controls whether we should use WeakMember or not, and will be replaced
> with runtime flag governed by the experiment.
> 
> Do you think we can land this to beta (or stable) and doing Finch experiments
> there?

Context: the design doc 
https://docs.google.com/document/d/1Pj4vzxzCKC_11SugdJ_x7wq7ErE-pHtf4w4FLKdcF...
receives some comments recommending Finch, and I'd like to determine the
feasibility of Finch experiments on beta/stable, while I think we need to switch
strong/weak references by runtime flag like this CL.

[Nate Chapin, 2016-04-28 16:45:21]

On 2016/04/26 12:59:01, hiroshige wrote:
> This is WIP CL for doing a Finch experiment to make references from
MemoryCache
> to Resource weak.
> |gWeak| controls whether we should use WeakMember or not, and will be replaced
> with runtime flag governed by the experiment.
> 
> Do you think we can land this to beta (or stable) and doing Finch experiments
> there?

We should land it on trunk first, no matter what. We might(?) be able to merge
to beta, but I'd be surprised about merging to stable. I'm not sure what the
rules are.

[hiroshige, 2016-05-10 09:22:15]

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

[commit-bot: I haz the power, 2016-05-10 09:22:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/60001

[hiroshige, 2016-05-10 10:02:01]

Description was changed from

==========
[Weak MemoryCache 1b] Make Reference from MemoryCache to Resource weak (Finch)

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

to

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL
- Adds a runtime flag |MemoryCache::m_isWeak| (unchanged during execution),
- Switches whether the references from MemoryCache to Resource are weak or not
  based on the flag, and
- Makes the flag controlled by a Field Trial.

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

[hiroshige, 2016-05-10 10:53:08]

Description was changed from

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL
- Adds a runtime flag |MemoryCache::m_isWeak| (unchanged during execution),
- Switches whether the references from MemoryCache to Resource are weak or not
  based on the flag, and
- Makes the flag controlled by a Field Trial.

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

to

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
  Tests are modified:
  - CSSStyleSheetResourceTest (a client is added to keep Resource alive)
  - http/tests/inspector/network/network-memory-cached-resource.html
    (an image is used instead of a stylesheet to keep Resource alive)
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

[hiroshige, 2016-05-10 10:53:15]

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

[commit-bot: I haz the power, 2016-05-10 10:53:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/120001

[commit-bot: I haz the power, 2016-05-10 12:02:00]

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

[commit-bot: I haz the power, 2016-05-10 12:02:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hiroshige, 2016-05-11 10:23:48]

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

[commit-bot: I haz the power, 2016-05-11 10:24:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/140001

[commit-bot: I haz the power, 2016-05-11 10:25:56]

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

[commit-bot: I haz the power, 2016-05-11 10:25:58]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[hiroshige, 2016-05-12 09:30:22]

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

[commit-bot: I haz the power, 2016-05-12 09:30:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/160001

[commit-bot: I haz the power, 2016-05-12 10:55:34]

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

[commit-bot: I haz the power, 2016-05-12 10:55:35]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-05-17 09:11:45]

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

[hiroshige, 2016-05-17 09:11:46]

PTAL.

I'm planning to land this to M-53.

[yhirano, 2016-05-19 07:39:22]

Don't you need resources.grd and about_flags.cc changes for
DoNotUnlockSharedBuffer?

[hiroshige, 2016-05-26 06:15:19]

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

[commit-bot: I haz the power, 2016-05-26 06:15:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/200001

[commit-bot: I haz the power, 2016-05-26 06:31:28]

Dry run: None

[hiroshige, 2016-05-26 07:07:15]

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

[commit-bot: I haz the power, 2016-05-26 07:07:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/220001

[commit-bot: I haz the power, 2016-05-26 08:06:43]

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

[commit-bot: I haz the power, 2016-05-26 08:06:44]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hiroshige, 2016-05-27 07:46:41]

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

[commit-bot: I haz the power, 2016-05-27 07:46:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/240001

[commit-bot: I haz the power, 2016-05-27 07:48:46]

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

[commit-bot: I haz the power, 2016-05-27 07:48:48]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[hiroshige, 2016-05-27 08:10:54]

Patchset #13 (id:240001) has been deleted

[hiroshige, 2016-05-27 08:21:26]

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

[commit-bot: I haz the power, 2016-05-27 08:21:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/260001

[hiroshige, 2016-05-27 08:21:40]

Patchset #11 (id:200001) has been deleted

[hiroshige, 2016-05-27 08:21:54]

Patchset #11 (id:220001) has been deleted

[hiroshige, 2016-05-27 08:22:31]

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

[hiroshige, 2016-05-27 08:28:29]

On 2016/05/19 07:39:22, yhirano wrote:
> Don't you need resources.grd and about_flags.cc changes for
> DoNotUnlockSharedBuffer?

I thought DoNotUnlockSharedBuffer is too internal to add it as a chrome://flags
flag and expected less behavior changes for DoNotUnlockSharedBuffer, thus I
didn't added a flag for it.
But I don't have a strong preference about this.

Added +isherman@ (also as tools/metrics/ OWNER).

Also, I split out test changes.

[commit-bot: I haz the power, 2016-05-27 12:35:25]

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

[commit-bot: I haz the power, 2016-05-27 12:35:27]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[yhirano, 2016-05-30 08:13:24]

> I thought DoNotUnlockSharedBuffer is too internal to add
> it as a chrome://flags flag and expected less behavior changes
> for DoNotUnlockSharedBuffer, thus I didn't added a
> flag for it.
> But I don't have a strong preference about this.
OK, I don't have a strong opinion either.

Please update the issue description.

https://codereview.chromium.org/1915113005/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/MemoryCache.cpp (right):

https://codereview.chromium.org/1915113005/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/MemoryCache.cpp:65: :
m_inLiveDecodedResourcesList(false)
[optional] I prefer member initialization at their definitions if initialized
with constants.

https://codereview.chromium.org/1915113005/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/MemoryCache.cpp:394: Resource* resource =
entry->resource();
Can |resource| be null? If not, can you put an assertion?

[Ilya Sherman, 2016-05-31 23:20:22]

histograms.xml lgtm

[hiroshige, 2016-06-01 02:23:10]

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

[commit-bot: I haz the power, 2016-06-01 02:23:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/300001

[hiroshige, 2016-06-01 02:24:38]

Description was changed from

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
  Tests are modified:
  - CSSStyleSheetResourceTest (a client is added to keep Resource alive)
  - http/tests/inspector/network/network-memory-cached-resource.html
    (an image is used instead of a stylesheet to keep Resource alive)
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

to

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

[hiroshige, 2016-06-01 02:25:14]

Also I updated the CL description.

https://codereview.chromium.org/1915113005/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/MemoryCache.cpp (right):

https://codereview.chromium.org/1915113005/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/MemoryCache.cpp:65: :
m_inLiveDecodedResourcesList(false)
On 2016/05/30 08:13:24, yhirano wrote:
> [optional] I prefer member initialization at their definitions if initialized
> with constants.

This CL is to be reverted on canary, so I prefer not doing this at this time to
keep changes a little small.

https://codereview.chromium.org/1915113005/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/MemoryCache.cpp:394: Resource* resource =
entry->resource();
On 2016/05/30 08:13:24, yhirano wrote:
> Can |resource| be null? If not, can you put an assertion?

I think |resource| should be non-null here (because call sites contains direct
null checks of |resource()|, or null checks of getEntryForResource(), which
returns null when |resource()| is null).
I added an assertion.

[commit-bot: I haz the power, 2016-06-01 04:10:23]

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

[commit-bot: I haz the power, 2016-06-01 04:10:25]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-06-01 05:05:37]

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

[hiroshige, 2016-06-01 05:05:38]

+kinuko@ as an OWNER, particularly of
  content/
  third_party/WebKit/Source/platform/

Memo: other OWNERs:

japhet@ and kinuko@:
  third_party/WebKit/Source/core/fetch/
  third_party/WebKit/Source/web/
  third_party/WebKit/platform/web/

isherman@:
  tools/metrics/

*:
  chrome/

[yhirano, 2016-06-01 08:27:02]

lgtm

[Nate Chapin, 2016-06-01 23:56:37]

LGTM, but I think you'll still need kinuko's review for platform/OWNERS

[kinuko, 2016-06-02 01:51:51]

lgtm, thanks for making this work in Finch setting!

[hiroshige, 2016-06-02 05:01:34]

The CQ bit was checked by hiroshige@chromium.org

[hiroshige, 2016-06-02 05:01:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/1915113005/#ps300001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-06-02 05:01:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1915113005/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1915113005/300001

[commit-bot: I haz the power, 2016-06-02 06:27:53]

Description was changed from

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

to

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

[commit-bot: I haz the power, 2016-06-02 06:27:55]

Committed patchset #13 (id:300001)

[commit-bot: I haz the power, 2016-06-02 06:29:48]

Description was changed from

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462
==========

to

==========
[WeakMemoryCache] Field-Trial: Make Reference from MemoryCache to Resource weak

This CL adds two RuntimeEnabledFeatures controlled by Field Trials to evaluate
their feasibility:
- |WeakMemoryCache| switches whether the references from MemoryCache to
  Resource are weak or not.
  A corresponding flag |enable-weak-memorycache| is added to chrome://flags.
  Null checks are added to MemoryCache to handle weak pointers.
- |DoNotUnlockSharedBuffer| skips SharedBuffer::unlock() in Resource::unlock().

This CL will be reverted before going to stable (or even before going to beta).

BUG=603462

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

[commit-bot: I haz the power, 2016-06-02 06:29:50]

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