Skip to content

Commit

Permalink
M106 merge: Fix the website metrics issue when navigate to inactivate…
Browse files Browse the repository at this point in the history 
…d tab.

This CL is the follow up CL for CL:3836214. Adding tests to verify the
website metrics.

Two issues are fixed in this CL:

1. When updating the url info, not only checking the browser window's
activated statue, but also checks the tab's activated status, because
when tabs are inactivated, it could be updated as well.

2. When saving the url info to the pref, check
running_time_in_two_hours, because even if the
running_time_in_five_minutes for the tab is zero, we should still save
the url in the pref, as the UKM is recorded each 2 hours.

BUG=1355259

(cherry picked from commit 830b2e3)

Change-Id: Ia77f9f8b1ece081540df002c22b305f531918cae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3841308
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1038036}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3855496
Commit-Queue: Tim Sergeant <tsergeant@chromium.org>
Reviewed-by: Tim Sergeant <tsergeant@chromium.org>
Auto-Submit: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/branch-heads/5249@{#94}
Cr-Branched-From: 4f7bea5-refs/heads/main@{#1036826}
  • Loading branch information
Nancy Wang authored and Chromium LUCI CQ committed Aug 25, 2022
1 parent 160851e commit f5217c6
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 4 deletions.
17 changes: 14 additions & 3 deletions chrome/browser/apps/app_service/metrics/website_metrics.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,12 @@ void WebsiteMetrics::OnWebContentsUpdated(content::WebContents* web_contents) {
// contents::WebContentsObserver::PrimaryPageChanged(), set the visible url as
// default value for the ukm key url.
webcontents_to_ukm_key_[web_contents] = web_contents->GetVisibleURL();
auto it = window_to_web_contents_.find(window);
bool is_activated = wm::IsActiveWindow(window) &&
it != window_to_web_contents_.end() &&
it->second == web_contents;
AddUrlInfo(web_contents->GetVisibleURL(), base::TimeTicks::Now(),
UrlContent::kFullUrl, wm::IsActiveWindow(window),
UrlContent::kFullUrl, is_activated,
/*promotable=*/false);
}

Expand Down Expand Up @@ -409,8 +413,12 @@ void WebsiteMetrics::OnInstallableWebAppStatusUpdated(
}

DCHECK(!app_banner_manager->manifest().scope.is_empty());
auto window_it = window_to_web_contents_.find(window);
bool is_activated = wm::IsActiveWindow(window) &&
window_it != window_to_web_contents_.end() &&
window_it->second == web_contents;
UpdateUrlInfo(it->second, app_banner_manager->manifest().scope,
UrlContent::kScope, wm::IsActiveWindow(window),
UrlContent::kScope, is_activated,
/*promotable=*/true);
it->second = app_banner_manager->manifest().scope;
}
Expand Down Expand Up @@ -507,9 +515,12 @@ void WebsiteMetrics::SaveUsageTime() {
// the raw data collected in a 5 minutes slot.
it.second.running_time_in_two_hours +=
GetRandomNoise() * it.second.running_time_in_five_minutes;
dict.Set(it.first.spec(), it.second.ConvertToValue());
it.second.running_time_in_five_minutes = base::TimeDelta();
}
// Save all urls running time in the past two hours to the user pref.
if (!it.second.running_time_in_two_hours.is_zero()) {
dict.Set(it.first.spec(), it.second.ConvertToValue());
}
}
}

Expand Down
144 changes: 143 additions & 1 deletion chrome/browser/apps/app_service/metrics/website_metrics_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ class WebsiteMetricsBrowserTest : public InProcessBrowserTest {
WindowOpenDisposition::NEW_FOREGROUND_TAB);
}

content::WebContents* InsertBackgroundTab(Browser* browser,
const std::string& url) {
return NavigateAndWait(browser, url,
WindowOpenDisposition::NEW_BACKGROUND_TAB);
}

