Commit 5f68fe87 authored by John Delaney's avatar John Delaney Committed by Commit Bot

Fix flaky AdsPLMO.AdFrameSizeInterventionMediaStatusPlayed

This test is flaking due to not waiting for resources to finish
loading. Currently "AddMinimumAdResourceExpectation" only checks
for resources reported as ads to be identified, rather than waiting
for the resources to complete. This modifies the method to
wait for complete ad resources.

This change modifies the EvalJS call that plays the
video to not have a user gesture so it is clear that the frame
doesn't receive user activation. This is purely to provide clarity in
the code and does not change the testing path.

Flake analysis: https://analysis.chromium.org/p/chromium/flake-portal/analysis/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNGMxNmRkZDA1NmY4ZTE3NGMzOGQ1Zjg2Y2FiMGJkYjBjNzM5MzRmMww

Change-Id: Ide73b6696610a134c4d0c5990d3a92685f1fc5b9
Reviewed-on: https://chromium-review.googlesource.com/c/1495674
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636619}
parent 9deda679
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/download_test_observer.h" #include "content/public/test/download_test_observer.h"
#include "content/public/test/test_navigation_observer.h" #include "content/public/test/test_navigation_observer.h"
#include "media/base/media_switches.h"
#include "net/dns/mock_host_resolver.h" #include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/controllable_http_response.h" #include "net/test/embedded_test_server/controllable_http_response.h"
#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/embedded_test_server.h"
...@@ -502,7 +503,7 @@ class AdsPageLoadMetricsTestWaiter ...@@ -502,7 +503,7 @@ class AdsPageLoadMetricsTestWaiter
bool ExpectationsSatisfied() const override { bool ExpectationsSatisfied() const override {
int num_ad_resources = 0; int num_ad_resources = 0;
for (auto& kv : page_resources_) { for (auto& kv : page_resources_) {
if (kv.second->reported_as_ad_resource) if (kv.second->reported_as_ad_resource && kv.second->is_complete)
num_ad_resources++; num_ad_resources++;
} }
return num_ad_resources >= expected_minimum_num_ad_resources_ && return num_ad_resources >= expected_minimum_num_ad_resources_ &&
...@@ -529,6 +530,12 @@ class AdsPageLoadMetricsObserverResourceBrowserTest ...@@ -529,6 +530,12 @@ class AdsPageLoadMetricsObserverResourceBrowserTest
subresource_filter::testing::CreateSuffixRule("disallow.zip")}); subresource_filter::testing::CreateSuffixRule("disallow.zip")});
} }
void SetUpCommandLine(base::CommandLine* command_line) override {
command_line->AppendSwitchASCII(
switches::kAutoplayPolicy,
switches::autoplay::kNoUserGestureRequiredPolicy);
}
void OpenLinkInFrame(const content::ToRenderFrameHost& adapter, void OpenLinkInFrame(const content::ToRenderFrameHost& adapter,
const std::string& link_id, const std::string& link_id,
bool has_gesture) { bool has_gesture) {
...@@ -863,9 +870,9 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest, ...@@ -863,9 +870,9 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
GURL url = embedded_test_server()->GetURL("foo.com", "/frame_factory.html"); GURL url = embedded_test_server()->GetURL("foo.com", "/frame_factory.html");
ui_test_utils::NavigateToURL(browser(), url); ui_test_utils::NavigateToURL(browser(), url);
// This page will load an autoplay video. // This frame will load a video.
contents->GetMainFrame()->ExecuteJavaScriptForTests( EXPECT_TRUE(
base::ASCIIToUTF16("createAdFrame('multiple_mimes.html', 'test');")); ExecJs(contents, "createAdFrame('multiple_mimes.html', 'test');"));
// Intercept one of the resources loaded by "multiple_mimes.html" and load // Intercept one of the resources loaded by "multiple_mimes.html" and load
// enough bytes to trigger the intervention. // enough bytes to trigger the intervention.
...@@ -878,7 +885,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest, ...@@ -878,7 +885,7 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
waiter->AddMinimumAdResourceExpectation(8); waiter->AddMinimumAdResourceExpectation(8);
waiter->Wait(); waiter->Wait();
// Wait for the video to play. // Wait for the video to autoplay in the frame.
content::RenderFrameHost* ad_frame = content::RenderFrameHost* ad_frame =
ChildFrameAt(contents->GetMainFrame(), 0); ChildFrameAt(contents->GetMainFrame(), 0);
EXPECT_EQ("true", content::EvalJsWithManualReply( EXPECT_EQ("true", content::EvalJsWithManualReply(
...@@ -886,7 +893,8 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest, ...@@ -886,7 +893,8 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverResourceBrowserTest,
"var video = document.getElementsByTagName('video')[0];" "var video = document.getElementsByTagName('video')[0];"
"video.onplaying = () => { " "video.onplaying = () => { "
"window.domAutomationController.send('true'); };" "window.domAutomationController.send('true'); };"
"video.play();")); "video.play();",
content::EXECUTE_SCRIPT_NO_USER_GESTURE));
// Close all tabs to report metrics. // Close all tabs to report metrics.
browser()->tab_strip_model()->CloseAllTabs(); browser()->tab_strip_model()->CloseAllTabs();
......
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