CrOS files: purge analytics from everything except the files app.

Analytics is disabled in these CrOS apps, and isn't going to be re-enabled. Generally, "metrics.js" is analytics. Restrict dependencies on it to the files app only. "metrics_base.js" is UMA and is fine to depend on, but it needs an attached file that provides a UMA prefix. Add missing methods, which were incorrectly using the files app UMA config. Remove metrics.js AND metrics_base from the shared, background_common_scripts.js. audio_player doesn't use metrics_base (UMA) and doesn't declare the API permission in its manifest. Audit things still in background_common_scripts.js to ensure they don't use analytics/metrics. There's just one: volumeManagerUtil.reportMountError. Delete it. Note the file_manager/foreground/js/directory_model.js still uses getFileSystemProviderName from metrics_events.js. Other notes: - Remove the single analytics call in image_loader - Remove metricsPrivate from the image_loader extension manifest (only the image_loader_client depends on this, which is loaded into other component apps - not the image loader) - Remove unused dependencies from manifest files. - Bump manifest versions to update cached extension permissions. Bug: 847729 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: Ifb5d306b38ba8bc0759ac56080855abe78f09b88 Reviewed-on: https://chromium-review.googlesource.com/c/1280064 Commit-Queue: Trent Apted <[email protected]> Reviewed-by: Stuart Langley <[email protected]> Reviewed-by: Naoki Fukino <[email protected]> Cr-Commit-Position: refs/heads/master@{#600323}
feiyue1206 · Oct 17, 2018 · b58575a · b58575a
1 parent ff5128b
commit b58575a
Show file tree

Hide file tree

Showing 21 changed files with 46 additions and 85 deletions.
diff --git a/ui/file_manager/audio_player/js/BUILD.gn b/ui/file_manager/audio_player/js/BUILD.gn
@@ -17,10 +17,8 @@ js_library("closure_compile_externs") {
   sources = []
   externs_list = [
     "$externs_path/chrome_extensions.js",
-    "$externs_path/metrics_private.js",
     "../../externs/audio_player_foreground.js",
     "../../externs/platform.js",
-    "//third_party/analytics/externs.js",
   ]
 }
 

diff --git a/ui/file_manager/audio_player/manifest.json b/ui/file_manager/audio_player/manifest.json
@@ -3,7 +3,7 @@
   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsU2jLo1oiASjXO+/+qYfgojW4Y5TCIBNFzcAdxaF78BRteygWkJqbUVyGZxtSk/RDe3NdNyQCxsyU1YfYXZQvGRNDwKw5tzGyE4dcedveipaJW174VGd3GViS0WWist3HRxfYRZvRP5E8l/4NxXB0+crfq2WolO8La25js1QkCsggQ1lC8g24NRrPnTAWZxvSD6L64R0UoVoe68HdC4mRISe9/OqjyiAfb4Ajgooq8dyzkV8AJTKRjFTmYPlcc5Ba21rXzRt22TnDh2U38m/OEvTu69cyTIxAkBjUa/2gu7N588k9XzaMhTjiolSWxBDQuLZRp8fNjO0R27jouo3FwIDAQAB",
   "manifest_version": 2,
   "name": "Audio Player",
-  "version": "1.0",
+  "version": "1.2",
   "description": "Audio Player",
   "display_in_launcher": false,
   "incognito" : "split",
