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

Issue 772204 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Move UMA collection to taskified offline page model.

Project Member Reported by romax@chromium.org, Oct 6 2017

Issue description

There's a need to move UMA collection in OfflinePageModelImpl to the taskified tasks, in order to preserve the data collection of Offline Page features.
This will be the main tracking bug for related issues.
 

Comment 1 by romax@chromium.org, Oct 9 2017

Maybe offline_event_logger can be used for related logging here. But UMA collection still makes more sense.
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 8 2017

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

commit 73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2
Author: Yafei Duan <romax@chromium.org>
Date: Fri Dec 08 21:41:57 2017

[Offline Pages] Adding UMA for consistency checks in OPMTaskified.

Adding UMAs for temporary and persistent page consistency checks.
- Marked old UMAs obsolete because the metric collection splits into
  temporary and persistent.
- Combined the Delete*Result into a single metric with the ability to
  record most possible failures during the synchronous operations.
- Adding tests using HistogramTester.

This is part of the UMA migration from OfflinePageModel to OPMTaskified.

Bug:  772204 
Change-Id: I3bb13c305775523381ffec7a593387b82dda82e1
Reviewed-on: https://chromium-review.googlesource.com/807252
Commit-Queue: Yafei Duan <romax@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522881}
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/model/persistent_pages_consistency_check_task.cc
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/model/persistent_pages_consistency_check_task.h
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/model/persistent_pages_consistency_check_task_unittest.cc
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/model/temporary_pages_consistency_check_task.cc
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/model/temporary_pages_consistency_check_task.h
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/model/temporary_pages_consistency_check_task_unittest.cc
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/components/offline_pages/core/offline_store_types.h
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/73e94b9aeb8e440db2929b9f7be1dfd0d8ee6cc2/tools/metrics/histograms/histograms.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 14 2017

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

commit 8cc1a2d668d50ad6081aa53cbf48846ad663424e
Author: Yafei Duan <romax@chromium.org>
Date: Thu Dec 14 02:08:17 2017

[Offline Pages] Adding event counter UMAs to OPMTaskified.

Adding the following UMAs to OfflinePageModelTaskified:
- SavePageCount, recorded when a page is saved.
- AccessPageCount, recorded when a page is accessed.
- DeletePageCount, recorded when a page gets deleted. (not including
  deletions made during clearing temporary pages and consistency check).
The samples of these UMAs are enums of the Offline Pages namespaces.

Also added corresponding tests in the existing unit tests.

Bug:  772204 
Change-Id: I25cda79b7986f37ec5b4d1663868aac7b022912a
Reviewed-on: https://chromium-review.googlesource.com/815937
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523984}
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/BUILD.gn
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/client_namespace_constants.h
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/mark_page_accessed_task.cc
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/mark_page_accessed_task_unittest.cc
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/offline_page_item_generator.h
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/offline_page_model_taskified.cc
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[add] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/offline_page_model_utils.cc
[add] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/offline_page_model_utils.h
[add] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/components/offline_pages/core/model/offline_page_model_utils_unittest.cc
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/8cc1a2d668d50ad6081aa53cbf48846ad663424e/tools/metrics/histograms/histograms.xml

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 17 2017

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

commit dc171dd33a0d6d491d05d111e318956d9428c35c
Author: Yafei Duan <romax@chromium.org>
Date: Sun Dec 17 07:30:50 2017

[Offline Pages] Adding UMA for saving and deleting pages.

Adding UMA for saving and deleting pages:
- Refactored DeletePageTask for metric collection.
- Added new histogram names and made old ones obsolete.
- Updated related tests with histogram tester.

This CL also contains:
- Renamed test_util to test_utils for consistency.
- Added offline_pages::model_utils for utility methods used across all
  tasks and model itself.
- Updated OfflinePageItemGenerator for testing support

Bug:  772204 
Change-Id: I894da05d510addbf01f3dd9b7ccda29b27404752
Reviewed-on: https://chromium-review.googlesource.com/811829
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524619}
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/BUILD.gn
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/clear_legacy_temporary_pages_task_unittest.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/clear_storage_task_unittest.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/delete_page_task.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/delete_page_task_unittest.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_item_generator.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_item_generator.h
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_model_taskified.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_model_taskified.h
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_model_utils.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_model_utils.h
[rename] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_test_utils.cc
[rename] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/offline_page_test_utils.h
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/persistent_pages_consistency_check_task_unittest.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/components/offline_pages/core/model/temporary_pages_consistency_check_task_unittest.cc
[modify] https://crrev.com/dc171dd33a0d6d491d05d111e318956d9428c35c/tools/metrics/histograms/histograms.xml

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 23 2017

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

