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

Media Engagement: Fix session link on same origin

When opening a new tab for the same origin as a the current tab,
the new tab was incorrectly linked to the currently open tab's
MediaEngagementSession.

This adds a check to only allow the sessions to be linked if the
new tab navigation originated from a link on the current tab.

BUG=819807

Change-Id: I02f892372f1aa92f830f891a369732a4fc7b8883
Reviewed-on: https://chromium-review.googlesource.com/962242
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544026}
parent 43e42e20
...@@ -146,8 +146,8 @@ class MediaEngagementBrowserTest : public InProcessBrowserTest { ...@@ -146,8 +146,8 @@ class MediaEngagementBrowserTest : public InProcessBrowserTest {
WaitForWasRecentlyAudible(); WaitForWasRecentlyAudible();
} }
void OpenTab(const GURL& url) { void OpenTab(const GURL& url, ui::PageTransition transition) {
NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK); NavigateParams params(browser(), url, transition);
params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB; params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
// params.opener does not need to be set in the context of this test because // params.opener does not need to be set in the context of this test because
// it will use the current tab by default. // it will use the current tab by default.
...@@ -158,8 +158,12 @@ class MediaEngagementBrowserTest : public InProcessBrowserTest { ...@@ -158,8 +158,12 @@ class MediaEngagementBrowserTest : public InProcessBrowserTest {
content::WaitForLoadStop(params.target_contents); content::WaitForLoadStop(params.target_contents);
} }
void OpenTabAndwaitForPlayAndAudible(const GURL& url) { void OpenTabAsLink(const GURL& url) {
OpenTab(url); OpenTab(url, ui::PAGE_TRANSITION_LINK);
}
void OpenTabAndWaitForPlayAndAudible(const GURL& url) {
OpenTabAsLink(url);
WaitForPlay(); WaitForPlay();
WaitForWasRecentlyAudible(); WaitForWasRecentlyAudible();
...@@ -222,7 +226,7 @@ class MediaEngagementBrowserTest : public InProcessBrowserTest { ...@@ -222,7 +226,7 @@ class MediaEngagementBrowserTest : public InProcessBrowserTest {
EXPECT_TRUE(content::ExecuteScript(GetWebContents(), script)); EXPECT_TRUE(content::ExecuteScript(GetWebContents(), script));
} }
void OpenTab() { void OpenTabAsLink() {
ui_test_utils::NavigateToURLWithDisposition( ui_test_utils::NavigateToURLWithDisposition(
browser(), GURL("chrome://about"), browser(), GURL("chrome://about"),
WindowOpenDisposition::NEW_FOREGROUND_TAB, WindowOpenDisposition::NEW_FOREGROUND_TAB,
...@@ -414,7 +418,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, ...@@ -414,7 +418,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
RecordEngagement_NotVisible) { RecordEngagement_NotVisible) {
LoadTestPageAndWaitForPlayAndAudible("engagement_test.html", false); LoadTestPageAndWaitForPlayAndAudible("engagement_test.html", false);
OpenTab(); OpenTabAsLink();
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
CloseTab(); CloseTab();
ExpectScores(1, 1, 1, 1); ExpectScores(1, 1, 1, 1);
...@@ -423,7 +427,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, ...@@ -423,7 +427,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
RecordEngagement_NotVisible_AudioOnly) { RecordEngagement_NotVisible_AudioOnly) {
LoadTestPageAndWaitForPlayAndAudible("engagement_test_audio.html", false); LoadTestPageAndWaitForPlayAndAudible("engagement_test_audio.html", false);
OpenTab(); OpenTabAsLink();
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
CloseTab(); CloseTab();
ExpectScores(1, 1, 1, 1); ExpectScores(1, 1, 1, 1);
...@@ -557,7 +561,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, ...@@ -557,7 +561,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
LoadTestPageAndWaitForPlayAndAudible(url, false); LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
OpenTab(GURL("about:blank")); OpenTabAsLink(GURL("about:blank"));
LoadTestPageAndWaitForPlayAndAudible(url, false); LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
...@@ -572,7 +576,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, SessionNewTabSameURL) { ...@@ -572,7 +576,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, SessionNewTabSameURL) {
LoadTestPageAndWaitForPlayAndAudible(url, false); LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
OpenTabAndwaitForPlayAndAudible(url); OpenTabAndWaitForPlayAndAudible(url);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
browser()->tab_strip_model()->CloseAllTabs(); browser()->tab_strip_model()->CloseAllTabs();
...@@ -587,7 +591,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, SessionNewTabSameOrigin) { ...@@ -587,7 +591,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, SessionNewTabSameOrigin) {
LoadTestPageAndWaitForPlayAndAudible(url, false); LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
OpenTabAndwaitForPlayAndAudible(other_url); OpenTabAndWaitForPlayAndAudible(other_url);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
browser()->tab_strip_model()->CloseAllTabs(); browser()->tab_strip_model()->CloseAllTabs();
...@@ -602,7 +606,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, SessionNewTabCrossOrigin) { ...@@ -602,7 +606,7 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, SessionNewTabCrossOrigin) {
LoadTestPageAndWaitForPlayAndAudible(url, false); LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
OpenTabAndwaitForPlayAndAudible(other_url); OpenTabAndWaitForPlayAndAudible(other_url);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
browser()->tab_strip_model()->CloseAllTabs(); browser()->tab_strip_model()->CloseAllTabs();
...@@ -619,13 +623,13 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest, ...@@ -619,13 +623,13 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
LoadTestPageAndWaitForPlayAndAudible(url, false); LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
OpenTabAndwaitForPlayAndAudible(other_url); OpenTabAndWaitForPlayAndAudible(other_url);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
CloseTab(); CloseTab();
ASSERT_EQ(other_url, GetWebContents()->GetLastCommittedURL()); ASSERT_EQ(other_url, GetWebContents()->GetLastCommittedURL());
OpenTabAndwaitForPlayAndAudible(url); OpenTabAndWaitForPlayAndAudible(url);
AdvanceMeaningfulPlaybackTime(); AdvanceMeaningfulPlaybackTime();
browser()->tab_strip_model()->CloseAllTabs(); browser()->tab_strip_model()->CloseAllTabs();
...@@ -644,6 +648,25 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementPreloadBrowserTest, ...@@ -644,6 +648,25 @@ IN_PROC_BROWSER_TEST_F(MediaEngagementPreloadBrowserTest,
EXPECT_TRUE(MediaEngagementPreloadedList::GetInstance()->loaded()); EXPECT_TRUE(MediaEngagementPreloadedList::GetInstance()->loaded());
} }
IN_PROC_BROWSER_TEST_F(MediaEngagementBrowserTest,
SessionNewTabNavigateSameURLWithOpener_Typed) {
const GURL& url = http_server().GetURL("/engagement_test.html");
LoadTestPageAndWaitForPlayAndAudible(url, false);
AdvanceMeaningfulPlaybackTime();
OpenTab(url, ui::PAGE_TRANSITION_TYPED);
WaitForPlay();
WaitForWasRecentlyAudible();
AdvanceMeaningfulPlaybackTime();
browser()->tab_strip_model()->CloseAllTabs();
// The new tab should only count as the same visit if we visited that tab
// through a link or reload (duplicate tab).
ExpectScores(2, 2, 2, 2);
}
class MediaEngagementPrerenderBrowserTest : public MediaEngagementBrowserTest { class MediaEngagementPrerenderBrowserTest : public MediaEngagementBrowserTest {
public: public:
void SetUpOnMainThread() override { void SetUpOnMainThread() override {
......
...@@ -161,9 +161,18 @@ void MediaEngagementContentsObserver::DidFinishNavigation( ...@@ -161,9 +161,18 @@ void MediaEngagementContentsObserver::DidFinishNavigation(
if (session_ && session_->IsSameOriginWith(new_origin)) if (session_ && session_->IsSameOriginWith(new_origin))
return; return;
// Only get the opener if the navigation originated from a link.
content::WebContents* opener = nullptr;
if (ui::PageTransitionCoreTypeIs(navigation_handle->GetPageTransition(),
ui::PAGE_TRANSITION_LINK) ||
ui::PageTransitionCoreTypeIs(navigation_handle->GetPageTransition(),
ui::PAGE_TRANSITION_RELOAD)) {
opener = GetOpener();
}
bool was_restored = bool was_restored =
navigation_handle->GetRestoreType() != content::RestoreType::NONE; navigation_handle->GetRestoreType() != content::RestoreType::NONE;
session_ = GetOrCreateSession(new_origin, GetOpener(), was_restored); session_ = GetOrCreateSession(new_origin, opener, was_restored);
} }
MediaEngagementContentsObserver::PlayerState::PlayerState(base::Clock* clock) MediaEngagementContentsObserver::PlayerState::PlayerState(base::Clock* clock)
...@@ -529,6 +538,7 @@ content::WebContents* MediaEngagementContentsObserver::GetOpener() const { ...@@ -529,6 +538,7 @@ content::WebContents* MediaEngagementContentsObserver::GetOpener() const {
browser->tab_strip_model()->GetIndexOfWebContents(web_contents()); browser->tab_strip_model()->GetIndexOfWebContents(web_contents());
if (index == TabStripModel::kNoTab) if (index == TabStripModel::kNoTab)
continue; continue;
// Whether or not the |opener| is null, this is the right tab strip. // Whether or not the |opener| is null, this is the right tab strip.
return browser->tab_strip_model()->GetOpenerOfWebContentsAt(index); return browser->tab_strip_model()->GetOpenerOfWebContentsAt(index);
} }
......
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