New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847729 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 907702

Blocking:
issue 646705



Sign in to add a comment

Remove Google Analytics from the Files App

Project Member Reported by sashab@chromium.org, May 30 2018

Issue description

Remove Google Analytics from the Files App, since its no longer used.
 
Cc: dpa...@chromium.org
+1 on removing. It also seems that this file is only used by File Manager, so if it is removed from there, can we remove it from Chrome as a whole? See [1].

[1] https://cs.chromium.org/search/?q=%22chrome://resources/js/analytics.js%22&type=cs
Owner: slangley@chromium.org
Cc: slangley@chromium.org mcirimele@chromium.org weifangsun@chromium.org
Labels: -Pri-3 M-70 Pri-2
Owner: noel@chromium.org
Labels: -M-70 M-71
Owner: slangley@chromium.org
Status: Started (was: Assigned)
What is the status of this bug? See related CL at https://chromium-review.googlesource.com/c/chromium/src/+/1244296.
WIP - 1 or 2 weeks till it lands I guess.
Labels: -M-71 M-72
https://chromium-review.googlesource.com/c/chromium/src/+/1280064 purges analytics from everything except the core files app. (i.e. from image_loader, video_player, gallery, audio_player).
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b58575aeea416b66b735eb96ce77476023fb0789

commit b58575aeea416b66b735eb96ce77476023fb0789
Author: Trent Apted <tapted@chromium.org>
Date: Wed Oct 17 09:16:31 2018

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 <tapted@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600323}
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/audio_player/js/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/audio_player/manifest.json
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/file_manager/background/js/background_common_scripts.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/file_manager/background/js/background_scripts.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/file_manager/background/js/volume_manager_util.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/file_manager/common/js/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/file_manager/common/js/metrics.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/gallery/js/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/gallery/js/background_scripts.js
[add] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/gallery/js/gallery_metrics.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/gallery/js/gallery_scripts.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/gallery/js/image_editor/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/gallery/manifest.json
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/image_loader/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/image_loader/background_scripts.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/image_loader/manifest.json
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/image_loader/request.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/video_player/js/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/video_player/js/background_scripts.js
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/video_player/js/cast/BUILD.gn
[modify] https://crrev.com/b58575aeea416b66b735eb96ce77476023fb0789/ui/file_manager/video_player/manifest.json

Labels: Files-Fixit-2018
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3ccd2593bf76cbe1d0611374813294c838cae77

commit d3ccd2593bf76cbe1d0611374813294c838cae77
Author: Stuart Langley <slangley@google.com>
Date: Wed Nov 14 09:53:36 2018

Remove analytics from the photo/video import flow, replace with UMA.

Analytics was disabled some time ago, and we want to remove this from
the files app. Start with media upload flow.

- Mark as obsolete UMA's that we no longer record (they stopped being
  recorded some time ago).
- Add UMAs to replace the metrics that were being recorded using
  analytics.
- Fix some of the noise in the unit tests.
- Remove some of the analytics events dead code.

Bug:  847729 
Change-Id: Ic501553e6967aa401d9b3fcc5cd2c56024756d10
Reviewed-on: https://chromium-review.googlesource.com/c/1328062
Commit-Queue: Stuart Langley <slangley@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607941}
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/background/js/media_import_handler.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/background/js/media_import_handler_unittest.html
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/background/js/media_import_handler_unittest.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/common/js/metrics_base.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/common/js/metrics_events.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/foreground/js/import_controller.js
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/foreground/js/import_controller_unittest.html
[modify] https://crrev.com/d3ccd2593bf76cbe1d0611374813294c838cae77/ui/file_manager/file_manager/foreground/js/import_controller_unittest.js

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f984f4e13ac498176597639a09adf43faca3ea4

commit 4f984f4e13ac498176597639a09adf43faca3ea4
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 01:16:22 2018

Remove analytics tracking from import.RuntimeHistoryLoader

No change in test behavior, no new tests.

