New issue
Advanced search Search tips

Issue 882280 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Layout Test http/tests/intersection-observer/v2/cross-origin-effects.html is flaky

Project Member Reported by yhirano@chromium.org, Sep 10

Issue description

The following layout test is flaky.

http/tests/intersection-observer/v2/cross-origin-effects.html
http/tests/intersection-observer/v2/cross-origin-occlusion.html 


 
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b4c707fd3ce173675649149fb7db13d539797bf

commit 7b4c707fd3ce173675649149fb7db13d539797bf
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Mon Sep 10 01:31:23 2018

Revert "[IntersectionObserver] Fix the concept of "tracking document""

This reverts commit 488a36e8d850ba3f2d05165b594356327d6deb4e.

Reason for revert: Speculative revert for 
 - http/tests/intersection-observer/v2/cross-origin-effects.html
 - http/tests/intersection-observer/v2/cross-origin-occlusion.html  

Original change's description:
> [IntersectionObserver] Fix the concept of "tracking document"
> 
> As explained in the bug, "tracking document" should be associated with
> IntersectionObservation, not IntersectionObserver.
> 
> One side effect of this change is that for a given observer, each of its
> observations runs its algorithm independently, so that notifications may
> not be generated in the order in which the observations were created
> (i.e., the order of calls to observer.observe()). To preserve the
> idiomatic web platform behavior that notifications are delivered in
> the order in which the targets were observed, this patch moves the
> storage of pending notifications from IntersectionObserver to
> IntersectionObservation.
> 
> BUG=879798
> R=​chrishtr@chromium.org
> 
> Change-Id: I9b6f6ad8a26387f0c072ccdff7f18cea9a88004c
> Reviewed-on: https://chromium-review.googlesource.com/1200388
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Commit-Queue: Stefan Zager <szager@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589678}

TBR=szager@chromium.org,chrishtr@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 879798,  882280 
Change-Id: I885276c561c8d6f21afdc059d080d58fac3b899e
Reviewed-on: https://chromium-review.googlesource.com/1215423
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589817}
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/1357ec4126fbc69acfc54cb520bbe096c1de82b6/third_party/WebKit/LayoutTests/external/wpt/intersection-observer/target-in-different-window.html
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/WebKit/LayoutTests/external/wpt/lint.whitelist
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/WebKit/LayoutTests/http/tests/intersection-observer/resources/v2-subframe.html
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/dom/element.h
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/execution_context/execution_context.h
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/element_intersection_observer_data.cc
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/element_intersection_observer_data.h
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observation.cc
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observation.h
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observer.cc
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observer.h
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observer_controller.cc
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observer_controller.h
[modify] https://crrev.com/7b4c707fd3ce173675649149fb7db13d539797bf/third_party/blink/renderer/core/intersection_observer/intersection_observer_test.cc

Owner: szager@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be431e79c6ecdc6a097e90f9defa077a0af7e2af

commit be431e79c6ecdc6a097e90f9defa077a0af7e2af
Author: Stefan Zager <szager@chromium.org>
Date: Fri Sep 14 21:39:33 2018

[IntersectionObserverV2] Move all tests to virtual test suite

Previously, IOv2 layout tests would enable IOv2 in script via:

if (window.internals && internals.runtimeFlags) {
  internals.runtimeFlags.intersectionObserverV2Enabled true;
}

There are two problems with this approach:

1. By the time the above code runs, it may be too late: the prototype
for the IntersectionObserverEntry javascript type may already have been
created *without* the isVisible attribute. This was causing flaky test
failures, because the exact timing of when the prototype is created may
vary between runs.

2. Because the layout test runner re-uses content_shell processes for
multiple tests, the side effects from turning on the feature flag
affect tests that later use the same renderer process (see bug).

This CL also adds DisableIntersectionObserverV2ThrottleForTesting,
which eliminates the need to add a bunch of 100ms timers to the tests.
Slow tests are bad.

Finally, this CL reorders the handshake for cross-origin tests that
rely on postMessage, to ensure that both the embedding frame and the
child frame are properly initialized before starting the test
procedure.

BUG= 882280 , 883676 
R=chrishtr@chromium.org,foolip@chromium.org

Change-Id: I18e62c2cf099bf74ef4afc43f46a794c7a322582
Reviewed-on: https://chromium-review.googlesource.com/1220044
Commit-Queue: Stefan Zager <szager@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591483}
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/VirtualTestSuites
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/http/tests/intersection-observer/resources/v2-subframe.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/http/tests/intersection-observer/v2/cross-origin-effects.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/http/tests/intersection-observer/v2/cross-origin-occlusion.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/resources/intersection-observer-test-utils.js
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/v2/animated-occlusion.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/v2/box-shadow.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/v2/iframe-target.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/v2/simple-effects.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/v2/simple-occlusion.html
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/intersection-observer/v2/text-shadow.html
[rename] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/virtual/intersection-observer-v2/http/tests/intersection-observer/v2/README.txt
[copy] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/WebKit/LayoutTests/virtual/intersection-observer-v2/intersection-observer/v2/README.txt
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/blink/renderer/core/testing/internals.cc
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/blink/renderer/core/testing/internals.h
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/blink/renderer/core/testing/internals.idl
[modify] https://crrev.com/be431e79c6ecdc6a097e90f9defa077a0af7e2af/third_party/blink/renderer/platform/runtime_enabled_features.json5

Status: Fixed (was: Assigned)

Sign in to add a comment