Add histgrams to Folder shortcut feature in Files.app.

Add histgrams to Folder shortcut feature in Files.app.

BUG=none
TEST=Files.app runs, the volume list on the left works.
R=asvitkine@chromium.org, mtomasz@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=217148

[mtomasz, 2013-08-09 07:50:47]

https://codereview.chromium.org/22634008/diff/1/chrome/browser/resources/file...
File chrome/browser/resources/file_manager/js/folder_shortcuts_data_model.js
(right):

https://codereview.chromium.org/22634008/diff/1/chrome/browser/resources/file...
chrome/browser/resources/file_manager/js/folder_shortcuts_data_model.js:187: 
nit: Remove this empty line.

[yoshiki, 2013-08-09 08:41:43]

@mtomasz: PTAL

https://codereview.chromium.org/22634008/diff/1/chrome/browser/resources/file...
File chrome/browser/resources/file_manager/js/folder_shortcuts_data_model.js
(right):

https://codereview.chromium.org/22634008/diff/1/chrome/browser/resources/file...
chrome/browser/resources/file_manager/js/folder_shortcuts_data_model.js:187: 
On 2013/08/09 07:50:47, mtomasz wrote:
> nit: Remove this empty line.

Done.

[mtomasz, 2013-08-12 02:54:56]

Sorry, I forgot to type lgtm. lgtm.

[commit-bot: I haz the power, 2013-08-12 03:18:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/22634008/5001

[commit-bot: I haz the power, 2013-08-12 03:18:59]

Failed to apply patch for
chrome/browser/resources/file_manager/js/navigation_list.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/resources/file_manager/js/navigation_list.js
  Hunk #1 FAILED at 313.
  Hunk #3 FAILED at 362.
  2 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/resources/file_manager/js/navigation_list.js.rej

Patch:       chrome/browser/resources/file_manager/js/navigation_list.js
Index: chrome/browser/resources/file_manager/js/navigation_list.js
diff --git a/chrome/browser/resources/file_manager/js/navigation_list.js
b/chrome/browser/resources/file_manager/js/navigation_list.js
index
481e05d9481fa6eb8eb4b3628b68e166fec1f5c4..56ee87472b7ea8dddc39944c758329b1d9a3e79a
100644
--- a/chrome/browser/resources/file_manager/js/navigation_list.js
+++ b/chrome/browser/resources/file_manager/js/navigation_list.js
@@ -313,6 +313,7 @@ NavigationList.prototype.renderRoot_ = function(path) {
 
   var handleClick = function() {
     if (item.selected && path !== this.directoryModel_.getCurrentDirPath()) {
+      metrics.recordUserAction('FolderShortcut.Navigate');
       this.directoryModel_.changeDirectory(path);
     }
   }.bind(this);
@@ -349,6 +350,7 @@ NavigationList.prototype.setContextMenu = function(menu) {
 
 /**
  * Selects the n-th item from the list.
+ *
  * @param {number} index Item index.
  * @return {boolean} True for success, otherwise false.
  */
@@ -361,9 +363,12 @@ NavigationList.prototype.selectByIndex = function(index) {
     return false;
 
   // Prevents double-moving to the current directory.
+  // eg. When user clicks the item, changing directory has already been done in
+  //     click handler.
   if (this.directoryModel_.getCurrentDirEntry().fullPath == newPath)
     return false;
 
+  metrics.recordUserAction('FolderShortcut.Navigate');
   this.directoryModel_.changeDirectory(newPath);
   return true;
 };

[commit-bot: I haz the power, 2013-08-12 03:28:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/22634008/13001

[commit-bot: I haz the power, 2013-08-12 03:42:15]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[yoshiki, 2013-08-12 04:10:08]

@asvitkine: Could you look at histgrams.xml?

[mtomasz, 2013-08-12 08:45:40]

On 2013/08/12 04:10:08, yoshiki wrote:
> @asvitkine: Could you look at histgrams.xml?

One more thing:
histgams -> histograms in the title.

Thanks.

[Alexei Svitkine (slow), 2013-08-12 17:22:46]

https://codereview.chromium.org/22634008/diff/13001/tools/metrics/histograms/...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/22634008/diff/13001/tools/metrics/histograms/...
tools/metrics/histograms/histograms.xml:3890: +    Chrome OS File Browser: this
is recorded when the user adds a folder
Nit: Don't we usually write ChromeOS in one word?

https://codereview.chromium.org/22634008/diff/13001/tools/metrics/histograms/...
tools/metrics/histograms/histograms.xml:3896: +  <summary>Chrome OS File
Browser: number of folder shorcuts.</summary>
Mention when this is recorded.

[yoshiki, 2013-08-12 22:27:59]

@asvikine: PTAL. Thanks.

https://codereview.chromium.org/22634008/diff/13001/tools/metrics/histograms/...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/22634008/diff/13001/tools/metrics/histograms/...
tools/metrics/histograms/histograms.xml:3890: +    Chrome OS File Browser: this
is recorded when the user adds a folder
On 2013/08/12 17:22:47, Alexei Svitkine wrote:
> Nit: Don't we usually write ChromeOS in one word?

I think you're right. But in this file, "Chrome OS" is common. To make
consistency, "Chrome OS" is better in this case.

https://codereview.chromium.org/22634008/diff/13001/tools/metrics/histograms/...
tools/metrics/histograms/histograms.xml:3896: +  <summary>Chrome OS File
Browser: number of folder shorcuts.</summary>
On 2013/08/12 17:22:47, Alexei Svitkine wrote:
> Mention when this is recorded.

Added.

[Alexei Svitkine (slow), 2013-08-13 00:29:55]

histograms.xml lgtm

[yoshiki, 2013-08-13 00:34:58]

Thanks all for reviewing. Going to commit.

[yoshiki, 2013-08-13 01:13:06]

Committed patchset #5 manually as r217148 (presubmit successful).