Quick View UMA: export how quick view was opened.

Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=try & manually checked the UMA were exported in
     chrome://histograms as expected.
BUG=680018
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2628833002
Cr-Commit-Position: refs/heads/master@{#443839}
Committed: https://chromium.googlesource.com/chromium/src/+/3d80af48c3cd0c301ca3f2e5ba0eab568b36ccc9

[oka, 2017-01-11 12:06:11]

Description was changed from

==========
Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=TBD.
BUG=680018
==========

to

==========
Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=TBD.
BUG=680018
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-01-11 12:09:32]

nit.

[oka, 2017-01-11 12:14:11]

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

[commit-bot: I haz the power, 2017-01-11 12:14:23]

Dry run: 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, 2017-01-11 12:27:33]

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

[commit-bot: I haz the power, 2017-01-11 12:27:34]

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

[oka, 2017-01-11 12:47:36]

Fix closure compilation error.

[oka, 2017-01-11 13:44:51]

Description was changed from

==========
Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=TBD.
BUG=680018
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=try & manually checked the UMA were exported in
     chrome://histograms as expected.
BUG=680018
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-01-11 13:45:42]

oka@chromium.org changed reviewers:
+ fukino@chromium.org

[oka, 2017-01-11 13:45:42]

PTAL.

[oka, 2017-01-11 13:46:06]

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

[commit-bot: I haz the power, 2017-01-11 13:46:19]

Dry run: 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, 2017-01-11 15:21:49]

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

[commit-bot: I haz the power, 2017-01-11 15:21:49]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[fukino, 2017-01-12 03:46:01]

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/quick_view_controller.js
(right):

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/quick_view_controller.js:150: *
@param {QuickViewUma.WayToOpen=} wayToOpen in which way opening of
If this is an optional parameter, it should be opt_wayToOpen. (But I guess it is
not optional?)

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/quick_view_uma.js (right):

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/quick_view_uma.js:23: * @enum
{!string}
nit: ! is redundant

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/quick_view_uma.js:87: * @param
{!QuickViewUma.WayToOpen} wayToOpen
nit: We usually don't use ! for enum types.

[oka, 2017-01-12 04:10:38]

Addressed comments.

[oka, 2017-01-12 04:13:29]

nit

[oka, 2017-01-12 04:14:02]

PTAL

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/quick_view_controller.js
(right):

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/quick_view_controller.js:150: *
@param {QuickViewUma.WayToOpen=} wayToOpen in which way opening of
On 2017/01/12 03:46:01, fukino wrote:
> If this is an optional parameter, it should be opt_wayToOpen. (But I guess it
is
> not optional?)

This is actually optional. It doesn't have to be provided if caller is sure that
Quick View is already open (L176). Added opt_ prefix.

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/js/quick_view_uma.js (right):

https://codereview.chromium.org/2628833002/diff/40001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/js/quick_view_uma.js:23: * @enum
{!string}
On 2017/01/12 03:46:01, fukino wrote:
> nit: ! is redundant

Oops. Done.

[oka, 2017-01-12 04:30:28]

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

[commit-bot: I haz the power, 2017-01-12 04:30:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fukino, 2017-01-12 04:54:44]

file_manager lgtm

[oka, 2017-01-12 04:59:28]

oka@chromium.org changed reviewers:
+ jwd@chromium.org

[oka, 2017-01-12 04:59:29]

jwd@ PTAL at histograms.xml.

[commit-bot: I haz the power, 2017-01-12 05:43:16]

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

[commit-bot: I haz the power, 2017-01-12 05:43:17]

Dry run: This issue passed the CQ dry run.

[oka, 2017-01-13 04:51:00]

oka@chromium.org changed reviewers:
+ holte@chromium.org

[oka, 2017-01-13 04:51:01]

holte, could you take a look at histograms.xml?

[Steven Holte, 2017-01-13 21:24:55]

lgtm

[oka, 2017-01-15 06:22:29]

The CQ bit was checked by oka@chromium.org

[commit-bot: I haz the power, 2017-01-15 06:22:41]

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, 2017-01-15 07:24:33]

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

