Commit db86c4aa authored by Becca Hughes's avatar Becca Hughes Committed by Commit Bot

[Audible Metrics] Fix concurrent playback histogram

The CloseNewestToExitConcurrentPlayback histogram was not
being recorded because of a race condition between closing
the tab and becoming inaudible and the web contents being
destroyed. This changes the metrics to check if a web
contents was recently audible instead of whether it is
currently audible.

BUG=936196

Change-Id: Ie3c1d8bbb0e11b099acbc2c163aeedf4b1c9d58f
Reviewed-on: https://chromium-review.googlesource.com/c/1490494Reviewed-by: default avatarFrank Liberato <liberato@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636274}
parent c4328744
...@@ -30,15 +30,16 @@ void AudibleMetrics::UpdateAudibleWebContentsState( ...@@ -30,15 +30,16 @@ void AudibleMetrics::UpdateAudibleWebContentsState(
RemoveAudibleWebContents(web_contents); RemoveAudibleWebContents(web_contents);
} }
void AudibleMetrics::WebContentsDestroyed(const WebContents* web_contents) { void AudibleMetrics::WebContentsDestroyed(const WebContents* web_contents,
if (audible_web_contents_.find(web_contents) == audible_web_contents_.end()) bool recently_audible) {
return; if (base::ContainsKey(audible_web_contents_, web_contents))
RemoveAudibleWebContents(web_contents);
// If we have two web contents and we go down to one, we should record // If we have two web contents and we go down to one, we should record
// whether we destroyed the most recent one. This is used to determine // whether we destroyed the most recent one. This is used to determine
// whether a user closes a new or old tab after starting playback if // whether a user closes a new or old tab after starting playback if
// they have multiple tabs. // they have multiple tabs.
if (audible_web_contents_.size() == 2) { if (audible_web_contents_.size() == 1 && recently_audible) {
ExitConcurrentPlaybackContents value = ExitConcurrentPlaybackContents value =
last_audible_web_contents_.back() == web_contents last_audible_web_contents_.back() == web_contents
? ExitConcurrentPlaybackContents::kMostRecent ? ExitConcurrentPlaybackContents::kMostRecent
...@@ -48,7 +49,7 @@ void AudibleMetrics::WebContentsDestroyed(const WebContents* web_contents) { ...@@ -48,7 +49,7 @@ void AudibleMetrics::WebContentsDestroyed(const WebContents* web_contents) {
"Media.Audible.CloseNewestToExitConcurrentPlayback", value); "Media.Audible.CloseNewestToExitConcurrentPlayback", value);
} }
RemoveAudibleWebContents(web_contents); last_audible_web_contents_.remove(web_contents);
} }
void AudibleMetrics::SetClockForTest(const base::TickClock* test_clock) { void AudibleMetrics::SetClockForTest(const base::TickClock* test_clock) {
...@@ -61,6 +62,10 @@ void AudibleMetrics::AddAudibleWebContents(const WebContents* web_contents) { ...@@ -61,6 +62,10 @@ void AudibleMetrics::AddAudibleWebContents(const WebContents* web_contents) {
1, 10, 11); 1, 10, 11);
audible_web_contents_.insert(web_contents); audible_web_contents_.insert(web_contents);
// Since the web contents is newly audible then move it to the back of the
// last audible web contents list.
last_audible_web_contents_.remove(web_contents);
last_audible_web_contents_.push_back(web_contents); last_audible_web_contents_.push_back(web_contents);
if (audible_web_contents_.size() > 1 && if (audible_web_contents_.size() > 1 &&
...@@ -82,7 +87,6 @@ void AudibleMetrics::AddAudibleWebContents(const WebContents* web_contents) { ...@@ -82,7 +87,6 @@ void AudibleMetrics::AddAudibleWebContents(const WebContents* web_contents) {
void AudibleMetrics::RemoveAudibleWebContents(const WebContents* web_contents) { void AudibleMetrics::RemoveAudibleWebContents(const WebContents* web_contents) {
audible_web_contents_.erase(web_contents); audible_web_contents_.erase(web_contents);
last_audible_web_contents_.remove(web_contents);
if (audible_web_contents_.size() <= 1 && if (audible_web_contents_.size() <= 1 &&
!concurrent_web_contents_start_time_.is_null()) { !concurrent_web_contents_start_time_.is_null()) {
......
...@@ -40,7 +40,8 @@ class CONTENT_EXPORT AudibleMetrics { ...@@ -40,7 +40,8 @@ class CONTENT_EXPORT AudibleMetrics {
void UpdateAudibleWebContentsState(const WebContents* web_contents, void UpdateAudibleWebContentsState(const WebContents* web_contents,
bool audible); bool audible);
void WebContentsDestroyed(const WebContents* web_contents); void WebContentsDestroyed(const WebContents* web_contents,
bool recently_audible);
void SetClockForTest(const base::TickClock* test_clock); void SetClockForTest(const base::TickClock* test_clock);
...@@ -56,9 +57,14 @@ class CONTENT_EXPORT AudibleMetrics { ...@@ -56,9 +57,14 @@ class CONTENT_EXPORT AudibleMetrics {
size_t max_concurrent_audible_web_contents_in_session_; size_t max_concurrent_audible_web_contents_in_session_;
const base::TickClock* clock_; const base::TickClock* clock_;
// This stores the audible web contents in insertion order. // This stores the audible web contents in insertion order. We add a
// web contents to the list when it becomes audible and remove it is
// destroyed.
std::list<const WebContents*> last_audible_web_contents_; std::list<const WebContents*> last_audible_web_contents_;
// Stores all the web contents that are currently audible. We add a web
// contents to the set when it becomes currently audible and remove it when it
// is no longer audible.
std::set<const WebContents*> audible_web_contents_; std::set<const WebContents*> audible_web_contents_;
DISALLOW_COPY_AND_ASSIGN(AudibleMetrics); DISALLOW_COPY_AND_ASSIGN(AudibleMetrics);
......
...@@ -72,7 +72,12 @@ MediaWebContentsObserver::MediaWebContentsObserver(WebContents* web_contents) ...@@ -72,7 +72,12 @@ MediaWebContentsObserver::MediaWebContentsObserver(WebContents* web_contents)
MediaWebContentsObserver::~MediaWebContentsObserver() = default; MediaWebContentsObserver::~MediaWebContentsObserver() = default;
void MediaWebContentsObserver::WebContentsDestroyed() { void MediaWebContentsObserver::WebContentsDestroyed() {
audible_metrics_->WebContentsDestroyed(web_contents()); AudioStreamMonitor* audio_stream_monitor =
web_contents_impl()->audio_stream_monitor();
audible_metrics_->WebContentsDestroyed(
web_contents(), audio_stream_monitor->WasRecentlyAudible() &&
!web_contents()->IsAudioMuted());
} }
void MediaWebContentsObserver::RenderFrameDeleted( void MediaWebContentsObserver::RenderFrameDeleted(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment