Skip to content

Commit

Permalink
[battery] Add the usage scenario event providers and enable UKMs
Browse files Browse the repository at this point in the history
This includes some fixes for TabUsageScenarioTracker::OnTabReplaced and
some new tests for the new code.

(cherry picked from commit 319a6c9)

Bug: 1153193
Change-Id: I88392d6db67495b2a186c0b65f76dd8056179a49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2720983
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#858748}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2741565
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Sébastien Marchand <sebmarchand@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4430@{#207}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
  • Loading branch information
sebmarchand authored and Chromium LUCI CQ committed Mar 7, 2021
1 parent 7268813 commit 8af5675
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 47 deletions.
16 changes: 8 additions & 8 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -828,8 +828,6 @@ static_library("browser") {
"metrics/oom/out_of_memory_reporter.h",
"metrics/power/battery_level_provider.cc",
"metrics/power/battery_level_provider.h",
"metrics/power/power_metrics_reporter.cc",
"metrics/power/power_metrics_reporter.h",
"metrics/process_memory_metrics_emitter.cc",
"metrics/process_memory_metrics_emitter.h",
"metrics/renderer_uptime_tracker.cc",
Expand All @@ -848,12 +846,6 @@ static_library("browser") {
"metrics/thread_watcher_report_hang.h",
"metrics/ukm_background_recorder_service.cc",
"metrics/ukm_background_recorder_service.h",
"metrics/usage_scenario/usage_scenario_data_store.cc",
"metrics/usage_scenario/usage_scenario_data_store.h",
"metrics/usage_scenario/usage_scenario_tracker.cc",
"metrics/usage_scenario/usage_scenario_tracker.h",
"metrics/usage_scenario/video_capture_event_provider.cc",
"metrics/usage_scenario/video_capture_event_provider.h",
"metrics/variations/chrome_variations_service_client.cc",
"metrics/variations/chrome_variations_service_client.h",
"native_window_notification_source.h",
Expand Down Expand Up @@ -3759,6 +3751,8 @@ static_library("browser") {
"metrics/first_web_contents_profiler.cc",
"metrics/first_web_contents_profiler.h",
"metrics/incognito_observer_desktop.cc",
"metrics/power/power_metrics_reporter.cc",
"metrics/power/power_metrics_reporter.h",
"metrics/tab_stats/tab_stats_data_store.cc",
"metrics/tab_stats/tab_stats_data_store.h",
"metrics/tab_stats/tab_stats_observer.h",
Expand All @@ -3767,6 +3761,12 @@ static_library("browser") {
"metrics/tab_stats/tab_stats_tracker_delegate.h",
"metrics/usage_scenario/tab_usage_scenario_tracker.cc",
"metrics/usage_scenario/tab_usage_scenario_tracker.h",
"metrics/usage_scenario/usage_scenario_data_store.cc",
"metrics/usage_scenario/usage_scenario_data_store.h",
"metrics/usage_scenario/usage_scenario_tracker.cc",
"metrics/usage_scenario/usage_scenario_tracker.h",
"metrics/usage_scenario/video_capture_event_provider.cc",
"metrics/usage_scenario/video_capture_event_provider.h",
"notifications/muted_notification_handler.cc",
"notifications/muted_notification_handler.h",
"notifications/notification_system_observer.cc",
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/metrics/power/power_metrics_reporter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@ void PowerMetricsReporter::OnAggregatedMetricsSampled(
ReportHistograms(sampling_interval, interval_duration,
discharge_mode_and_rate.first,
discharge_mode_and_rate.second);
if (report_ukms_) {
ReportUKMs(metrics, interval_duration, discharge_mode_and_rate.first,
discharge_mode_and_rate.second);
}

ReportUKMs(metrics, interval_duration, discharge_mode_and_rate.first,
discharge_mode_and_rate.second);
}

void PowerMetricsReporter::ReportHistograms(
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/metrics/power/power_metrics_reporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ class PowerMetricsReporter
return battery_state_;
}

void EnableUKMReportingForTesting() { report_ukms_ = true; }

protected:
// Any change to this enum should be reflected in the corresponding enums.xml
// and ukm.xml
Expand Down Expand Up @@ -95,10 +93,6 @@ class PowerMetricsReporter
// The first interval will start when this class gets created.
base::TimeTicks interval_begin_ = base::TimeTicks::Now();

// Indicates if ukms should be reported upon OnAggregatedMetricsSampled().
// TODO(1153193): Remove once ukm is recorded.
bool report_ukms_ = false;

SEQUENCE_CHECKER(sequence_checker_);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ class PowerMetricsReporterUnitTest : public testing::Test {
battery_provider_ = battery_provider.get();
power_metrics_reporter_ = std::make_unique<PowerMetricsReporter>(
data_store_.AsWeakPtr(), std::move(battery_provider));
power_metrics_reporter_->EnableUKMReportingForTesting();
}

protected:
Expand Down
60 changes: 36 additions & 24 deletions chrome/browser/metrics/usage_scenario/tab_usage_scenario_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,27 +66,22 @@ void TabUsageScenarioTracker::OnTabAdded(content::WebContents* web_contents) {

void TabUsageScenarioTracker::OnTabRemoved(content::WebContents* web_contents) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto iter = visible_tabs_.find(web_contents);
DCHECK_EQ(iter != visible_tabs_.end(),
web_contents->GetVisibility() == content::Visibility::VISIBLE);
auto video_iter = contents_playing_video_.find(web_contents);
// If |web_contents| is tracked in the list of visible WebContents then a
// synthetic visibility change event should be emitted.
if (iter != visible_tabs_.end()) {
OnTabBecameHidden(&iter);
}
if (video_iter != contents_playing_video_.end()) {
contents_playing_video_.erase(video_iter);
}
OnWebContentsRemoved(web_contents);
usage_scenario_data_store_->OnTabClosed();
}

void TabUsageScenarioTracker::OnTabReplaced(
content::WebContents* old_contents,
content::WebContents* new_contents) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
OnWebContentsRemoved(old_contents);
DCHECK(!base::Contains(visible_tabs_, old_contents));
DCHECK(!base::Contains(contents_playing_video_, old_contents));
DCHECK_NE(content_with_media_playing_fullscreen_, old_contents);

// Start tracking |new_contents| if needed.
if (new_contents->GetVisibility() == content::Visibility::VISIBLE)
OnTabVisibilityChanged(new_contents);
}

void TabUsageScenarioTracker::OnTabVisibilityChanged(
Expand Down Expand Up @@ -216,18 +211,6 @@ void TabUsageScenarioTracker::OnDisplayRemoved(const display::Display& unused) {
}
}

void TabUsageScenarioTracker::InsertContentsInMapOfVisibleTabs(
content::WebContents* web_contents) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!base::Contains(visible_tabs_, web_contents));
auto iter = visible_tabs_.emplace(web_contents,
GetNavigationInfoForContents(web_contents));
if (iter.first->second.first != ukm::kInvalidSourceId) {
usage_scenario_data_store_->OnUkmSourceBecameVisible(
iter.first->second.first, iter.first->second.second);
}
}

void TabUsageScenarioTracker::OnContentStoppedPlayingMediaFullScreen() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
usage_scenario_data_store_->OnFullScreenVideoEndsOnSingleMonitor();
Expand Down Expand Up @@ -259,4 +242,33 @@ void TabUsageScenarioTracker::OnTabBecameHidden(
usage_scenario_data_store_->OnWindowHidden();
}

void TabUsageScenarioTracker::OnWebContentsRemoved(
content::WebContents* web_contents) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto iter = visible_tabs_.find(web_contents);
DCHECK_EQ(iter != visible_tabs_.end(),
web_contents->GetVisibility() == content::Visibility::VISIBLE);
auto video_iter = contents_playing_video_.find(web_contents);
// If |web_contents| is tracked in the list of visible WebContents then a
// synthetic visibility change event should be emitted.
if (iter != visible_tabs_.end()) {
OnTabBecameHidden(&iter);
}
if (video_iter != contents_playing_video_.end()) {
contents_playing_video_.erase(video_iter);
}
}

void TabUsageScenarioTracker::InsertContentsInMapOfVisibleTabs(
content::WebContents* web_contents) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!base::Contains(visible_tabs_, web_contents));
auto iter = visible_tabs_.emplace(web_contents,
GetNavigationInfoForContents(web_contents));
if (iter.first->second.first != ukm::kInvalidSourceId) {
usage_scenario_data_store_->OnUkmSourceBecameVisible(
iter.first->second.first, iter.first->second.second);
}
}

} // namespace metrics
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ class TabUsageScenarioTracker : public TabStatsObserver,
// visible. |visible_tab_iter| should be an iterator in |visible_contents_|.
void OnTabBecameHidden(VisibleTabsMap::iterator* visible_tab_iter);

// Should be called when a WebContents is being destroyed, there's 2 possible
// causes for this:
// - The tab that contains it is being removed.
// - A tab's WebContents is being replaced.
void OnWebContentsRemoved(content::WebContents* web_contents);

void InsertContentsInMapOfVisibleTabs(content::WebContents* web_contents);

// Non-owning. Needs to outlive this class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ IN_PROC_BROWSER_TEST_F(TabUsageScenarioTrackerBrowserTest, TabCrash) {
IN_PROC_BROWSER_TEST_F(TabUsageScenarioTrackerBrowserTest, TabDiscard) {
AddTabAtIndex(1, embedded_test_server()->GetURL("/title2.html"),
ui::PAGE_TRANSITION_LINK);
EXPECT_EQ(content::Visibility::VISIBLE,
browser()->tab_strip_model()->GetWebContentsAt(1)->GetVisibility());
tick_clock_.Advance(kInterval);

auto interval_data = data_store_.ResetIntervalData();
Expand All @@ -290,9 +292,53 @@ IN_PROC_BROWSER_TEST_F(TabUsageScenarioTrackerBrowserTest, TabDiscard) {
interval_data.source_id_for_longest_visible_origin_duration);

// Induce a discard of the active tab.
tick_clock_.Advance(kInterval * 2);
DiscardTab(browser()->tab_strip_model()->GetWebContentsAt(1));
tick_clock_.Advance(kInterval);
interval_data = data_store_.ResetIntervalData();
EXPECT_EQ(2U, interval_data.max_tab_count);
EXPECT_EQ(1U, interval_data.max_visible_window_count);
EXPECT_EQ(0U, interval_data.top_level_navigation_count);
EXPECT_EQ(0U, interval_data.tabs_closed_during_interval);
EXPECT_TRUE(
interval_data.time_playing_video_full_screen_single_monitor.is_zero());
EXPECT_TRUE(interval_data.time_with_open_webrtc_connection.is_zero());
EXPECT_TRUE(interval_data.time_playing_video_in_visible_tab.is_zero());
EXPECT_EQ(expected_source_id,
interval_data.source_id_for_longest_visible_origin);
EXPECT_EQ(kInterval * 2,
interval_data.source_id_for_longest_visible_origin_duration);

// Do a navigation on the discarded tab.
EXPECT_TRUE(
content::NavigateToURL(browser()->tab_strip_model()->GetWebContentsAt(1),
embedded_test_server()->GetURL("/title2.html")));
expected_source_id = ukm::GetSourceIdForWebContentsDocument(
browser()->tab_strip_model()->GetWebContentsAt(1));
tick_clock_.Advance(kInterval);
interval_data = data_store_.ResetIntervalData();
EXPECT_EQ(2U, interval_data.max_tab_count);
EXPECT_EQ(1U, interval_data.max_visible_window_count);
EXPECT_EQ(1U, interval_data.top_level_navigation_count);
EXPECT_EQ(0U, interval_data.tabs_closed_during_interval);
EXPECT_TRUE(
interval_data.time_playing_video_full_screen_single_monitor.is_zero());
EXPECT_TRUE(interval_data.time_with_open_webrtc_connection.is_zero());
EXPECT_TRUE(interval_data.time_playing_video_in_visible_tab.is_zero());
EXPECT_EQ(expected_source_id,
interval_data.source_id_for_longest_visible_origin);
EXPECT_EQ(kInterval,
interval_data.source_id_for_longest_visible_origin_duration);

// Same tests but with this time the discarded tab is hidden.
browser()->tab_strip_model()->ActivateTabAt(0);
EXPECT_EQ(content::Visibility::VISIBLE,
browser()->tab_strip_model()->GetWebContentsAt(0)->GetVisibility());
tick_clock_.Advance(kInterval * 2);
DiscardTab(browser()->tab_strip_model()->GetWebContentsAt(1));
tick_clock_.Advance(kInterval);
expected_source_id = ukm::GetSourceIdForWebContentsDocument(
browser()->tab_strip_model()->GetWebContentsAt(0));
interval_data = data_store_.ResetIntervalData();
EXPECT_EQ(2U, interval_data.max_tab_count);
EXPECT_EQ(1U, interval_data.max_visible_window_count);
Expand All @@ -302,6 +348,25 @@ IN_PROC_BROWSER_TEST_F(TabUsageScenarioTrackerBrowserTest, TabDiscard) {
interval_data.time_playing_video_full_screen_single_monitor.is_zero());
EXPECT_TRUE(interval_data.time_with_open_webrtc_connection.is_zero());
EXPECT_TRUE(interval_data.time_playing_video_in_visible_tab.is_zero());
EXPECT_EQ(expected_source_id,
interval_data.source_id_for_longest_visible_origin);
EXPECT_EQ(kInterval * 3,
interval_data.source_id_for_longest_visible_origin_duration);

// Do a navigation on the discarded tab.
EXPECT_TRUE(
content::NavigateToURL(browser()->tab_strip_model()->GetWebContentsAt(1),
embedded_test_server()->GetURL("/title2.html")));
tick_clock_.Advance(kInterval);
interval_data = data_store_.ResetIntervalData();
EXPECT_EQ(2U, interval_data.max_tab_count);
EXPECT_EQ(1U, interval_data.max_visible_window_count);
EXPECT_EQ(1U, interval_data.top_level_navigation_count);
EXPECT_EQ(0U, interval_data.tabs_closed_during_interval);
EXPECT_TRUE(
interval_data.time_playing_video_full_screen_single_monitor.is_zero());
EXPECT_TRUE(interval_data.time_with_open_webrtc_connection.is_zero());
EXPECT_TRUE(interval_data.time_playing_video_in_visible_tab.is_zero());
EXPECT_EQ(expected_source_id,
interval_data.source_id_for_longest_visible_origin);
EXPECT_EQ(kInterval,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@

#include "chrome/browser/metrics/usage_scenario/usage_scenario_tracker.h"

UsageScenarioTracker::UsageScenarioTracker() {}
#include "chrome/browser/metrics/tab_stats/tab_stats_tracker.h"

UsageScenarioTracker::UsageScenarioTracker()
: tab_usage_scenario_tracker_(&data_store_),
video_capture_event_provider_(&data_store_) {
metrics::TabStatsTracker::GetInstance()->AddObserverAndSetInitialState(
&tab_usage_scenario_tracker_);
}

UsageScenarioTracker::~UsageScenarioTracker() = default;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#define CHROME_BROWSER_METRICS_USAGE_SCENARIO_USAGE_SCENARIO_TRACKER_H_

#include "base/sequence_checker.h"
#include "chrome/browser/metrics/usage_scenario/tab_usage_scenario_tracker.h"
#include "chrome/browser/metrics/usage_scenario/usage_scenario_data_store.h"
#include "chrome/browser/metrics/usage_scenario/video_capture_event_provider.h"

// Registers as an observer to various components to maintain a
// UsageScenarioDataStore.
Expand All @@ -25,7 +27,11 @@ class UsageScenarioTracker {
private:
UsageScenarioDataStoreImpl data_store_;

// TODO(crbug.com/1153193): Add the events providers for the data store.
// Track tab related information.
metrics::TabUsageScenarioTracker tab_usage_scenario_tracker_;

// Tracks tabs capturing video.
VideoCaptureEventProvider video_capture_event_provider_;

SEQUENCE_CHECKER(sequence_checker_);
};
Expand Down
4 changes: 2 additions & 2 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3585,11 +3585,9 @@ test("unit_tests") {
"../browser/metrics/chrome_metrics_services_manager_client_unittest.cc",
"../browser/metrics/oom/out_of_memory_reporter_unittest.cc",
"../browser/metrics/power/battery_level_provider_unittest.cc",
"../browser/metrics/power/power_metrics_reporter_unittest.cc",
"../browser/metrics/process_memory_metrics_emitter_unittest.cc",
"../browser/metrics/tab_footprint_aggregator_unittest.cc",
"../browser/metrics/thread_watcher_unittest.cc",
"../browser/metrics/usage_scenario/usage_scenario_data_store_unittest.cc",
"../browser/navigation_predictor/navigation_predictor_renderer_warmup_client_unittest.cc",
"../browser/navigation_predictor/navigation_predictor_unittest.cc",
"../browser/net/chrome_network_delegate_unittest.cc",
Expand Down Expand Up @@ -4067,7 +4065,9 @@ test("unit_tests") {
"../browser/media/feeds/media_feeds_converter_unittest.cc",
"../browser/media/feeds/media_feeds_fetcher_unittest.cc",
"../browser/media/feeds/media_feeds_service_unittest.cc",
"../browser/metrics/power/power_metrics_reporter_unittest.cc",
"../browser/metrics/usage_scenario/tab_usage_scenario_tracker_unittest.cc",
"../browser/metrics/usage_scenario/usage_scenario_data_store_unittest.cc",
"../browser/notifications/muted_notification_handler_unittest.cc",
"../browser/notifications/screen_capture_notification_blocker_unittest.cc",
"../browser/password_manager/generated_password_leak_detection_pref_unittest.cc",
Expand Down

0 comments on commit 8af5675

Please sign in to comment.