histograms.xml for WebCore.Scripts.*

histograms.xml for WebCore.Scripts.*

The corresponding Blink side change is
https://codereview.chromium.org/531693002/.

BUG=

Committed: https://crrev.com/d19f6d7c5fa412b774d839773a1e59deba4f83e5
Cr-Commit-Position: refs/heads/master@{#293699}

[marja, 2014-09-02 08:42:25]

marja@chromium.org changed reviewers:
+ isherman@chromium.org, jwd@chromium.org
- jochen@chromium.org

[marja, 2014-09-02 08:42:26]

isherman, ptal

(jwd, you're listed as "owners in training", you're welcome to have a look too)

[Ilya Sherman, 2014-09-03 01:37:08]

LGTM % a nit.

https://codereview.chromium.org/531713002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/531713002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:36510: +<histogram
name="WebCore.Scripts.ParsingBlocking.AlreadyLoaded" enum="Boolean">
nit: Please use a more custom-tailored boolean enum with labels "Blocked" and
"Unblocked" (or "Not blocked", if you prefer).

[marja, 2014-09-04 14:54:11]

Thanks for having a look! Some naming discussion below:

https://codereview.chromium.org/531713002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/531713002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:36510: +<histogram
name="WebCore.Scripts.ParsingBlocking.AlreadyLoaded" enum="Boolean">
On 2014/09/03 01:37:08, Ilya Sherman wrote:
> nit: Please use a more custom-tailored boolean enum with labels "Blocked" and
> "Unblocked" (or "Not blocked", if you prefer).

Hmm, I'm not sure about the naming you suggest... so
"WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = "Blocked" would mean that the
script was not already loaded, and
"WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = "Not blocked / Unblocked" 
would mean that the script *was* already loaded? IMO this is confusing.

To clarify: "ParsingBlocking" is here because this monitors attributes of
"parsing blocking scripts". It does not monitor whether something was parsing
blocking or not; it monitors whether a parsing blocking script was already
loaded from the network when it was needed. Did you get the term "Blocked" from
"ParserBlocking"?

How about adding BooleanLoaded with "Loaded" / "Not loaded", and then
"WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = "Loaded" would mean that the
script was loaded...

Otoh, "WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = True expresses the same
thing. So IDK.

[Ilya Sherman, 2014-09-04 17:16:40]

https://codereview.chromium.org/531713002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/531713002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:36510: +<histogram
name="WebCore.Scripts.ParsingBlocking.AlreadyLoaded" enum="Boolean">
On 2014/09/04 14:54:10, marja wrote:
> On 2014/09/03 01:37:08, Ilya Sherman wrote:
> > nit: Please use a more custom-tailored boolean enum with labels "Blocked"
and
> > "Unblocked" (or "Not blocked", if you prefer).
> 
> Hmm, I'm not sure about the naming you suggest... so
> "WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = "Blocked" would mean that
the
> script was not already loaded, and
> "WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = "Not blocked / Unblocked" 
> would mean that the script *was* already loaded? IMO this is confusing.
> 
> To clarify: "ParsingBlocking" is here because this monitors attributes of
> "parsing blocking scripts". It does not monitor whether something was parsing
> blocking or not; it monitors whether a parsing blocking script was already
> loaded from the network when it was needed. Did you get the term "Blocked"
from
> "ParserBlocking"?
> 
> How about adding BooleanLoaded with "Loaded" / "Not loaded", and then
> "WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = "Loaded" would mean that the
> script was loaded...
> 
> Otoh, "WebCore.Scripts.ParsingBlocking.AlreadyLoaded" = True expresses the
same
> thing. So IDK.

Sorry, I meant "Loaded" / "Not loaded" -- not sure why I wrote "blocked" / "not
blocked".  It's not critical to add this enum, but IMO it's slightly nicer when
using the dashboard to have semantically rich labels so that I don't need to
glance back at the histogram name or <summary> to remind myself which case is
"true" and which case is "false".  LGTM either way.

[marja, 2014-09-08 08:23:39]

The CQ bit was checked by marja@chromium.org

[marja, 2014-09-08 08:24:02]

(added BooleanLoaded enum)

[commit-bot: I haz the power, 2014-09-08 08:24:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/marja@chromium.org/531713002/20001

[commit-bot: I haz the power, 2014-09-08 09:49:40]

Committed patchset #2 (id:20001) as e5968cb1666fa8c28b5186a433e3a327b9f54c4d

[commit-bot: I haz the power, 2014-09-10 03:45:30]

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