Commit a8827c34 authored by Tommy Steimel's avatar Tommy Steimel Committed by Commit Bot

GMC: Only count click to go back to tabs as interactions

This CL updates the auto-dismiss logic to only count click-to-tabs as
interactions instead of counting when the tab is focused by any means.
The only reason we had it that way was to accommodate the
"pre-refactor" world where it wasn't feasible to always detect the
difference.

Bug: 1026079
Change-Id: I8806f64ade7019716413ef54c07693574f192dfc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1937141Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719586}
parent 5b12c493
...@@ -81,11 +81,6 @@ void MediaNotificationService::Session::WebContentsDestroyed() { ...@@ -81,11 +81,6 @@ void MediaNotificationService::Session::WebContentsDestroyed() {
owner_->RemoveItem(id_); owner_->RemoveItem(id_);
} }
void MediaNotificationService::Session::OnWebContentsFocused(
content::RenderWidgetHost*) {
OnSessionInteractedWith();
}
void MediaNotificationService::Session::MediaSessionInfoChanged( void MediaNotificationService::Session::MediaSessionInfoChanged(
media_session::mojom::MediaSessionInfoPtr session_info) { media_session::mojom::MediaSessionInfoPtr session_info) {
bool playing = bool playing =
...@@ -364,6 +359,8 @@ void MediaNotificationService::OnContainerClicked(const std::string& id) { ...@@ -364,6 +359,8 @@ void MediaNotificationService::OnContainerClicked(const std::string& id) {
if (it == sessions_.end()) if (it == sessions_.end())
return; return;
it->second.OnSessionInteractedWith();
content::WebContents* web_contents = it->second.web_contents(); content::WebContents* web_contents = it->second.web_contents();
if (!web_contents) if (!web_contents)
return; return;
......
...@@ -134,7 +134,6 @@ class MediaNotificationService ...@@ -134,7 +134,6 @@ class MediaNotificationService
// content::WebContentsObserver implementation. // content::WebContentsObserver implementation.
void WebContentsDestroyed() override; void WebContentsDestroyed() override;
void OnWebContentsFocused(content::RenderWidgetHost*) override;
// media_session::mojom::MediaControllerObserver: // media_session::mojom::MediaControllerObserver:
void MediaSessionInfoChanged( void MediaSessionInfoChanged(
...@@ -163,10 +162,10 @@ class MediaNotificationService ...@@ -163,10 +162,10 @@ class MediaNotificationService
// called if the value has not already been set. // called if the value has not already been set.
void set_dismiss_reason(GlobalMediaControlsDismissReason reason); void set_dismiss_reason(GlobalMediaControlsDismissReason reason);
private:
// Called when a session is interacted with (to reset |inactive_timer_|). // Called when a session is interacted with (to reset |inactive_timer_|).
void OnSessionInteractedWith(); void OnSessionInteractedWith();
private:
void StartInactiveTimer(); void StartInactiveTimer();
void OnInactiveTimerFired(); void OnInactiveTimerFired();
......
...@@ -240,12 +240,6 @@ class MediaNotificationServiceTest : public testing::Test { ...@@ -240,12 +240,6 @@ class MediaNotificationServiceTest : public testing::Test {
item_itr->second.WebContentsDestroyed(); item_itr->second.WebContentsDestroyed();
} }
void SimulateTabFocused(const base::UnguessableToken& id) {
auto item_itr = service_->sessions_.find(id.ToString());
EXPECT_NE(service_->sessions_.end(), item_itr);
item_itr->second.OnWebContentsFocused(nullptr);
}
void SimulatePlaybackStateChanged(const base::UnguessableToken& id, void SimulatePlaybackStateChanged(const base::UnguessableToken& id,
bool playing) { bool playing) {
MediaSessionInfoPtr session_info(MediaSessionInfo::New()); MediaSessionInfoPtr session_info(MediaSessionInfo::New());
...@@ -265,6 +259,10 @@ class MediaNotificationServiceTest : public testing::Test { ...@@ -265,6 +259,10 @@ class MediaNotificationServiceTest : public testing::Test {
item_itr->second.MediaSessionPositionChanged(base::nullopt); item_itr->second.MediaSessionPositionChanged(base::nullopt);
} }
void SimulateNotificationClicked(const base::UnguessableToken& id) {
service_->OnContainerClicked(id.ToString());
}
void SimulateDismissButtonClicked(const base::UnguessableToken& id) { void SimulateDismissButtonClicked(const base::UnguessableToken& id) {
service_->OnContainerDismissed(id.ToString()); service_->OnContainerDismissed(id.ToString());
} }
...@@ -734,10 +732,10 @@ TEST_F(MediaNotificationServiceTest, DelaysHidingNotifications_Interactions) { ...@@ -734,10 +732,10 @@ TEST_F(MediaNotificationServiceTest, DelaysHidingNotifications_Interactions) {
AdvanceClockMinutes(59); AdvanceClockMinutes(59);
EXPECT_TRUE(HasActiveNotifications()); EXPECT_TRUE(HasActiveNotifications());
// If the user goes back to the tab, it should reset the hide timer. // If the user clicks to go back to the tab, it should reset the hide timer.
ExpectHistogramInteractionDelayAfterPause(base::TimeDelta::FromMinutes(59), ExpectHistogramInteractionDelayAfterPause(base::TimeDelta::FromMinutes(59),
0); 0);
SimulateTabFocused(id); SimulateNotificationClicked(id);
ExpectHistogramInteractionDelayAfterPause(base::TimeDelta::FromMinutes(59), ExpectHistogramInteractionDelayAfterPause(base::TimeDelta::FromMinutes(59),
1); 1);
AdvanceClockMinutes(50); AdvanceClockMinutes(50);
......
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