Bug:  905934 ,  847729 
No-try: true
Change-Id: Ic1a9b352260670b643bb60cae6cfb5483f80ea70
Reviewed-on: https://chromium-review.googlesource.com/c/1345374
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609879}
[modify] https://crrev.com/4f984f4e13ac498176597639a09adf43faca3ea4/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/4f984f4e13ac498176597639a09adf43faca3ea4/ui/file_manager/file_manager/background/js/import_history.js
[modify] https://crrev.com/4f984f4e13ac498176597639a09adf43faca3ea4/ui/file_manager/file_manager/background/js/import_history_unittest.js

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4aa1743483c6efb140c26f0dc5d447b0161b7a27

commit 4aa1743483c6efb140c26f0dc5d447b0161b7a27
Author: Noel Gordon <noel@chromium.org>
Date: Wed Nov 21 13:14:41 2018

After CL:1345374 metrics.ImportEvents are unused: remove

 - remove metrics.ImportEvents
 - remove metrics.timing.Variables.HISTORY_LOAD

No change in behavior, no new tests.

Bug:  905934 ,  847729 
Change-Id: I2f5ab9659dc5e134bdb47d8c770a1f918b4a0126
Reviewed-on: https://chromium-review.googlesource.com/c/1345381
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610025}
[modify] https://crrev.com/4aa1743483c6efb140c26f0dc5d447b0161b7a27/ui/file_manager/file_manager/common/js/metrics_events.js

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b67026281f65cf8bbea945be0e78bac0a2925ea6

commit b67026281f65cf8bbea945be0e78bac0a2925ea6
Author: Stuart Langley <slangley@google.com>
Date: Thu Nov 22 00:53:41 2018

Remove google analytics from DriveDuplicateFinder.

Replace the exiting data that was being recorded using GA with UMA
for long hash calculate and search by hash results.

GA was disabled some time back so this data has not been being
recorded for some time. We'll switch to UMA for the time being and
then make a decision on either keeping or removing these metrics
altogether at a later date.

Bug:  847729 
Change-Id: I6b2fac50a3275b1cd3684ef3b438f26d147ea64c
Reviewed-on: https://chromium-review.googlesource.com/c/1341434
Commit-Queue: Stuart Langley <slangley@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610275}
[modify] https://crrev.com/b67026281f65cf8bbea945be0e78bac0a2925ea6/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/b67026281f65cf8bbea945be0e78bac0a2925ea6/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/b67026281f65cf8bbea945be0e78bac0a2925ea6/ui/file_manager/file_manager/background/js/duplicate_finder.js
[modify] https://crrev.com/b67026281f65cf8bbea945be0e78bac0a2925ea6/ui/file_manager/file_manager/background/js/duplicate_finder_unittest.js
[modify] https://crrev.com/b67026281f65cf8bbea945be0e78bac0a2925ea6/ui/file_manager/file_manager/common/js/metrics_events.js

Blockedon: 907702
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f53a4eeb0c5551069ffa381ab74d09f912a8837f

commit f53a4eeb0c5551069ffa381ab74d09f912a8837f
Author: Stuart Langley <slangley@google.com>
Date: Thu Nov 22 06:09:13 2018

Remove google analytics entirely.

This CL removes the last remaining traces of GA from files app.

It does not replace the remaining metrics that were being recorded
with UMAs, we're removing them as well as either:
a) There's coverage via an existing UMA, or
b) We don't care to record that metric anymore.

Bug:  847729 
Change-Id: I602de642bac43df64f1ecd9035314d5019268aa8
Reviewed-on: https://chromium-review.googlesource.com/c/1347648
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610317}
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/externs/background/file_browser_background.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/background/js/background_scripts.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/background/js/media_import_handler_unittest.html
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/common/js/BUILD.gn
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/common/js/OWNERS
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/common/js/importer_common.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/common/js/metrics.js
[delete] https://crrev.com/606aacfb544375ed79312b389e9da7314fe48b4d/ui/file_manager/file_manager/common/js/metrics_events.js
[delete] https://crrev.com/606aacfb544375ed79312b389e9da7314fe48b4d/ui/file_manager/file_manager/common/js/test_tracker.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/foreground/js/BUILD.gn
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/foreground/js/import_controller_unittest.html
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/foreground/js/main.js
[modify] https://crrev.com/f53a4eeb0c5551069ffa381ab74d09f912a8837f/ui/file_manager/file_manager/foreground/js/main_scripts.js

