Commit 20ae05a6 authored by Fergal Daly's avatar Fergal Daly Committed by Commit Bot

Fix AdsPageLoadMetricsObserverTest + RenderDocument for subframes.

See https://chromium.googlesource.com/chromium/src/+/HEAD/docs/render_document.md

When RenderDocument for subframes is enabled, 2 tests fail:
- AdsPageLoadMetricsObserverTest.HeavyAdCpuInterventionInBackground
- AdsPageLoadMetricsObserverTest.HeavyAdNetworkInterventionInBackgrounded

Failures can be seen in the CQ of this change which enables RD for
subframes:
- https://crrev.com/c/2241368/6

They fail because the timing changes and SendInterventionReport ends
up being processed before FlushForTesting is called. At that point
there is no quit closure and so the message is dropped.

This is fixed by storing the message even if there is no quit
closure. This then requires a fix for HeavyAdPolicyProvided because it
reuses the same FrameRemoteTester for many tests and some messages from
one iteration linger for the next. This flushes the pipe at the start
of each iteration.

Also added a SCOPED_TRACE to this loop so you know which test failed when
one fails.

Bug: 1064944
Change-Id: I67b38f48e562ba4ecd4fad25afdc58cfb18f2b8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500975
Auto-Submit: Fergal Daly <fergal@chromium.org>
Reviewed-by: default avatarEric Robinson <ericrobinson@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821701}
parent 0b2a2168
......@@ -417,10 +417,11 @@ class FrameRemoteTester : public content::FakeLocalFrame {
// blink::mojom::LocalFrame
void SendInterventionReport(const std::string& id,
const std::string& message) override {
if (!on_empty_report_callback_)
return;
if (id.empty()) {
if (!on_empty_report_callback_)
return;
std::move(on_empty_report_callback_).Run();
return;
}
......@@ -2533,11 +2534,18 @@ TEST_F(AdsPageLoadMetricsObserverTest, HeavyAdPolicyProvided) {
};
for (const auto& test_case : kTestCases) {
SCOPED_TRACE(base::StringPrintf(
"policy: %s, exceed_network: %d, exceed_cpu: %d, "
"intervention_expected: %d",
test_case.policy.c_str(), test_case.exceed_network,
test_case.exceed_cpu, test_case.intervention_expected));
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeatureWithParameters(
features::kHeavyAdIntervention, {{"kUnloadPolicy", test_case.policy}});
RenderFrameHost* main_frame = NavigateMainFrame(kNonAdUrl);
RenderFrameHost* ad_frame = CreateAndNavigateSubFrame(kAdUrl, main_frame);
// Clear out any pending messages.
HasInterventionReportsAfterFlush(ad_frame);
ErrorPageWaiter waiter(web_contents());
if (test_case.exceed_network) {
......
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