ChromeOS should record FileBrowser.CreateNewFolder.... events

ChromeOS should record FileBrowser.CreateNewFolder.... events

Added UMA user action implemented in http://codereview.chromium.org/8490014/
to extract_actions.py, updated chromeactions.txt.

Note: when I ran extract_actions.py --hash I saw a number of new actions show up
in chromeactions.txt. Someone has probably forgot to update it.
I am only adding the one line related to my action in this patch.

BUG=chromium-os:22860
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110303

[Vladislav Kaznacheev, 2011-11-11 11:17:32]

Hi,

I hope I am sending this patch to the right people (I checked the most recent
CLs similar to this one).

The actual event is generated in Javascript and send to
chrome.experimental.metrics which calls recordComputedEvent, so this patch only
touches these two files.

Best regards,

  Vlad

[Ilya Sherman, 2011-11-11 23:39:20]

> Note: when I ran extract_actions.py --hash I saw a number of new actions
> show up in chromeactions.txt. Someone has probably forgot to update it. 
> I am only adding the one line related to my action in this patch. 

Please go ahead and add all of them.  People sometimes forget to run the script
when they add new metrics, and it's better to add them late than never.

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py
File chrome/tools/extract_actions.py (right):

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
Can this be named "FileBrowser.CreateNewFolder" instead, without the extension
name appended?

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
Hmm, I'm confused.  It looks like [1] calls metrics.recordAction() both for
"Create.*" and for "CreateNewFolder".  How does one of these generate a
histogram, and the other one generate a "user action" count?

[1]
http://codereview.chromium.org/8490014/diff/4001/chrome/browser/resources/fil...

[Vladislav Kaznacheev, 2011-11-14 10:47:12]

