Commit 0a482765 authored by John Delaney's avatar John Delaney Committed by Commit Bot

Fix AdsPageLoadMetricsObserverBrowserTest.FramePixelSize flakiness

This test is flaking on bots due to incorrectly recording the size
of adframes with a display: none style. The underlying problem is that
display:none frames do not send size updates to the browser process.
As such, in this test there is a race condition between the first size
update being sent, and the display property being set on the frame.
Because we do not really care about the size of display:none frames
as they are not visible to the user, I will remove this part
of the test and replace with a TODO until the underlying issue can be
solved.

This was previously not flaking because we did not record the size
of display:none iframes.

FintIt: https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vMjQ5NjAyODVhYzAyYmYzM2M0NjNhZjI5MmQ5ZTVhNzZkN2JlNWUzZAw

Bug: 924357
Change-Id: I12ff78c8aad14cd44767ce916e162e53636e4c6c
Reviewed-on: https://chromium-review.googlesource.com/c/1456496
Commit-Queue: John Delaney <johnidel@chromium.org>
Reviewed-by: default avatarJosh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629605}
parent 41e6cd78
......@@ -370,6 +370,9 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
"PageLoad.Clients.Ads.Visible.Bytes.AdFrames.PerFrame.Total", 0);
}
// TODO(https://crbug.com/929136): Investigate why setting display: none on the
// frame will cause size updates to not be received. Verify that we record the
// correct sizes for display: none iframes.
IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest, FramePixelSize) {
base::HistogramTester histogram_tester;
auto waiter = CreatePageLoadMetricsTestWaiter();
......@@ -397,20 +400,9 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest, FramePixelSize) {
"frame = createAdIframe(); frame.width=10; frame.height = 1000; "
"frame.src = '/ads_observer/pixel.png';"));
// Create a 300x300 iframe with display none that should also be recorded.
ASSERT_TRUE(ExecJs(web_contents,
"frame = createAdIframe(); frame.width=300; frame.height "
"= 300; frame.src "
"= '/ads_observer/pixel.png';"));
// Set the frame to display none.
// TODO(johnidel): Investigate why setting display: none on the frame will
// cause size updates to not be received.
ASSERT_TRUE(ExecJs(web_contents, "frame.style.display = 'none';"));
// Wait for each frames resource to load so that they will have non-zero
// bytes.
waiter->AddMinimumCompleteResourcesExpectation(7);
waiter->AddMinimumCompleteResourcesExpectation(6);
waiter->Wait();
// Navigate away to force the histogram recording.
......@@ -420,10 +412,6 @@ IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest, FramePixelSize) {
histogram_tester.ExpectBucketCount(kSmallestDimensionHistogramId, 0, 1);
histogram_tester.ExpectBucketCount(kSmallestDimensionHistogramId, 10, 1);
histogram_tester.ExpectBucketCount(kSmallestDimensionHistogramId, 100, 1);
// Verify the display: none frame is recorded.
histogram_tester.ExpectBucketCount(kSqrtNumberOfPixelsHistogramId, 300, 1);
histogram_tester.ExpectBucketCount(kSmallestDimensionHistogramId, 300, 1);
}
IN_PROC_BROWSER_TEST_F(AdsPageLoadMetricsObserverBrowserTest,
......
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