Perhaps LayoutView::ContentBoxRect() is correct, but we are using an incorrect rect for viewport intersection observation.

Issue 835780 has been merged into this issue.

The following steps can reproduce the bug in production:

1. Navigate to https://www.politico.com/privacy-policy;
2. Open Developer tools and enable mobile emulation, select Pixel XL, portrait;
3. Double click once or twice to change the viewport zoom level, until the main text is shown in about the left half of the viewport;
4. Scroll to close to the bottom, but not to let the ad scroll into the view;
5. Slowly scroll down

Expected: The ad should show once it enters the viewport
Actual The ad shows only after it scrolls to the middle of the viewport.

Thanks, I'll investigate today.

Taking a look, it seems the root_rect_ is correct to me, here's what I get when the ad is visible and we're fully zoomed out.

[1:1:0425/142249.124208:ERROR:intersection_geometry.cc(115)] TargetRect: "0,0 600x500"
[1:1:0425/142249.124563:ERROR:intersection_geometry.cc(141)] RootRect: "0,0 822x1646"
[1:1:0425/142249.124768:ERROR:intersection_geometry.cc(167)] ApplyRootMargin: "0,0 822x1646"
[1:1:0425/142249.125065:ERROR:intersection_geometry.cc(178)] MapToVisualRectInAncestorSpace: "0,0 600x500" => "111,25738 600x500" does_intersect=1
[1:1:0425/142249.125388:ERROR:intersection_geometry.cc(186)]    After scroll adjustment: "111,2091 600x500"
[1:1:0425/142249.125618:ERROR:intersection_geometry.cc(192)]    After clip by "0,0 822x1646" => 0
[1:1:0425/142249.125957:ERROR:intersection_observation.cc(79)]   !!!! NO INTERSECT

The RootRect looks correct to me - the viewport size is 411x823 / 0.55 (visualViewport.scale) == 742x1486 so that's close and I assume there's some margin applied. When we switch to "mobile mode" the root LayoutView will be resized after layout to the size of the viewport at minimum scale. Perhaps you were looking at values too soon, before a layout had occurred?

What looks wrong to me is the "After scroll adjustment" (111, 2091). getBoundingClientRect on the iframe says (55.5, 1045.5) which is exactly half that. Perhaps we're applying a conversion twice? Stefan, since IntersectionObserver, could you take a look?

Re #c5: All geometries you observed seem doubled (by device scale?). Are you testing on a high-dpi machine with use-zoom-for-dsf on? The root rect 0,0 822x1646 might not be 411x823/visualViewport.scale, but 411x823*device-scale. I think the correct root rect should be 822x1646/visualViewport.scale.

I feel that the problem is that we are using the layout viewport in visibility observation: https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/intersection_geometry.cc?rcl=5e7b602ba3cf5aca330fc1bf3ac92fbc29e8c388&l=132. 

My tests was on Pixel 2 XL, and Linux (low-dpi) with mobile emulation.

Ah, I'm on a 2X hi-dpi Linux so that's probably it - but I assumed that mobile emulation would emulate the DSF too :(

> I think the correct root rect should be 822x1646/visualViewport.scale

Ah, yes - I think you're right. 

> I feel that the problem is that we are using the layout viewport in visibility observation

Using visual viewport would fix this case but I think this is masking a deeper bug. The layout viewport doesn't change size when you pinch-zoom and the rect you see when you're fully zoomed out is the layout viewport so I think layout viewport is correct in this case (you might report an intersection when the iframe is offscreen but not the other way around). Perhaps we're not resizing the LayoutView correctly to the minimum scale (or IntersectionObserver calculates the size before it's resized).

Stefan, could you take a look? I have another stable blocker I'm currently looking at. If not, I'll get to this after that.

I should have some time to take another dive today.