@@ -44,7 +44,6 @@
   "app": {
     "background": {
       "scripts": [
-        "chrome://resources/js/analytics.js",
         "chrome://resources/js/cr.js",
         "chrome://resources/js/cr/event_target.js",
         "chrome://resources/js/cr/ui.js",

diff --git a/ui/file_manager/file_manager/background/js/background_common_scripts.js b/ui/file_manager/file_manager/background/js/background_common_scripts.js
@@ -14,8 +14,6 @@
 // <include src="../../common/js/async_util.js">
 // <include src="../../common/js/file_type.js">
 // <include src="../../common/js/metrics_base.js">
-// <include src="../../common/js/metrics_events.js">
-// <include src="../../common/js/metrics.js">
 // <include src="../../common/js/files_app_entry_types.js">
 // <include src="../../common/js/util.js">
 // <include src="../../common/js/volume_manager_common.js">

diff --git a/ui/file_manager/file_manager/background/js/background_scripts.js b/ui/file_manager/file_manager/background/js/background_scripts.js
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// <include src="../../common/js/metrics_events.js">
+// <include src="../../common/js/metrics.js">
 // <include src="metrics_start.js">
 // <include src="../../common/js/lru_cache.js">
 // <include src="../../common/js/progress_center_common.js">

diff --git a/ui/file_manager/file_manager/background/js/volume_manager_util.js b/ui/file_manager/file_manager/background/js/volume_manager_util.js
@@ -179,7 +179,8 @@ volumeManagerUtil.createVolumeInfo = function(volumeMetadata) {
         console.warn('Failed to mount a file system: ' +
             volumeMetadata.volumeId + ' because of: ' +
             (error.stack || error));
-        volumeManagerUtil.reportMountError(volumeMetadata, error);
+
+        // TODO(crbug/847729): Report a mount error via UMA.
 
         return new VolumeInfoImpl(
             /** @type {VolumeManagerCommon.VolumeType} */
@@ -196,39 +197,3 @@ volumeManagerUtil.createVolumeInfo = function(volumeMetadata) {
             (volumeMetadata.diskFileSystemType), volumeMetadata.iconSet);
       });
 };
-
-/**
- * Reports a mount error to analytics in the form of
- * "mount {errorType} {volumeType}", like
- * "mount timeout(resolveIsolatedEntries) provided:ZipUnpacker".
- * Note that errorType and volumeType must be an element of fixed set of strings
- * to avoid sending dynamic strings to analytics.
- *
- * @param {chrome.fileManagerPrivate.VolumeMetadata} volumeMetadata
- * @param {*} error
- */
-volumeManagerUtil.reportMountError = function(volumeMetadata, error) {
-  var errorType = 'error';
-  if (error instanceof Error) {
-    if (error.message.startsWith(
-        volumeManagerUtil.TIMEOUT_STR_REQUEST_FILE_SYSTEM)) {
-      errorType = volumeManagerUtil.TIMEOUT_STR_REQUEST_FILE_SYSTEM;
-    }
-    if (error.message.startsWith(
-        volumeManagerUtil.TIMEOUT_STR_RESOLVE_ISOLATED_ENTRIES)) {
-      errorType = volumeManagerUtil.TIMEOUT_STR_RESOLVE_ISOLATED_ENTRIES;
-    }
-  }
-  var volumeType = volumeMetadata.volumeType;
-  if (volumeMetadata.volumeType === VolumeManagerCommon.VolumeType.PROVIDED) {
-    volumeType +=
-        ':' + metrics.getFileSystemProviderName(volumeMetadata.providerId);
-  }
-  var description = 'mount ' + errorType + ' ' + volumeType;
-  var fatal =
-      volumeMetadata.volumeType === VolumeManagerCommon.VolumeType.DOWNLOADS ||
-      volumeMetadata.volumeType === VolumeManagerCommon.VolumeType.DRIVE;
-
-  if (window.background && window.background.tracker)
-    window.background.tracker.sendException(description, fatal);
-};
diff --git a/ui/file_manager/file_manager/common/js/BUILD.gn b/ui/file_manager/file_manager/common/js/BUILD.gn
@@ -124,21 +124,25 @@ js_unittest("lru_cache_unittest") {
 }
 
 js_library("metrics") {
+  # |metrics| and |metrics_events| are for analytics. Don't leak this dependency
+  # outside of the core files app.
+  visibility = []
+  visibility = [ "//ui/file_manager/file_manager/*" ]
   deps = [
     ":metrics_base",
     "../../../externs:file_manager_private",
     "//ui/webui/resources/js:assert",
   ]
-  externs_list = [
-    "$externs_path/metrics_private.js",
-    "//third_party/analytics/externs.js",
-  ]
+  externs_list = [ "//third_party/analytics/externs.js" ]
 }
 
 js_library("metrics_base") {
+  externs_list = [ "$externs_path/metrics_private.js" ]
 }
 
 js_library("metrics_events") {
+  visibility = []
+  visibility = [ "//ui/file_manager/file_manager/*" ]
   deps = [
     ":metrics_base",
   ]

diff --git a/ui/file_manager/file_manager/common/js/metrics.js b/ui/file_manager/file_manager/common/js/metrics.js
@@ -10,15 +10,6 @@
 
 var metrics = metrics || metricsBase;
 
-/**
- * Analytics tracking ID for the Files app.
- * @const {!Object<string, string>}
- */
-metrics.TRACKING_IDS = {
-  hhaomjibdihmijegdhdafkllkbggdgoj: 'UA-38248358-9',  // Files app
-  pmfjbimdmchhbnneeidfognadeopoehp: 'UA-38248358-13'  // Image Loader
-};
-
 /**
  * Convert a short metric name to the full format.
  *
@@ -62,8 +53,8 @@ metrics.createTracker_ = function() {
 
   // Create a tracker, add a filter that only enables analytics when UMA is
   // enabled.
-  metrics.tracker_ = metrics.analytics_.getTracker(
-      metrics.TRACKING_IDS[chrome.runtime.id]);
+  const kFilesAppTrackingId = 'UA-38248358-9';
+  metrics.tracker_ = metrics.analytics_.getTracker(kFilesAppTrackingId);
   metrics.tracker_.addFilter(metrics.umaEnabledFilter_);
 };
 

diff --git a/ui/file_manager/gallery/js/BUILD.gn b/ui/file_manager/gallery/js/BUILD.gn
@@ -109,8 +109,8 @@ js_unittest("gallery_data_model_unittest") {
 
 js_library("gallery_item") {
   deps = [
+    ":gallery_metrics",
     ":gallery_util",
-    "../../file_manager/common/js:metrics",
     "../../file_manager/common/js:util",
     "../../file_manager/foreground/js/metadata:metadata_model",
     "../../file_manager/foreground/js/metadata:thumbnail_model",
@@ -128,6 +128,12 @@ js_unittest("gallery_item_unittest") {
   ]
 }
 
+js_library("gallery_metrics") {
+  deps = [
+    "../../file_manager/common/js:metrics_base",
+  ]
+}
+
 js_library("gallery_util") {
   deps = [
     "../../file_manager/common/js:file_type",
@@ -181,8 +187,8 @@ js_library("slide_mode") {
     ":gallery_constants",
     ":gallery_data_model",
     ":gallery_item",
+    ":gallery_metrics",
     ":ribbon",
-    "../../file_manager/common/js:metrics",
     "../../file_manager/common/js:util",
     "../../file_manager/foreground/elements:files_toggle_ripple",
     "image_editor:image_adjust",

diff --git a/ui/file_manager/gallery/js/background_scripts.js b/ui/file_manager/gallery/js/background_scripts.js
@@ -2,5 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// <include src="gallery_metrics.js">
 // <include src="test_util.js">
 // <include src="background.js">
diff --git a/ui/file_manager/gallery/js/gallery_metrics.js b/ui/file_manager/gallery/js/gallery_metrics.js
@@ -0,0 +1,14 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var metrics = metricsBase;
+
+/**
+ * @private
+ */
+metrics.convertName_ = function(name) {
+  // Historically, Gallery incorrectly uses the Files App prefix. Keep doing
+  // that to preserve continuity.
+  return 'FileBrowser.' + name;
+};
diff --git a/ui/file_manager/gallery/js/gallery_scripts.js b/ui/file_manager/gallery/js/gallery_scripts.js
@@ -10,7 +10,7 @@
 
 // <include src="../../base/js/error_counter.js">
 // <include src="../../file_manager/common/js/metrics_base.js">
-// <include src="../../file_manager/common/js/metrics.js">
+// <include src="gallery_metrics.js">
 // <include src="../../file_manager/foreground/js/metrics_start.js">
 
 // <include src="../../file_manager/common/js/lru_cache.js">

diff --git a/ui/file_manager/gallery/js/image_editor/BUILD.gn b/ui/file_manager/gallery/js/image_editor/BUILD.gn
@@ -173,7 +173,7 @@ js_library("image_view") {
     ":image_util",
     ":viewport",
     "..:gallery_item",
-    "../../../file_manager/common/js:metrics",
+    "../../../file_manager/common/js:metrics_base",
     "../../../file_manager/foreground/js:thumbnail_loader",
     "//ui/webui/resources/js:assert",
   ]

diff --git a/ui/file_manager/gallery/manifest.json b/ui/file_manager/gallery/manifest.json
@@ -3,7 +3,7 @@
   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1hFTC+sl9e4ufs6ccGhspbgnMQb3GMikA/QwghFVp00WDwFu7no8xIOWJwY9lFQP+NrbSsze3JL9Wg6FmHC6xKIncwZJKwyDDd2g+9/gEZLn5Ar1piPyf+ELtuX+m0Pjp0l2+rVMz2UiP5OUvFqvmCZqJJQnTVjRut3IMjDP6npb5HyDTgqlPgNHWmsLAQZZKTyYfqswBFkvmwiSHTNJuxkh+i1hxo2m8RcBQsXWL8Mt9+WPl0uABIZc7UvLoZwNz1pAKWb5sv0y4oBugpw4ZVIvCT/pxplLXF35GGBNWAkgimkpYu+SldoZQV8SZW1kUSIcrpYW80mA7KxfK5H8vwIDAQAB",
   "manifest_version": 2,
   "name": "Gallery",
-  "version": "2.0",
+  "version": "2.2",
   "description": "Picture browser app",
   "display_in_launcher": false,
   "incognito" : "split",
@@ -72,7 +72,6 @@
   "app": {
     "background": {
       "scripts": [
-        "chrome://resources/js/analytics.js",
         "chrome://resources/js/cr.js",
         "chrome://resources/js/cr/event_target.js",
         "chrome://resources/js/cr/ui/array_data_model.js",

diff --git a/ui/file_manager/image_loader/BUILD.gn b/ui/file_manager/image_loader/BUILD.gn
@@ -110,8 +110,6 @@ js_library("request") {
     ":load_image_request",
     ":piex_loader",
     "../file_manager/common/js:file_type",
-    "../file_manager/common/js:metrics",
-    "../file_manager/common/js:metrics_events",
   ]
   externs_list = [ "../externs/platform.js" ]
 }

diff --git a/ui/file_manager/image_loader/background_scripts.js b/ui/file_manager/image_loader/background_scripts.js
@@ -3,9 +3,6 @@
 // found in the LICENSE file.
 
 // <include src="../file_manager/common/js/file_type.js">
-// <include src="../file_manager/common/js/metrics_base.js">
-// <include src="../file_manager/common/js/metrics.js">
-// <include src="../file_manager/common/js/metrics_events.js">
 // <include src="../file_manager/foreground/js/metadata/image_orientation.js">
 // <include src="cache.js">
 // <include src="load_image_request.js">

diff --git a/ui/file_manager/image_loader/manifest.json b/ui/file_manager/image_loader/manifest.json
@@ -2,7 +2,7 @@
   // chrome-extension://pmfjbimdmchhbnneeidfognadeopoehp
   "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDowC9B4+K6zbl4PnALNyOUgra/MPdD8gZ39Fk/IAJWt03qrN7vz1gd/mmrBg0EEIsyLRmUmfyVEfvcIUOZxFqn4A9D2aaRSvNHy9qkasZMBDEql8Nt2iNZm/kGS7sizidDV6Bc/vyLNiH1gKOXBQ42JIxKjgtrmnhGV2giw2vJGwIDAQAB",
   "name": "Image loader",
-  "version": "0.1",
+  "version": "0.2",
   "description": "Image loader",
   "incognito" : "split",
   "manifest_version": 2,
@@ -12,16 +12,13 @@
       "fileSystem": ["requestFileSystem"]
     },
     "fileManagerPrivate",
-    "https://www.google-analytics.com/",
     "https://www.googledrive.com/",
     "https://lh3.googleusercontent.com/",
-    "metricsPrivate",
     "storage"
   ],
-  "content_security_policy": "default-src 'none'; script-src 'self' blob: filesystem: chrome://resources chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj; style-src 'self' blob: filesystem:; frame-src 'self' blob: filesystem:; img-src 'self' blob: filesystem: data:; media-src 'self' blob: filesystem:; connect-src 'self' blob: filesystem: https://www.googledrive.com https://www.google-analytics.com https://lh3.googleusercontent.com",
+  "content_security_policy": "default-src 'none'; script-src 'self' blob: filesystem: chrome://resources chrome-extension://hhaomjibdihmijegdhdafkllkbggdgoj; style-src 'self' blob: filesystem:; frame-src 'self' blob: filesystem:; img-src 'self' blob: filesystem: data:; media-src 'self' blob: filesystem:; connect-src 'self' blob: filesystem: https://www.googledrive.com https://lh3.googleusercontent.com",
   "background": {
     "scripts": [
-      "chrome://resources/js/analytics.js",
       "chrome://resources/js/assert.js",
       "background_scripts.js"
     ],

diff --git a/ui/file_manager/image_loader/request.js b/ui/file_manager/image_loader/request.js
@@ -276,12 +276,7 @@ ImageRequest.prototype.downloadOriginal_ = function(onSuccess, onFailure) {
 
   // Load RAW images by using Piex loader instead of XHR.
   if (fileType.type === 'raw') {
-    var timer = metrics.getTracker().startTiming(
-        metrics.Categories.INTERNALS,
-        metrics.timing.Variables.EXTRACT_THUMBNAIL_FROM_RAW,
-        fileType.subtype);
     this.piexLoader_.load(this.request_.url).then(function(data) {
-      timer.send();
       var blob = new Blob([data.thumbnail], {type: 'image/jpeg'});
       var url = URL.createObjectURL(blob);
       this.image_.src = url;

diff --git a/ui/file_manager/video_player/js/BUILD.gn b/ui/file_manager/video_player/js/BUILD.gn
@@ -23,7 +23,6 @@ js_library("closure_compile_externs") {
     "$externs_path/metrics_private.js",
     "../../externs/chrome_cast.js",
     "../../externs/platform.js",
-    "//third_party/analytics/externs.js",
   ]
 }
 
@@ -54,7 +53,7 @@ js_library("video_player") {
     "cast:media_manager",
     "//ui/file_manager/base/js:error_counter",
     "//ui/file_manager/base/js:filtered_volume_manager",
-    "//ui/file_manager/file_manager/common/js:metrics",
+    "//ui/file_manager/file_manager/common/js:metrics_base",
     "//ui/file_manager/file_manager/common/js:util",
     "//ui/file_manager/image_loader:image_loader_client",
     "//ui/webui/resources/js:i18n_template_no_process",

diff --git a/ui/file_manager/video_player/js/background_scripts.js b/ui/file_manager/video_player/js/background_scripts.js
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// <include src="video_player_metrics.js">
 // <include src="test_util.js">
 // The main background script must be at the end.
 // <include src="background.js">
diff --git a/ui/file_manager/video_player/js/cast/BUILD.gn b/ui/file_manager/video_player/js/cast/BUILD.gn
@@ -19,12 +19,10 @@ js_library("closure_compile_externs") {
   externs_list = [
     "$externs_path/chrome_extensions.js",
     "$externs_path/media_player_private.js",
-    "$externs_path/metrics_private.js",
     "../../../externs/app_window_common.js",
     "../../../externs/background/volume_manager_factory.js",
     "../../../externs/chrome_cast.js",
     "../../../externs/platform.js",
-    "//third_party/analytics/externs.js",
   ]
 }
 

diff --git a/ui/file_manager/video_player/manifest.json b/ui/file_manager/video_player/manifest.json
@@ -3,7 +3,7 @@
   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAw0/gRbJc545iEGRZs20Rl/HtrSUp8H3gJd4Y6hCe0CG1xQiJhZ5nc8qZyxa96gMxRAKBq54S6sjVVtV6uS70oU6FvrvwItByYkkqr4ZE7eMJKwMqnGItxWbh6KBodf89lpKoIy6MtYTqubBhXB/IQBZsXah90tXwRzaaJNWw+2BBRIhcPsH3ng+wgN7rwFxo4HIv9ZpqkYlx90rwkfjOmKPPnSXyXFIBJfmqfdbd8PLtcxzzOTE+vxwoXZuYWrthKm4uKfNqXIYns74sSJlqyKfctuR+nQdNh8uePv0e+/Ul3wER1/jIXULLjfyoaklyDs+ak3SDf+xWScJ+0LJ0AwIDAQAB",
   "manifest_version": 2,
   "name": "Video Player",
-  "version": "1.0",
+  "version": "1.2",
   "description": "Video Player",
   "display_in_launcher": false,
   "incognito" : "split",
@@ -54,7 +54,6 @@
   "app": {
     "background": {
       "scripts": [
-        "chrome://resources/js/analytics.js",
         "chrome://resources/js/cr.js",
         "chrome://resources/js/cr/event_target.js",
         "chrome://resources/js/cr/ui/array_data_model.js",