MPArch: Ensure that Prerendering doesn't flush UKM metrics
All uses of IsInMainFrame have converted to IsInPrimaryMainFrame
for MPArch (Refer to crbug.com/1218946). As one of ensuring that
the conversion is correct, this CL adds a test to ensure
IsInPrimaryMainFrame use of LiteVideoObserver::DidFinishNavigation
works correctly on prerendering via LiteVideo's entry recorded number.
Bug: 1231382, 1218946
Change-Id: Ia5669b8bc23c2b7eeb19c36579c957e170222c8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3043066
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#904546}
diff --git a/chrome/browser/lite_video/lite_video_keyed_service_browsertest.cc b/chrome/browser/lite_video/lite_video_keyed_service_browsertest.cc
index cd448c12..b185cae5 100644
--- a/chrome/browser/lite_video/lite_video_keyed_service_browsertest.cc
+++ b/chrome/browser/lite_video/lite_video_keyed_service_browsertest.cc
@@ -34,6 +34,8 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/network_connection_change_simulator.h"
+#include "content/public/test/prerender_test_util.h"
+#include "net/dns/mock_host_resolver.h"
#include "net/nqe/effective_connection_type.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_source.h"
@@ -863,3 +865,71 @@
histogram_tester()->ExpectTotalCount(
"LiteVideo.CanApplyLiteVideo.UserBlocklist.SubFrame", 0);
}
+
+class LiteVideoKeyedServicePrerenderBrowserTest
+ : public LiteVideoKeyedServiceBrowserTest {
+ public:
+ LiteVideoKeyedServicePrerenderBrowserTest() = default;
+ ~LiteVideoKeyedServicePrerenderBrowserTest() override = default;
+ LiteVideoKeyedServicePrerenderBrowserTest(
+ const LiteVideoKeyedServicePrerenderBrowserTest&) = delete;
+
+ LiteVideoKeyedServicePrerenderBrowserTest& operator=(
+ const LiteVideoKeyedServicePrerenderBrowserTest&) = delete;
+
+ void SetUpCommandLine(base::CommandLine* command_line) override {
+ prerender_helper_ = std::make_unique<content::test::PrerenderTestHelper>(
+ base::BindRepeating(
+ &LiteVideoKeyedServicePrerenderBrowserTest::GetWebContents,
+ base::Unretained(this)));
+ LiteVideoKeyedServiceBrowserTest::SetUpCommandLine(command_line);
+ }
+
+ void SetUpOnMainThread() override {
+ prerender_helper_->SetUpOnMainThread(embedded_test_server());
+ host_resolver()->AddRule("*", "127.0.0.1");
+ ASSERT_TRUE(embedded_test_server()->Start());
+ }
+
+ content::test::PrerenderTestHelper& prerender_test_helper() {
+ return *prerender_helper_;
+ }
+
+ content::WebContents* GetWebContents() {
+ return browser()->tab_strip_model()->GetActiveWebContents();
+ }
+
+ private:
+ std::unique_ptr<content::test::PrerenderTestHelper> prerender_helper_;
+};
+
+INSTANTIATE_TEST_SUITE_P(UsingOptGuide,
+ LiteVideoKeyedServicePrerenderBrowserTest,
+ ::testing::Bool(),
+ ::testing::PrintToStringParamName());
+
+IN_PROC_BROWSER_TEST_P(LiteVideoKeyedServicePrerenderBrowserTest,
+ PrerenderingDontFlushUKMMetrics) {
+ GURL initial_url = embedded_test_server()->GetURL("/empty.html");
+ GURL prerender_url = embedded_test_server()->GetURL("/title1.html");
+ ASSERT_NE(ui_test_utils::NavigateToURL(browser(), initial_url), nullptr);
+
+ ukm::TestAutoSetUkmRecorder ukm_recorder;
+ // Load a test page in the prerender.
+ const int host_id = prerender_test_helper().AddPrerender(prerender_url);
+ content::RenderFrameHost* prerendered_render_frame_host =
+ prerender_test_helper().GetPrerenderedMainFrameHost(host_id);
+ ASSERT_TRUE(prerendered_render_frame_host);
+ auto entries =
+ ukm_recorder.GetEntriesByName(ukm::builders::LiteVideo::kEntryName);
+ // The number of the entry should be 0 since FlushUKMMetrics is not called by
+ // the lite video observer in prerendering.
+ EXPECT_EQ(0u, entries.size());
+
+ // Activate the prerendered page.
+ prerender_test_helper().NavigatePrimaryPage(prerender_url);
+ entries = ukm_recorder.GetEntriesByName(ukm::builders::LiteVideo::kEntryName);
+ // The number of the entry should be 1 since FlushUKMMetrics is called by
+ // the lite video observer after activating.
+ EXPECT_EQ(1u, entries.size());
+}
diff --git a/chrome/browser/lite_video/lite_video_observer.cc b/chrome/browser/lite_video/lite_video_observer.cc
index 0f95514..8e71770 100644
--- a/chrome/browser/lite_video/lite_video_observer.cc
+++ b/chrome/browser/lite_video/lite_video_observer.cc
@@ -92,9 +92,6 @@
lite_video::LiteVideoBlocklistReason blocklist_reason =
lite_video::LiteVideoBlocklistReason::kUnknown;
- // TODO(https://crbug.com/1218946): With MPArch there may be multiple main
- // frames. This caller was converted automatically to the primary main frame
- // to preserve its semantics. Follow up to confirm correctness.
if (navigation_handle->IsInPrimaryMainFrame()) {
FlushUKMMetrics();
routing_ids_to_notify_.clear();