The politico page seems to have removed the iframe this morning (it did strike me as odd that there'd be an ad on a privacy policy page...). I've put together a local repro (attached here).

From that, it looks like we're using the LayoutView as the root box but we should really be using the FrameView size. The "minimum-scale resize" I mentioned above applied only to the FrameView, not the LayoutView. The LayoutView is always sized to the ICB. This can differ from the minimum-scale rect if we have an element on the page that's wider than 100% of the ICB width. The attached repro should make this clear.

The fix should be easy - I'll put something up today.

Deleted: iframe.html 1015 bytes

iframe.html 1015 bytes View Download

Also applying RBS as the dup'd bug had it and this is something we'll want to merge to stable

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

commit 3fba0d485776197459f82e248c3af1024a4ad398
Author: David Bokan <bokan@chromium.org>
Date: Sun Apr 29 01:09:58 2018

Use FrameView rect for IntersectionObserver root

This bug occurs because of Clank's "resize main frame after layout"
behavior. In a nutshell: we want users to be able to zoom out to that
the entire width of the content fits on screen. Because we can't zoom
out further than the layout viewport (i.e. the main FrameView), after
a layout occurs we resize the main FrameView to match the content width.
This leaves us in an interesting situation - the viewport scroller is
actually larger than the initial containing block (i.e. the LayoutView
size). This situation cannot occur on desktop and generally doesn't
occur on well-behaved mobile sites (sites that don't allow zoom out).

The bug was occuring because the intersection observer used the
LayoutView/ICB size for the root element. This is incorrect in the
situation described above. We should really use the ViewRect which is
the scroller bounds.

Bug:  836408 
Change-Id: I96029cdb11a6a16353d5b8b628b0182c13926a8d
Reviewed-on: https://chromium-review.googlesource.com/1031118
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554661}
[modify] https://crrev.com/3fba0d485776197459f82e248c3af1024a4ad398/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/3fba0d485776197459f82e248c3af1024a4ad398/third_party/WebKit/LayoutTests/virtual/android/frame-size/README.txt
[add] https://crrev.com/3fba0d485776197459f82e248c3af1024a4ad398/third_party/WebKit/LayoutTests/virtual/android/frame-size/intersection-observer-root-uses-frame-size.html
[modify] https://crrev.com/3fba0d485776197459f82e248c3af1024a4ad398/third_party/blink/renderer/core/layout/intersection_geometry.cc

The fix should be in 68.0.3415.0. Xianzhu: can you confirm it's fixed in Canary?

I just tried the test case in #c9. It works on canary and ToT. Thanks for the fix.

Requesting merge to M67

This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit 89bded7abf72e48ca215f97e28dc4737bb6b9109
Author: David Bokan <bokan@chromium.org>
Date: Tue May 01 18:07:31 2018

Use FrameView rect for IntersectionObserver root

This bug occurs because of Clank's "resize main frame after layout"
behavior. In a nutshell: we want users to be able to zoom out to that
the entire width of the content fits on screen. Because we can't zoom
out further than the layout viewport (i.e. the main FrameView), after
a layout occurs we resize the main FrameView to match the content width.
This leaves us in an interesting situation - the viewport scroller is
actually larger than the initial containing block (i.e. the LayoutView
size). This situation cannot occur on desktop and generally doesn't
occur on well-behaved mobile sites (sites that don't allow zoom out).

The bug was occuring because the intersection observer used the
LayoutView/ICB size for the root element. This is incorrect in the
situation described above. We should really use the ViewRect which is
the scroller bounds.

Bug:  836408 
Change-Id: I96029cdb11a6a16353d5b8b628b0182c13926a8d
Reviewed-on: https://chromium-review.googlesource.com/1031118
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#554661}(cherry picked from commit 3fba0d485776197459f82e248c3af1024a4ad398)
Reviewed-on: https://chromium-review.googlesource.com/1037604
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#415}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/89bded7abf72e48ca215f97e28dc4737bb6b9109/third_party/WebKit/LayoutTests/VirtualTestSuites
[add] https://crrev.com/89bded7abf72e48ca215f97e28dc4737bb6b9109/third_party/WebKit/LayoutTests/virtual/android/frame-size/README.txt
[add] https://crrev.com/89bded7abf72e48ca215f97e28dc4737bb6b9109/third_party/WebKit/LayoutTests/virtual/android/frame-size/intersection-observer-root-uses-frame-size.html
[modify] https://crrev.com/89bded7abf72e48ca215f97e28dc4737bb6b9109/third_party/blink/renderer/core/layout/intersection_geometry.cc