web_app::AppId InstallWebApp(const std::string& start_url,
web_app::UserDisplayMode user_display_mode) {
auto info = std::make_unique<WebAppInstallInfo>();
Expand Down Expand Up @@ -401,6 +407,7 @@ IN_PROC_BROWSER_TEST_F(WebsiteMetricsBrowserTest, ForegroundTabNavigate) {
VerifyUrlInfo(GURL("https://b.example.org"), UrlContent::kFullUrl,
/*is_activated=*/true, /*promotable=*/false);

website_metrics()->OnFiveMinutes();
browser->tab_strip_model()->CloseAllTabs();
EXPECT_TRUE(webcontents_to_observer_map().empty());
EXPECT_TRUE(window_to_web_contents().empty());
Expand All @@ -409,7 +416,6 @@ IN_PROC_BROWSER_TEST_F(WebsiteMetricsBrowserTest, ForegroundTabNavigate) {
/*is_activated=*/false, /*promotable=*/false);
VerifyUrlInfo(GURL("https://b.example.org"), UrlContent::kFullUrl,
/*is_activated=*/false, /*promotable=*/false);
website_metrics()->OnFiveMinutes();
VerifyUrlInfoInPref(GURL("https://a.example.org"), UrlContent::kFullUrl,
/*promotable=*/false);
VerifyUrlInfoInPref(GURL("https://b.example.org"), UrlContent::kFullUrl,
Expand All @@ -424,6 +430,142 @@ IN_PROC_BROWSER_TEST_F(WebsiteMetricsBrowserTest, ForegroundTabNavigate) {
EXPECT_TRUE(url_infos().empty());
}

IN_PROC_BROWSER_TEST_F(WebsiteMetricsBrowserTest, NavigateToBackgroundTab) {
auto website_metrics_ptr = std::make_unique<apps::TestWebsiteMetrics>(
ProfileManager::GetPrimaryUserProfile());
auto* metrics = website_metrics_ptr.get();
app_platform_metrics_service_->SetWebsiteMetricsForTesting(
std::move(website_metrics_ptr));

Browser* browser = CreateBrowser();
auto* window = browser->window()->GetNativeWindow();
EXPECT_EQ(1u, window_to_web_contents().size());
// Open a tab in foreground.
GURL url1 =
embedded_test_server()->GetURL("/banners/no_manifest_test_page.html");
auto* tab1 = InsertForegroundTab(browser, url1.spec());
EXPECT_EQ(1u, webcontents_to_observer_map().size());
EXPECT_TRUE(base::Contains(webcontents_to_observer_map(),
window_to_web_contents()[window]));
EXPECT_EQ(window_to_web_contents()[window]->GetVisibleURL(), url1);
EXPECT_EQ(1u, webcontents_to_ukm_key().size());
EXPECT_EQ(webcontents_to_ukm_key()[tab1], url1);
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/true, /*promotable=*/false);

// Navigate the background tab to a url with a manifest.
GURL url2 =
embedded_test_server()->GetURL("/banners/manifest_test_page.html");
auto ukm_key = url2.GetWithoutFilename();
auto* tab2 = InsertBackgroundTab(browser, url2.spec());
metrics->AwaitForInstallableWebAppCheck(ukm_key);
EXPECT_EQ(2u, webcontents_to_observer_map().size());
EXPECT_TRUE(base::Contains(webcontents_to_observer_map(), tab2));
EXPECT_EQ(1u, window_to_web_contents().size());
EXPECT_EQ(window_to_web_contents()[window]->GetVisibleURL(), url1);
EXPECT_EQ(2u, webcontents_to_ukm_key().size());
EXPECT_EQ(webcontents_to_ukm_key()[tab2], ukm_key);
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/true, /*promotable=*/false);
VerifyUrlInfo(ukm_key, UrlContent::kScope,
/*is_activated=*/false, /*promotable=*/true);

website_metrics()->OnFiveMinutes();
browser->tab_strip_model()->CloseAllTabs();
EXPECT_TRUE(webcontents_to_observer_map().empty());
EXPECT_TRUE(window_to_web_contents().empty());
EXPECT_TRUE(webcontents_to_ukm_key().empty());
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/false, /*promotable=*/false);
VerifyUrlInfo(ukm_key, UrlContent::kScope,
/*is_activated=*/false, /*promotable=*/true);
VerifyUrlInfoInPref(url1, UrlContent::kFullUrl,
/*promotable=*/false);
VerifyNoUrlInfoInPref(ukm_key);

// Simulate recording the UKMs to clear the local usage time records.
website_metrics()->OnTwoHours();
VerifyUsageTimeUkm(url1, UrlContent::kFullUrl,
/*promotable=*/false);
VerifyNoUsageTimeUkm(ukm_key);
EXPECT_TRUE(url_infos().empty());
}

