WebFonts: classify memory cache hit case in WebFont.CacheHit metric

WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.

Committed: https://crrev.com/265b136995c55d4a1abcf82f8ad24c192d27af24
Cr-Commit-Position: refs/heads/master@{#415783}

[Takashi Toyoshima, 2016-08-30 10:53:10]

Description was changed from

==========
WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.
==========

to

==========
WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.
==========

[Takashi Toyoshima, 2016-08-30 10:53:11]

toyoshim@chromium.org changed reviewers:
+ ksakamoto@chromium.org

[Takashi Toyoshima, 2016-08-30 10:53:24]

Can you take a look?

[Kunihiko Sakamoto, 2016-08-31 03:27:17]

lgtm, thanks for doing this.

[Takashi Toyoshima, 2016-08-31 05:10:43]

toyoshim@chromium.org changed reviewers:
+ isherman@chromium.org, tkent@chromium.org

[Takashi Toyoshima, 2016-08-31 05:11:30]

Requesting OWNERS reviews.
+tkent for WebKit
+isherman for metrics

[tkent, 2016-08-31 05:21:30]

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right):

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:273: :
isLoadedFromMemoryCache ? MemoryHit
Why don't you refer to m_isLoadedFromMemoryCache directly?

[Takashi Toyoshima, 2016-08-31 05:31:50]

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right):

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:273: :
isLoadedFromMemoryCache ? MemoryHit
Because here is inside an inner class, and this instance is instantiated without
a reference to the parent, RemoteFontFaceSource class instance.

[Takashi Toyoshima, 2016-08-31 05:38:09]

Let me change a little

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right):

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:271: int
histogramValue = font->url().protocolIsData() ? DataUrl
From a chat with yhirano, I should check isLoadedFromMemoryCache first because
wasCached() could be both true and false even if isLoadedFromMemoryCache is
true. wasCached() just returns cached information about the disk cache for
memory cache hit cases.

[Takashi Toyoshima, 2016-08-31 05:43:17]

PTAL.

Patch Set 2 changes the condition check order to classify cache hit cases
correctly.
wasCached() can return true even if it comes from memory cache when data comes
from disk cache, and will return false memory cache holds data from network.

[Kunihiko Sakamoto, 2016-08-31 05:44:33]

still lgtm

[tkent, 2016-08-31 06:21:33]

lgtm

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp (right):

https://codereview.chromium.org/2289113002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp:273: :
isLoadedFromMemoryCache ? MemoryHit
On 2016/08/31 at 05:31:50, toyoshim wrote:
> Because here is inside an inner class, and this instance is instantiated
without a reference to the parent, RemoteFontFaceSource class instance.

I wasn't aware of it.  Got it.

[Ilya Sherman, 2016-08-31 19:11:08]

histograms.xml lgtm

[Takashi Toyoshima, 2016-08-31 20:34:34]

The CQ bit was checked by toyoshim@chromium.org

[commit-bot: I haz the power, 2016-08-31 20:35:10]

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-31 22:30:26]

Description was changed from

==========
WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.
==========

to

==========
WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.
==========

[commit-bot: I haz the power, 2016-08-31 22:30:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-31 22:32:38]

Description was changed from

==========
WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.
==========

to

==========
WebFonts: classify memory cache hit case in WebFont.CacheHit metric

wasCached() does not contain memory cache hit case. But WebFonts
intervention study's results imply memory cache hit case is not
minor group, but probably bigger one than Miss group.

This patch adds one additional group MemoryHit to WebFoint.CacheHit
metric to ensure memory cache hit ratio.

Committed: https://crrev.com/265b136995c55d4a1abcf82f8ad24c192d27af24
Cr-Commit-Position: refs/heads/master@{#415783}
==========

[commit-bot: I haz the power, 2016-08-31 22:32:40]

Patchset 2 (id:??) landed as
https://crrev.com/265b136995c55d4a1abcf82f8ad24c192d27af24
Cr-Commit-Position: refs/heads/master@{#415783}