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

Issue 761524 link

Starred by 1 user
Status: Fixed
Owner:
█ holte@chromium.org
Last visit > 30 days ago
Closed: Apr 2018
Cc:
marinaciocea@chromium.org
khushals...@chromium.org
skare@chromium.org
isherman@chromium.org
asvitk...@chromium.org
█ rkaplow@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Improve TestUkmRecorder API

Project Member Reported by holte@chromium.org, Sep 1 2017 
TestUkmRecorder API should be improved to better handle the current usage expectations.

We expect many UKM entries to migrate to being recorded with NavigationIds instead of recording their own URLs and the recorder interface needs to have alternatives to URL as a way of checking which source the event is associated with.

We may also want to make some changes based on the switch to using codegenerated APIs to record events.

Comment 1 by holte@chromium.org, Sep 1 2017

Owner: holte@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 6 2017 

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

commit fb7e2fc2b070755afd2277d6d37996d3f5f503d9
Author: Steven Holte <holte@google.com>
Date: Wed Sep 06 01:10:42 2017

Fix ProcessMemoryMetricsEmitterTest.

Adds methods to TestUkmRecorder to support getting metrics by SourceId.

Bug:  761524 
Change-Id: I443a88d0bf98b8b791af13ef144c394891f98fe5
Reviewed-on: https://chromium-review.googlesource.com/648098
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499830}
[modify] https://crrev.com/fb7e2fc2b070755afd2277d6d37996d3f5f503d9/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
[modify] https://crrev.com/fb7e2fc2b070755afd2277d6d37996d3f5f503d9/components/ukm/test_ukm_recorder.cc
[modify] https://crrev.com/fb7e2fc2b070755afd2277d6d37996d3f5f503d9/components/ukm/test_ukm_recorder.h
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 17 2017 

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

commit b04a82a245e44afe18b7dbcba55c71173d91b43e
Author: Steven Holte <holte@google.com>
Date: Fri Nov 17 21:41:58 2017

Add new UKM test APIs.

These will replace most of the existing APIs in follow up CLs.

Bug:  761524 
Change-Id: I605dcb30c74aa603faf97c4ddbf6871ff87d4c06
Reviewed-on: https://chromium-review.googlesource.com/776068
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517565}
[modify] https://crrev.com/b04a82a245e44afe18b7dbcba55c71173d91b43e/components/ukm/test_ukm_recorder.cc
[modify] https://crrev.com/b04a82a245e44afe18b7dbcba55c71173d91b43e/components/ukm/test_ukm_recorder.h
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 29 2017 

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

commit 0fad9f9e64fbd0dbc8cc8bf698a5b07c5540f397
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Wed Nov 29 21:45:47 2017

Disable ProcessMemoryMetricsEmitterTest.FetchAndEmitMetricsWithExtensionsAndHostReuse

This test breaks when attempting to record new UKMs. A CL that upgrades this
and other tests to the new UKM testing API is in progress
(crrev.com/c/774120), but in the meantime, disable the test to allow moving
forward with other UKM CLs.

Bug:  761524 
Change-Id: I587cce285b0e20f4b741ab86df8b5d58c66a7d17
Reviewed-on: https://chromium-review.googlesource.com/794976
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520257}
[modify] https://crrev.com/0fad9f9e64fbd0dbc8cc8bf698a5b07c5540f397/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 30 2017 

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

commit 290a793995560fa7d8ae15af7e3e4ff000cb52ba
Author: Steven Holte <holte@google.com>
Date: Thu Nov 30 02:47:50 2017

Convert UKM tests to express expectations over specific entries.

As part of the switch to using shared SourceIds, we will start
associating the same URL with multiple SourceIds, and this will
break all tests that currently use GetSourceForUrl and similar.

This avoids that by converting existing tests to use a new test
recorder API which lets test express expectations over the entries
they are recording.

Bug:  761524 
TBR=bmcquade

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I43f0e09e026fa1373869ea5530427f88f6317a7a
Reviewed-on: https://chromium-review.googlesource.com/774120
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Peter Williamson <petewil@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520398}
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/cc/trees/ukm_manager_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/autofill/autofill_metrics_browsertest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/content_settings/sound_content_setting_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/media/media_engagement_contents_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/metrics/oom/out_of_memory_reporter_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/local_network_requests_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/previews_ukm_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/autofill/core/browser/credit_card_save_manager_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/offline_pages/core/offline_pages_ukm_reporter_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/password_manager/core/browser/password_autofill_manager_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/password_manager/core/browser/password_form_manager_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/password_manager/core/browser/password_manager_metrics_recorder_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/payments/core/journey_logger_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/translate/core/browser/BUILD.gn
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/translate/core/browser/translate_ranker_impl_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/ukm/content/source_url_recorder_test.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/ukm/test_ukm_recorder.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/components/ukm/test_ukm_recorder.h
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/content/browser/plugin_service_impl_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
[modify] https://crrev.com/290a793995560fa7d8ae15af7e3e4ff000cb52ba/media/mojo/services/watch_time_recorder_unittest.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 1 2017 

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

