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

Media Engagement: Do not record playback if no sound was made

If no sound was made then do not record significant playback

BUG=729998

Change-Id: I348649bfa2fefc2556717e011609ffa7813672a8
Reviewed-on: https://chromium-review.googlesource.com/537253Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarChrome Cunningham <chcunningham@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485573}
parent 4f8b5fb8
...@@ -143,6 +143,11 @@ bool MediaEngagementContentsObserver::IsSignificantPlayer( ...@@ -143,6 +143,11 @@ bool MediaEngagementContentsObserver::IsSignificantPlayer(
void MediaEngagementContentsObserver::OnSignificantMediaPlaybackTime() { void MediaEngagementContentsObserver::OnSignificantMediaPlaybackTime() {
DCHECK(!significant_playback_recorded_); DCHECK(!significant_playback_recorded_);
// Do not record significant playback if the tab did not make
// a sound in the last two seconds.
if (!web_contents()->WasRecentlyAudible())
return;
significant_playback_recorded_ = true; significant_playback_recorded_ = true;
if (committed_origin_.unique()) if (committed_origin_.unique())
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/test/web_contents_tester.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
class MediaEngagementContentsObserverTest class MediaEngagementContentsObserverTest
...@@ -22,6 +23,8 @@ class MediaEngagementContentsObserverTest ...@@ -22,6 +23,8 @@ class MediaEngagementContentsObserverTest
scoped_feature_list_.InitFromCommandLine("media-engagement", std::string()); scoped_feature_list_.InitFromCommandLine("media-engagement", std::string());
ChromeRenderViewHostTestHarness::SetUp(); ChromeRenderViewHostTestHarness::SetUp();
SetContents(content::WebContentsTester::CreateTestWebContents(
browser_context(), nullptr));
MediaEngagementService* service = MediaEngagementService::Get(profile()); MediaEngagementService* service = MediaEngagementService::Get(profile());
ASSERT_TRUE(service); ASSERT_TRUE(service);
...@@ -31,7 +34,7 @@ class MediaEngagementContentsObserverTest ...@@ -31,7 +34,7 @@ class MediaEngagementContentsObserverTest
playback_timer_ = new base::MockTimer(true, false); playback_timer_ = new base::MockTimer(true, false);
contents_observer_->SetTimerForTest(base::WrapUnique(playback_timer_)); contents_observer_->SetTimerForTest(base::WrapUnique(playback_timer_));
ASSERT_FALSE(GetStoredPlayerStatesCount()); SimulateInaudible();
} }
bool IsTimerRunning() const { return playback_timer_->IsRunning(); } bool IsTimerRunning() const { return playback_timer_->IsRunning(); }
...@@ -123,6 +126,16 @@ class MediaEngagementContentsObserverTest ...@@ -123,6 +126,16 @@ class MediaEngagementContentsObserverTest
contents_observer_->committed_origin_ = url::Origin(url); contents_observer_->committed_origin_ = url::Origin(url);
} }
void SimulateAudible() {
content::WebContentsTester::For(web_contents())
->SetWasRecentlyAudible(true);
}
void SimulateInaudible() {
content::WebContentsTester::For(web_contents())
->SetWasRecentlyAudible(false);
}
private: private:
// contents_observer_ auto-destroys when WebContents is destroyed. // contents_observer_ auto-destroys when WebContents is destroyed.
MediaEngagementContentsObserver* contents_observer_; MediaEngagementContentsObserver* contents_observer_;
...@@ -175,8 +188,8 @@ TEST_F(MediaEngagementContentsObserverTest, AreConditionsMet) { ...@@ -175,8 +188,8 @@ TEST_F(MediaEngagementContentsObserverTest, AreConditionsMet) {
web_contents()->SetAudioMuted(true); web_contents()->SetAudioMuted(true);
EXPECT_FALSE(AreConditionsMet()); EXPECT_FALSE(AreConditionsMet());
web_contents()->SetAudioMuted(false); web_contents()->SetAudioMuted(false);
SimulateIsHidden(); SimulateIsHidden();
EXPECT_FALSE(AreConditionsMet()); EXPECT_FALSE(AreConditionsMet());
...@@ -255,6 +268,7 @@ TEST_F(MediaEngagementContentsObserverTest, ...@@ -255,6 +268,7 @@ TEST_F(MediaEngagementContentsObserverTest,
SignificantPlaybackRecordedWhenTimerFires) { SignificantPlaybackRecordedWhenTimerFires) {
SimulatePlaybackStarted(0); SimulatePlaybackStarted(0);
SimulateIsVisible(); SimulateIsVisible();
SimulateAudible();
web_contents()->SetAudioMuted(false); web_contents()->SetAudioMuted(false);
SimulateResizeEvent(0, MediaEngagementContentsObserver::kSignificantSize); SimulateResizeEvent(0, MediaEngagementContentsObserver::kSignificantSize);
EXPECT_TRUE(IsTimerRunning()); EXPECT_TRUE(IsTimerRunning());
...@@ -271,10 +285,21 @@ TEST_F(MediaEngagementContentsObserverTest, InteractionsRecorded) { ...@@ -271,10 +285,21 @@ TEST_F(MediaEngagementContentsObserverTest, InteractionsRecorded) {
Navigate(url); Navigate(url);
ExpectScores(url, 0.0, 1, 0); ExpectScores(url, 0.0, 1, 0);
SimulateAudible();
SimulateSignificantPlaybackTime(); SimulateSignificantPlaybackTime();
ExpectScores(url, 0.0, 1, 1); ExpectScores(url, 0.0, 1, 1);
} }
TEST_F(MediaEngagementContentsObserverTest,
SignificantPlaybackNotRecordedIfAudioSilent) {
SimulatePlaybackStarted(0);
SimulateIsVisible();
SimulateInaudible();
web_contents()->SetAudioMuted(false);
EXPECT_FALSE(IsTimerRunning());
EXPECT_FALSE(WasSignificantPlaybackRecorded());
}
TEST_F(MediaEngagementContentsObserverTest, DoNotRecordAudiolessTrack) { TEST_F(MediaEngagementContentsObserverTest, DoNotRecordAudiolessTrack) {
EXPECT_EQ(0u, GetSignificantActivePlayersCount()); EXPECT_EQ(0u, GetSignificantActivePlayersCount());
......
...@@ -154,6 +154,9 @@ class WebContentsTester { ...@@ -154,6 +154,9 @@ class WebContentsTester {
// Sets the return value of GetLastCommittedUrl() of TestWebContents. // Sets the return value of GetLastCommittedUrl() of TestWebContents.
virtual void SetLastCommittedURL(const GURL& url) = 0; virtual void SetLastCommittedURL(const GURL& url) = 0;
// Override WasRecentlyAudible for testing.
virtual void SetWasRecentlyAudible(bool audible) = 0;
}; };
} // namespace content } // namespace content
......
...@@ -429,4 +429,8 @@ void TestWebContents::SaveFrameWithHeaders(const GURL& url, ...@@ -429,4 +429,8 @@ void TestWebContents::SaveFrameWithHeaders(const GURL& url,
save_frame_headers_ = headers; save_frame_headers_ = headers;
} }
void TestWebContents::SetWasRecentlyAudible(bool audible) {
audio_stream_monitor()->set_was_recently_audible_for_testing(audible);
}
} // namespace content } // namespace content
...@@ -92,6 +92,7 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester { ...@@ -92,6 +92,7 @@ class TestWebContents : public WebContentsImpl, public WebContentsTester {
const std::vector<SkBitmap>& bitmaps, const std::vector<SkBitmap>& bitmaps,
const std::vector<gfx::Size>& original_bitmap_sizes) override; const std::vector<gfx::Size>& original_bitmap_sizes) override;
void SetLastCommittedURL(const GURL& url) override; void SetLastCommittedURL(const GURL& url) override;
void SetWasRecentlyAudible(bool audible) override;
// True if a cross-site navigation is pending. // True if a cross-site navigation is pending.
bool CrossProcessNavigationPending(); bool CrossProcessNavigationPending();
......
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