Status: Fixed (was: Started)
Status: Assigned (was: Fixed)
Re-opening, since there are still some references to chrome://resources/js/analytics.js. Should these also be removed? See 

 - https://cs.chromium.org/chromium/src/ui/file_manager/base/js/error_counter.js?l=23
 - https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/manifest.json?l=198&dr=C
Also, the analytics.js file itself seems to still be included at https://cs.chromium.org/chromium/src/ui/file_manager/file_manager_resources.grd?l=24.
FYI, uploaded a CL removing any leftovers of analytics.js within file_manager/ at https://chromium-review.googlesource.com/c/chromium/src/+/1351507.

Once those are removed I can look into fully removing third_party/analytics from the repository. 
CL that removes third_party/analytics at https://chromium-review.googlesource.com/c/chromium/src/+/1352430.
Project Member

Comment 24 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64d6a56f094efbab83802c07ad3ddffa014f9903

commit 64d6a56f094efbab83802c07ad3ddffa014f9903
Author: dpapad <dpapad@chromium.org>
Date: Wed Nov 28 00:38:13 2018

WebUI cleanup: Remove third_party/analytics.

Last consumer of this dependency was the Files app, which no
longer uses it.

Bug:  847729 
Change-Id: Ibba5a18b722bc44ef74689834660a7197cc923d3
Reviewed-on: https://chromium-review.googlesource.com/c/1352430
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Steve McKay <smckay@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611460}
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/.gn
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/chrome/test/BUILD.gn
[delete] https://crrev.com/58e8c480febc79862c6ca7a569a9168f5aadbede/third_party/analytics/OWNERS
[delete] https://crrev.com/58e8c480febc79862c6ca7a569a9168f5aadbede/third_party/analytics/README.chromium
[delete] https://crrev.com/58e8c480febc79862c6ca7a569a9168f5aadbede/third_party/analytics/externs.js
[delete] https://crrev.com/58e8c480febc79862c6ca7a569a9168f5aadbede/third_party/analytics/google-analytics-bundle.js
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/background/js/media_import_handler_unittest.html
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/common/js/BUILD.gn
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/common/js/metrics_unittest.html
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/foreground/js/import_controller_unittest.html
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/file_manager/file_manager/test/BUILD.gn
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/webui/resources/PRESUBMIT.py
[delete] https://crrev.com/58e8c480febc79862c6ca7a569a9168f5aadbede/ui/webui/resources/js/analytics.js
[modify] https://crrev.com/64d6a56f094efbab83802c07ad3ddffa014f9903/ui/webui/resources/webui_resources.grd

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ecb81c3fe10845a0cbdbc7872a89054908c023cc

commit ecb81c3fe10845a0cbdbc7872a89054908c023cc
Author: Noel Gordon <noel@chromium.org>
Date: Fri Nov 30 02:34:03 2018

FilesApp analytics: remove metrics unittest

CL:1347648 removed google analytics from FilesApp so metrics_unittest,
which was disabled on  issue 842880 , can now be deleted [1].

[1] The metrics_unittest test fixtures required a {analytics.Tracker},
which no longer exists in FilesApp.

Bug:  847729 ,  860355 
Change-Id: I0ec71ef837e747cd6647d7359546b9876d62dbc8
Reviewed-on: https://chromium-review.googlesource.com/c/1355303
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612528}
[modify] https://crrev.com/ecb81c3fe10845a0cbdbc7872a89054908c023cc/chrome/browser/chromeos/file_manager/file_manager_jstest.cc
[delete] https://crrev.com/4cebe24d76e6eeb89c5820b7b1a2da067b16692b/ui/file_manager/file_manager/common/js/metrics_unittest.html
[delete] https://crrev.com/4cebe24d76e6eeb89c5820b7b1a2da067b16692b/ui/file_manager/file_manager/common/js/metrics_unittest.js

Status: Fixed (was: Assigned)
This seems done now.

Sign in to add a comment