I have added the missing actions.

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py
File chrome/tools/extract_actions.py (right):

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
I should have mentioned one more CL (http://codereview.chromium.org/8491038). It
changes "Create.<type>" user action into "Create" histogram with type as a
value.

Sorry about the confusion. 

On 2011/11/11 23:39:20, Ilya Sherman wrote:
> Hmm, I'm confused.  It looks like [1] calls metrics.recordAction() both for
> "Create.*" and for "CreateNewFolder".  How does one of these generate a
> histogram, and the other one generate a "user action" count?
> 
> [1]
>
http://codereview.chromium.org/8490014/diff/4001/chrome/browser/resources/fil...

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
My Javascript code uses chrome.experimental.metrics API. The current
implementation appends the extension name unconditionally in order to guarantee
unique names across a (potentially) large number of extensions that could use
the API in the future.

In reality my extension is the first client of this API with the second client
nowhere in sight. I have a permission from the API developers to change the
implementation within reason.

Personally I think extension names are ugly. I think that we can get rid of them
and check for possible name clashes when adding a new action to
chromeactions.txt

On 2011/11/11 23:39:20, Ilya Sherman wrote:
> Can this be named "FileBrowser.CreateNewFolder" instead, without the extension
> name appended?

[Ilya Sherman, 2011-11-14 12:08:13]

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py
File chrome/tools/extract_actions.py (right):

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
Ah, that makes more sense now.  Thanks for the clarification.

On 2011/11/14 10:47:12, Vladislav Kaznacheev wrote:
> I should have mentioned one more CL (http://codereview.chromium.org/8491038).
It
> changes "Create.<type>" user action into "Create" histogram with type as a
> value.
> 
> Sorry about the confusion. 
> 
> On 2011/11/11 23:39:20, Ilya Sherman wrote:
> > Hmm, I'm confused.  It looks like [1] calls metrics.recordAction() both for
> > "Create.*" and for "CreateNewFolder".  How does one of these generate a
> > histogram, and the other one generate a "user action" count?
> > 
> > [1]
> >
>
http://codereview.chromium.org/8490014/diff/4001/chrome/browser/resources/fil...
>

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
Is the chrome.experimental.metrics API intended to be used only be extensions
shipped as part of the Chromium source, or by external extensions as well?

If it is intended to be used only be extensions that are directly included as
part of Chromium, we should definitely not be appending the extensions names. 
Name clashes in such extensions' metrics do not seem any more likely than name
clashes everywhere else where we log metrics, so there's no need to do extra
work to avoid them for this particular case.

If the API is intended to be used by external extensions as well, it's probably
a good idea to have a chat with at least Jim Roskind (jar@) about the design of
this API and how third party extensions could make use of it (since metrics are
generally not released externally).

On 2011/11/14 10:47:12, Vladislav Kaznacheev wrote:
> My Javascript code uses chrome.experimental.metrics API. The current
> implementation appends the extension name unconditionally in order to
guarantee
> unique names across a (potentially) large number of extensions that could use
> the API in the future.
> 
> In reality my extension is the first client of this API with the second client
> nowhere in sight. I have a permission from the API developers to change the
> implementation within reason.
> 
> Personally I think extension names are ugly. I think that we can get rid of
them
> and check for possible name clashes when adding a new action to
> chromeactions.txt
> 
> On 2011/11/11 23:39:20, Ilya Sherman wrote:
> > Can this be named "FileBrowser.CreateNewFolder" instead, without the
extension
> > name appended?

[petkov, 2011-11-14 13:25:27]

http://codereview.chromium.org/8537008/diff/2001/chrome/tools/extract_actions.py
File chrome/tools/extract_actions.py (right):

http://codereview.chromium.org/8537008/diff/2001/chrome/tools/extract_actions...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
Is this user action reported by the Chrome process or a non-Chrome process? I'm
not sure that this function (as is) is the right place to add the action (see
the docstring) -- it was meant for user actions that came through ChromeOS's
libmetrics API which are processed by Chrome's ChromeOS-specific
external_metrics.cc.

If there's no better place to add the new action you could either change the
docstring or add a new function. Is the FileBrowser really ChromeOS-specific?

[Vladislav Kaznacheev, 2011-11-14 13:53:19]

http://codereview.chromium.org/8537008/diff/2001/chrome/tools/extract_actions.py
File chrome/tools/extract_actions.py (right):

http://codereview.chromium.org/8537008/diff/2001/chrome/tools/extract_actions...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
The File Browser is ChromeOS specific. It lives in the Chrome codebase and runs
as a Chrome extension but it is only enabled in Chrome OS.

I have added a dedicated function for this kind of actions.

On 2011/11/14 13:25:27, petkov wrote:
> Is this user action reported by the Chrome process or a non-Chrome process?
I'm
> not sure that this function (as is) is the right place to add the action (see
> the docstring) -- it was meant for user actions that came through ChromeOS's
> libmetrics API which are processed by Chrome's ChromeOS-specific
> external_metrics.cc.
> 
> If there's no better place to add the new action you could either change the
> docstring or add a new function. Is the FileBrowser really ChromeOS-specific?

[petkov, 2011-11-14 14:03:09]

Just the name thing, everything else looks OK to me.

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py
File chrome/tools/extract_actions.py (right):

http://codereview.chromium.org/8537008/diff/1/chrome/tools/extract_actions.py...
chrome/tools/extract_actions.py:227:
actions.add('FileBrowser.CreateNewFolder.hhaomjibdihmijegdhdafkllkbggdgoj')
jar@ should definitely take a look. I'd think that we shouldn't expose metrics
to external extensions because the metrics data is not visible outside Google.
That would mean that we don't have to worry about name collisions.

On 2011/11/14 12:08:14, Ilya Sherman wrote:
> Is the chrome.experimental.metrics API intended to be used only be extensions
> shipped as part of the Chromium source, or by external extensions as well?
> 
> If it is intended to be used only be extensions that are directly included as
> part of Chromium, we should definitely not be appending the extensions names. 
> Name clashes in such extensions' metrics do not seem any more likely than name
> clashes everywhere else where we log metrics, so there's no need to do extra
> work to avoid them for this particular case.
> 
> If the API is intended to be used by external extensions as well, it's
probably
> a good idea to have a chat with at least Jim Roskind (jar@) about the design
of
> this API and how third party extensions could make use of it (since metrics
are
> generally not released externally).
> 
> On 2011/11/14 10:47:12, Vladislav Kaznacheev wrote:
> > My Javascript code uses chrome.experimental.metrics API. The current
> > implementation appends the extension name unconditionally in order to
> guarantee
> > unique names across a (potentially) large number of extensions that could
use
> > the API in the future.
> > 
> > In reality my extension is the first client of this API with the second
client
> > nowhere in sight. I have a permission from the API developers to change the
> > implementation within reason.
> > 
> > Personally I think extension names are ugly. I think that we can get rid of
> them
> > and check for possible name clashes when adding a new action to
> > chromeactions.txt
> > 
> > On 2011/11/11 23:39:20, Ilya Sherman wrote:
> > > Can this be named "FileBrowser.CreateNewFolder" instead, without the
> extension
> > > name appended?

[Vladislav Kaznacheev, 2011-11-16 11:25:40]

PTAL. Removed the extension name.

[petkov, 2011-11-16 11:33:10]

LGTM

[commit-bot: I haz the power, 2011-11-16 12:55:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaznacheev@chromium.org/8537008/10001

[commit-bot: I haz the power, 2011-11-16 15:24:29]

Change committed as 110303