Commit 8fd675e0 authored by Hajime Hoshi's avatar Hajime Hoshi Committed by Commit Bot

BackForwardCache: Reset recorded metrics at navigations correctly

Before this CL, |BackForwardCacheMetrics::
browsing_instance_not_swapped_reason_| is not reset at navigation and
might be recorded in non-related navigations unexpectedly.

This CL fixes this by resetting it every navigation.

Bug: 1134847
Change-Id: Ib9e00f6937c5425c82cd2e40bdb47cb1fb740d38
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2501546Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822154}
parent 684e3bae
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#include "content/browser/renderer_host/frame_tree_node.h" #include "content/browser/renderer_host/frame_tree_node.h"
#include "content/browser/renderer_host/page_lifecycle_state_manager.h" #include "content/browser/renderer_host/page_lifecycle_state_manager.h"
#include "content/browser/renderer_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_frame_host_impl.h"
#include "content/browser/renderer_host/should_swap_browsing_instance.h"
#include "content/browser/web_contents/file_chooser_impl.h" #include "content/browser/web_contents/file_chooser_impl.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/content_navigation_policy.h" #include "content/common/content_navigation_policy.h"
...@@ -376,6 +377,30 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest, ...@@ -376,6 +377,30 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest,
<< location.ToString(); << location.ToString();
} }
void ExpectBrowsingInstanceNotSwappedReason(ShouldSwapBrowsingInstance reason,
base::Location location) {
base::HistogramBase::Sample sample = base::HistogramBase::Sample(reason);
AddSampleToBuckets(&expected_browsing_instance_not_swapped_reasons_,
sample);
EXPECT_THAT(histogram_tester_.GetAllSamples(
"BackForwardCache.HistoryNavigationOutcome."
"BrowsingInstanceNotSwappedReason"),
UnorderedElementsAreArray(
expected_browsing_instance_not_swapped_reasons_))
<< location.ToString();
if (!check_all_sites_)
return;
EXPECT_THAT(histogram_tester_.GetAllSamples(
"BackForwardCache.AllSites.HistoryNavigationOutcome."
"BrowsingInstanceNotSwappedReason"),
UnorderedElementsAreArray(
expected_browsing_instance_not_swapped_reasons_))
<< location.ToString();
}
void ExpectEvictedAfterCommitted( void ExpectEvictedAfterCommitted(
std::vector<BackForwardCacheMetrics::EvictedAfterDocumentRestoredReason> std::vector<BackForwardCacheMetrics::EvictedAfterDocumentRestoredReason>
reasons, reasons,
...@@ -503,6 +528,7 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest, ...@@ -503,6 +528,7 @@ class BackForwardCacheBrowserTest : public ContentBrowserTest,
std::vector<base::Bucket> expected_not_restored_; std::vector<base::Bucket> expected_not_restored_;
std::vector<base::Bucket> expected_blocklisted_features_; std::vector<base::Bucket> expected_blocklisted_features_;
std::vector<base::Bucket> expected_disabled_reasons_; std::vector<base::Bucket> expected_disabled_reasons_;
std::vector<base::Bucket> expected_browsing_instance_not_swapped_reasons_;
std::vector<base::Bucket> expected_eviction_after_committing_; std::vector<base::Bucket> expected_eviction_after_committing_;
std::unique_ptr<net::EmbeddedTestServer> https_server_; std::unique_ptr<net::EmbeddedTestServer> https_server_;
std::unordered_map<base::Feature, std::unordered_map<base::Feature,
...@@ -2494,6 +2520,23 @@ IN_PROC_BROWSER_TEST_F( ...@@ -2494,6 +2520,23 @@ IN_PROC_BROWSER_TEST_F(
{blink::scheduler::WebSchedulerTrackedFeature::kWebRTC, {blink::scheduler::WebSchedulerTrackedFeature::kWebRTC,
blink::scheduler::WebSchedulerTrackedFeature::kKeyboardLock}, blink::scheduler::WebSchedulerTrackedFeature::kKeyboardLock},
FROM_HERE); FROM_HERE);
ExpectBrowsingInstanceNotSwappedReason(
ShouldSwapBrowsingInstance::kNo_NotNeededForBackForwardCache,
FROM_HERE);
web_contents()->GetController().GoForward();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
ExpectBrowsingInstanceNotSwappedReason(
ShouldSwapBrowsingInstance::kNo_AlreadyHasMatchingBrowsingInstance,
FROM_HERE);
web_contents()->GetController().GoBack();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
ExpectBrowsingInstanceNotSwappedReason(
ShouldSwapBrowsingInstance::kNo_AlreadyHasMatchingBrowsingInstance,
FROM_HERE);
} else { } else {
ExpectNotRestored({BackForwardCacheMetrics::NotRestoredReason:: ExpectNotRestored({BackForwardCacheMetrics::NotRestoredReason::
kRenderFrameHostReused_CrossSite}, kRenderFrameHostReused_CrossSite},
......
...@@ -123,6 +123,7 @@ void BackForwardCacheMetrics::DidCommitNavigation( ...@@ -123,6 +123,7 @@ void BackForwardCacheMetrics::DidCommitNavigation(
not_restored_reasons_.reset(); not_restored_reasons_.reset();
blocklisted_features_ = 0; blocklisted_features_ = 0;
browsing_instance_not_swapped_reason_.reset();
disabled_reasons_.clear(); disabled_reasons_.clear();
previous_navigation_is_served_from_bfcache_ = previous_navigation_is_served_from_bfcache_ =
navigation->IsServedFromBackForwardCache(); navigation->IsServedFromBackForwardCache();
......
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