IN_PROC_BROWSER_TEST_F(WebsiteMetricsBrowserTest, ActiveBackgroundTab) {
auto website_metrics_ptr = std::make_unique<apps::TestWebsiteMetrics>(
ProfileManager::GetPrimaryUserProfile());
auto* metrics = website_metrics_ptr.get();
app_platform_metrics_service_->SetWebsiteMetricsForTesting(
std::move(website_metrics_ptr));

Browser* browser = CreateBrowser();
auto* window = browser->window()->GetNativeWindow();
EXPECT_EQ(1u, window_to_web_contents().size());
// Open a tab in foreground.
GURL url1 =
embedded_test_server()->GetURL("/banners/no_manifest_test_page.html");
auto* tab1 = InsertForegroundTab(browser, url1.spec());
EXPECT_EQ(1u, webcontents_to_observer_map().size());
EXPECT_TRUE(base::Contains(webcontents_to_observer_map(),
window_to_web_contents()[window]));
EXPECT_EQ(window_to_web_contents()[window]->GetVisibleURL(), url1);
EXPECT_EQ(1u, webcontents_to_ukm_key().size());
EXPECT_EQ(webcontents_to_ukm_key()[tab1], url1);
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/true, /*promotable=*/false);

// Navigate the background tab to a url with a manifest.
GURL url2 =
embedded_test_server()->GetURL("/banners/manifest_test_page.html");
auto ukm_key = url2.GetWithoutFilename();
auto* tab2 = InsertBackgroundTab(browser, url2.spec());
metrics->AwaitForInstallableWebAppCheck(ukm_key);
EXPECT_EQ(2u, webcontents_to_observer_map().size());
EXPECT_TRUE(base::Contains(webcontents_to_observer_map(), tab2));
EXPECT_EQ(1u, window_to_web_contents().size());
EXPECT_EQ(window_to_web_contents()[window]->GetVisibleURL(), url1);
EXPECT_EQ(2u, webcontents_to_ukm_key().size());
EXPECT_EQ(webcontents_to_ukm_key()[tab2], ukm_key);
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/true, /*promotable=*/false);
VerifyUrlInfo(ukm_key, UrlContent::kScope,
/*is_activated=*/false, /*promotable=*/true);
website_metrics()->OnFiveMinutes();

browser->tab_strip_model()->ActivateTabAt(1);
EXPECT_EQ(2u, webcontents_to_observer_map().size());
EXPECT_EQ(1u, window_to_web_contents().size());
EXPECT_EQ(window_to_web_contents()[window]->GetVisibleURL(), url2);
EXPECT_EQ(2u, webcontents_to_ukm_key().size());
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/false, /*promotable=*/false);
VerifyUrlInfo(ukm_key, UrlContent::kScope,
/*is_activated=*/true, /*promotable=*/true);
website_metrics()->OnFiveMinutes();

browser->tab_strip_model()->CloseAllTabs();
EXPECT_TRUE(webcontents_to_observer_map().empty());
EXPECT_TRUE(window_to_web_contents().empty());
EXPECT_TRUE(webcontents_to_ukm_key().empty());
VerifyUrlInfo(url1, UrlContent::kFullUrl,
/*is_activated=*/false, /*promotable=*/false);
VerifyUrlInfo(ukm_key, UrlContent::kScope,
/*is_activated=*/false, /*promotable=*/true);
website_metrics()->OnFiveMinutes();
VerifyUrlInfoInPref(url1, UrlContent::kFullUrl,
/*promotable=*/false);
VerifyUrlInfoInPref(ukm_key, UrlContent::kScope,
/*promotable=*/true);

// Simulate recording the UKMs to clear the local usage time records.
website_metrics()->OnTwoHours();
VerifyUsageTimeUkm(url1, UrlContent::kFullUrl,
/*promotable=*/false);
VerifyUsageTimeUkm(ukm_key, UrlContent::kScope,
/*promotable=*/true);
EXPECT_TRUE(url_infos().empty());
}

IN_PROC_BROWSER_TEST_F(WebsiteMetricsBrowserTest, NavigateToUrlWithManifest) {
auto website_metrics_ptr = std::make_unique<apps::TestWebsiteMetrics>(
ProfileManager::GetPrimaryUserProfile());
Expand Down

0 comments on commit f5217c6

Please sign in to comment.