commit b7f44f417e6e34c2713c0d08eef18184b2bfb6e7
Author: Yafei Duan <romax@chromium.org>
Date: Sat Dec 23 00:16:59 2017

[Offline Pages] Make metric DeletePage.AccessCount obsolete.

The metric is no longer used since the switch to the taskified model, so
marking as obsolete.

Bug:  772204 
Change-Id: I4a976594eed3c47a6a72bdc9ca606cd6287f775b
Reviewed-on: https://chromium-review.googlesource.com/841748
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526100}
[modify] https://crrev.com/b7f44f417e6e34c2713c0d08eef18184b2bfb6e7/tools/metrics/histograms/histograms.xml

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 9 2018

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

commit a0805d4e068d1e041f5115e9375d15744526ff3b
Author: Yafei Duan <romax@chromium.org>
Date: Tue Jan 09 04:21:24 2018

[Offline Pages] Adding metrics for ClearStorage and PageAccessInterval.

Adding histograms for ClearStorage and PageAccessInterval, which is part
of the UMA migration for OPMTaskified.

This CL contains:
- Adding ClearTemporaryPages.BatchSize and Result.
- Adding PageAccessInterval.
- Change the util method for appending namespace to histogram names from
  using client id to string.
- Related tests added/changed.

Bug:  772204 
Change-Id: Ifa0ccf2d369c6d610d5434bb30cd6b8be6ebc70a
Reviewed-on: https://chromium-review.googlesource.com/850999
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527896}
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/delete_page_task.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/delete_page_task_unittest.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/mark_page_accessed_task.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/mark_page_accessed_task_unittest.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/offline_page_model_taskified.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/offline_page_model_utils.cc
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/components/offline_pages/core/model/offline_page_model_utils.h
[modify] https://crrev.com/a0805d4e068d1e041f5115e9375d15744526ff3b/tools/metrics/histograms/histograms.xml

Comment 7 by romax@chromium.org, Jan 17 2018

Status: Started (was: Assigned)

Comment 8 by romax@chromium.org, Jan 24 2018

Status: Fixed (was: Started)
The migration seems completed, with low priority ones deferred.
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 5 2018

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

commit 26f4203c68f9f01fa84fa1a028b4f997f3dd9174
Author: Yafei Duan <romax@chromium.org>
Date: Mon Feb 05 21:15:46 2018

[Offline Pages] Marking metrics obsolete.

Since Offline Pages migrated from OfflinePageModelImpl to OPMTaskified,
there are a bunch of metrics that will no longer be collected. These
weren't marked as obsolete when the migration happened, so fixing now.

Bug:  772204 
Change-Id: I25f141aae67026e5cc7d7287f5f58991dfed8a9a
Reviewed-on: https://chromium-review.googlesource.com/902011
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534494}
[modify] https://crrev.com/26f4203c68f9f01fa84fa1a028b4f997f3dd9174/tools/metrics/histograms/histograms.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 30 2018

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

commit a65ecd17be064dfb7720ec40b2e2bb2b4df9acae
Author: Yafei Duan <romax@chromium.org>
Date: Fri Mar 30 02:03:43 2018

[Offline Pages] Collecting storage usage UMAs.

Introducing new OfflinePages.StorageInfo metrics which are collected
after every time an offline page is successfully saved.

The new metrics will report storage info for both internal and external
volumes:
- internal refers to the volume that contains the Chrome app.
- external refers to the volume that contains public download directory.

Also marking old UMAs as deprecated, as they are no longer collected in
taskified offline page model.

Bug:  772204 
Change-Id: Id3e7ed89d2106d0c255bc708be8a0b131f8f41f2
Reviewed-on: https://chromium-review.googlesource.com/976688
Reviewed-by: Dmitry Titov <dimich@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547070}
[modify] https://crrev.com/a65ecd17be064dfb7720ec40b2e2bb2b4df9acae/components/offline_pages/core/archive_manager.h
[modify] https://crrev.com/a65ecd17be064dfb7720ec40b2e2bb2b4df9acae/components/offline_pages/core/archive_manager_unittest.cc
[modify] https://crrev.com/a65ecd17be064dfb7720ec40b2e2bb2b4df9acae/components/offline_pages/core/model/offline_page_model_taskified.cc
[modify] https://crrev.com/a65ecd17be064dfb7720ec40b2e2bb2b4df9acae/components/offline_pages/core/model/offline_page_model_taskified_unittest.cc
[modify] https://crrev.com/a65ecd17be064dfb7720ec40b2e2bb2b4df9acae/tools/metrics/histograms/histograms.xml

Sign in to add a comment