[commit-bot: I haz the power, 2017-01-15 07:24:34]

Failed to apply patch for
ui/file_manager/file_manager/foreground/js/quick_view_controller.js:
While running git apply --index -p1;
  error: patch failed:
ui/file_manager/file_manager/foreground/js/quick_view_controller.js:76
  error: ui/file_manager/file_manager/foreground/js/quick_view_controller.js:
patch does not apply

Patch:       ui/file_manager/file_manager/foreground/js/quick_view_controller.js
Index: ui/file_manager/file_manager/foreground/js/quick_view_controller.js
diff --git a/ui/file_manager/file_manager/foreground/js/quick_view_controller.js
b/ui/file_manager/file_manager/foreground/js/quick_view_controller.js
index
c651a9e0dc9b05b1db065f9eddc97682245bae93..9bbe2c1a9355d9f46939e92c8be60b9dd150a500
100644
--- a/ui/file_manager/file_manager/foreground/js/quick_view_controller.js
+++ b/ui/file_manager/file_manager/foreground/js/quick_view_controller.js
@@ -76,7 +76,7 @@ function QuickViewController(
       'keydown', this.onKeyDownToOpen_.bind(this));
   listContainer.element.addEventListener('command', function(event) {
     if(event.command.id === 'get-info')
-      this.display_();
+      this.display_(QuickViewUma.WayToOpen.CONTEXT_MENU);
   }.bind(this));
   quickView.addEventListener('keydown', this.onQuickViewKeyDown_.bind(this));
   quickView.addEventListener('iron-overlay-closed', function() {
@@ -111,7 +111,7 @@ QuickViewController.prototype.onKeyDownToOpen_ =
function(event) {
     return;
   if (event.key === ' ') {
     event.preventDefault();
-    this.display_();
+    this.display_(QuickViewUma.WayToOpen.SPACE_KEY);
   }
 };
 
@@ -148,13 +148,15 @@ QuickViewController.prototype.onQuickViewKeyDown_ =
function(event) {
 /**
  * Display quick view.
  *
+ * @param {QuickViewUma.WayToOpen=} opt_wayToOpen in which way opening of
+ *     quick view was triggered. Can be omitted if quick view is already open.
  * @private
  */
-QuickViewController.prototype.display_ = function() {
+QuickViewController.prototype.display_ = function(opt_wayToOpen) {
   this.updateQuickView_().then(function() {
     if (!this.quickView_.isOpened()) {
       this.quickView_.open();
-      this.quickViewUma_.onOpened(this.entries_[0]);
+      this.quickViewUma_.onOpened(this.entries_[0], assert(opt_wayToOpen));
     }
   }.bind(this));
 };

[oka, 2017-01-15 07:53:06]

Rebase.

[oka, 2017-01-16 02:28:16]

The CQ bit was checked by oka@chromium.org

[oka, 2017-01-16 02:28:17]

The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org,
holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2628833002/#ps100001
(title: "Rebase.")

[commit-bot: I haz the power, 2017-01-16 02:28:31]

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, 2017-01-16 03:22:32]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484533696622930,
"parent_rev": "e3df8cd8ac45539711b1f0b6a00420f87d09d279", "commit_rev":
"3d80af48c3cd0c301ca3f2e5ba0eab568b36ccc9"}

[commit-bot: I haz the power, 2017-01-16 03:23:05]

Description was changed from

==========
Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=try & manually checked the UMA were exported in
     chrome://histograms as expected.
BUG=680018
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Quick View UMA: export how quick view was opened.

There are two ways to open quick view: from context menu or with space
key. This change exports in which way quick view was opened with name
'FileBrowser.QuickView.WayToOpen'.

TEST=try & manually checked the UMA were exported in
     chrome://histograms as expected.
BUG=680018
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2628833002
Cr-Commit-Position: refs/heads/master@{#443839}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d80af48c3cd0c301ca3f2e5ba0e...
==========

[commit-bot: I haz the power, 2017-01-16 03:23:06]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/3d80af48c3cd0c301ca3f2e5ba0e...