commit a0270165015193e77de5e1b9a429991e2c6a8b61
Author: Steven Holte <holte@google.com>
Date: Fri Dec 01 03:23:06 2017

Partial reland of UKM test conversion.

This relands 290a793995560fa7d8ae15af7e3e4ff000cb52ba which was
reverted by 16af3fb66bf46918d36df2239a51fc29ff453a65, except
that it omits the changes to test that flaked.

TBR=asvitkine

Bug:  761524 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I6d68e0a7cab7307544c4ddda5df3d153bc3d69a4
Reviewed-on: https://chromium-review.googlesource.com/802415
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520832}
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/cc/trees/ukm_manager_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/autofill/autofill_metrics_browsertest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/content_settings/sound_content_setting_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/media/media_engagement_contents_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/metrics/oom/out_of_memory_reporter_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/metrics/process_memory_metrics_emitter_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/from_gws_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/local_network_requests_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/previews_ukm_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/service_worker_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/subresource_filter_metrics_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/chrome/browser/ui/views/payments/payment_request_journey_logger_browsertest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/autofill/core/browser/credit_card_save_manager_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/offline_pages/core/offline_pages_ukm_reporter_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/password_manager/core/browser/password_autofill_manager_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/password_manager/core/browser/password_form_manager_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/password_manager/core/browser/password_manager_metrics_recorder_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/payments/core/journey_logger_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/translate/core/browser/BUILD.gn
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/translate/core/browser/translate_ranker_impl_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/components/ukm/content/source_url_recorder_test.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/content/browser/plugin_service_impl_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/content/browser/renderer_host/input/render_widget_host_latency_tracker_unittest.cc
[modify] https://crrev.com/a0270165015193e77de5e1b9a429991e2c6a8b61/media/mojo/services/watch_time_recorder_unittest.cc

Comment 7 by khushals...@chromium.org, Dec 1 2017

Cc: khushals...@chromium.org
Project Member

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

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

commit da50f815839272713b758bdae4ef76a22610b311
Author: Khushal <khushalsagar@chromium.org>
Date: Fri Dec 01 09:04:40 2017

Reland content: Fix MojoUkmRecorder setup for compositor UKMs.

This reverts commit 041e750d63a65138b360c39e4a5924c402bcb78d. This
disables compositor ukm in browser tests since the current test setup of
TestUkmRecorder assumes a single source of URL registration in some
of these tests, which break when the compositor starts using the same
recorder. This can be removed once the tests are refactored to eliminate
this case.

TBR=piman@chromium.org, holte@chromium.org

Bug:  761524 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Id86c959fd3a335864b6c382cae69e0411123c43e
Reviewed-on: https://chromium-review.googlesource.com/803058
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520891}
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/cc/trees/proxy_impl.cc
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/content/public/common/content_switches.cc
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/content/public/common/content_switches.h
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/content/public/test/browser_test_base.cc
[modify] https://crrev.com/da50f815839272713b758bdae4ef76a22610b311/content/renderer/render_thread_impl.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 15 2017 

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

commit 2d0c4b351fb580604ce6809291290f211892ceb6
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Fri Dec 15 01:08:05 2017

Disable ProcessMemoryMetricsEmitterTest browser_tests

These tests are blocking CLs that add new, unrelated UKMs due to how
they test metric collection. Work is already in progress to update these
tests; disable them until that work lands.

Bug:  761524 
Change-Id: If5716b417c0c72dac99e49a1440b00c2f1e3466b
Reviewed-on: https://chromium-review.googlesource.com/827666
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524272}
[modify] https://crrev.com/2d0c4b351fb580604ce6809291290f211892ceb6/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc

Comment 11 by holte@chromium.org, Apr 2 2018

Status: Fixed (was: Started)
The improvements covered by this bug have all landed.

Comment 12 by marinaciocea@chromium.org, May 21 2018

Cc: marinaciocea@chromium.org
I made a change to process_memory_metrics_emitter and noticed process_memory_metrics_emitter_browsertests are currently disabled. I want to update the tests to cover utility process metrics for audio service[1]. Is there any ongoing work for these browsertests?

[1] https://crrev.com/c/1064064

Comment 13 by holte@chromium.org, May 21 2018 

I don't think there is active work on it.

There are several open bugs about flakiness with the tests:

https://bugs.chromium.org/p/chromium/issues/detail?id=732501
https://bugs.chromium.org/p/chromium/issues/detail?id=731466
https://bugs.chromium.org/p/chromium/issues/detail?id=722673

I'm not sure which of them are stale at this point.

When I landed https://crrev.com/1235efd7af18f6fc979d6290bd86c4defec0559c
I believe that fixed all the UKM related failures at that point, but I believe the tests still had failures due to what looked like stale expectations on HistogramTester.
►

Sign in to add a comment