So far I have discovered that the coordinate conversion for the |rect_to_scroll| in LayoutObject::ScrollRectToVisibleRecursive does not work correctly for OOPIFs.

For nested non-OOPIF, when crossing the border from <iframe> to parent we do this transform [1]:
EnclosingLayoutRect(
                View()
                    ->LocalToAncestorQuad(
                        FloatRect(rect_to_scroll), parent_view,
                        kUseTransforms | kTraverseDocumentBoundaries)
                    .BoundingBox());

|parent_view| is the LayoutView in parent frame. For OOPIFs we do not have access to that in the <iframe> process. So instead we do [2]:
EnclosingLayoutRect(
      GetLayoutView()
          ->LocalToAbsoluteQuad(FloatRect(rect_to_scroll), 0)
          .BoundingBox());

Which in parent frame is followed with [3]:
LayoutRect new_rect_to_scroll = EnclosingLayoutRect(
      owner_object
          ->LocalToAncestorQuad(FloatRect(rect_to_scroll), owner_object->View(),
                                kUseTransforms | kTraverseDocumentBoundaries)
          .BoundingBox());

and then continue scrolling.

The workaround for OOPIF seems to be incompatible with what we do in non-OOPIF. I still have not figured out what is missing.

David, any thoughts on this problem?

[1]-
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBox.cpp?rcl=bb18f3689f44f3ab938449e6c34ded71f06b4d72&l=714

[2]-
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/LocalFrameView.cpp?rcl=bb18f3689f44f3ab938449e6c34ded71f06b4d72&l=4648

[3]-
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/exported/WebRemoteFrameImpl.cpp?rcl=bb18f3689f44f3ab938449e6c34ded71f06b4d72&l=355

Also cc-ing Ken who might know more about these transforms for OOPIF.

Ehsan, can you explain a bit more about what is going wrong? In both cases it seems like the code is converting the rect coordinates from the iframe's coordinate space to the parent box's coordinate space. At what point is it not doing the right thing?

IIUC, some "scroll offset" is missing in the conversion for the OOPIF case. Essentially, when I log the |rect_to_scroll| before and after the conversions, I see different values for OOPIF vs non-OOPIF and it seems like the OOPIF case is not including some "Scroll Offset". I am still investigating.

When you do LocalToAbsoluteQuad you're converting into "absolute" coordinates. Until --root-layer-scrolls ships (it just flipped in ToT), absolute is equivalent to "document" coordinates (i.e. relative to document origin, doesn't take scroll into account). So when you call owner_object->LocalToAncestorQuad, the rect you have isn't local to the owner_object; it's relative to the owner_object's contentDocument origin. You'll need to adjust to account for the scroll offset if RLS is off. There's conversion functions in LocalFrameView like AbsoluteToRootFrame. You want AbsoluteToFrame which doesn't yet exist but should be trivial to add based on the existing examples.

--root-layer-scrolls makes it so that Absolute == Frame coordinates which should be what you want. Interestingly, adding the --root-layer-scrolls flag doesn't help so maybe there's more going on than what I described...

Thanks David. I will take another look to figure out where the offset is. BTW, is --root-layer-scrolls implemented for OOPIFs? 

Yes, it shouldn't involve anything across frames so it should be agnostic to whether it's inside an OOPIF or not.

Hmm. So I investigated a bit more and I think the fix is non-trivial. Hopefully I am wrong given that I still don't quite understand the inner workings of scroll and layout.

I believe the issue is with LocalFrameView::ScrollIntoView. The input |rect_to_content| is always used as return value [1]. So we need to account for the offsets in ScrollRectToVisibleInRemoteParent() or something.

Now the real challenge is that, at the time we hit ScrollRectToVisibleInRemoteParent(), we do not know what the scroll offset is going to be. This is because when |params.is_for_scroll_sequence| is set, the LocalFrameView::ScrollIntoView would queue the animation and let the offset updates be handled at a later time [2].

If inside one of the OOPIFs we only had one LocalFrameView, the fix would have been simpler (perhaps just to clamp the pending offset and update |rect_to_scroll|
accordingly). But I don't know what to do with multiple frames, i.
   a.com 
  /     \
        b.com
         \
         b.com

which wold have two LocalFrameViews in process for b.com.

Maybe the right fix is to do what is needed for issue 741830.

cc-ing sunyunjia@ for any suggestions.

[1]- https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/LocalFrameView.cpp?rcl=eb989f5c18afd71f198d65e296f9cfd201774c4f&l=5004
[2]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/LocalFrameView.cpp?rcl=eb989f5c18afd71f198d65e296f9cfd201774c4f&l=4989

..actually the pending offset seems to have been applied here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBox.cpp?rcl=3bafd2d7542c4d85c8e3f1e2047c4145c78307ea&l=711

I found one (root) problem which is that the |rect| passed to LocalFrameView::ScrollRectToVisibleInRemoteParent seems to be incorrect. I am working on a CL and will continue discussions there.

FYI: LayoutFrameView::ScrollIntoView should be being used anymore since --root-layer-scrolls is currently enabled in stable (unless you're a few days behind ToT). Perhaps you're looking at the wrong place or the fact that's getting called is the issue?

Thanks. Yes I guess it is not. This line hits though:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBox.cpp?rcl=80693a3b5a96e9e390bafb401cd46a0feb0a61fd&l=707

but the problem is that we should pass |rect_to_scroll| to LocalFrameView::ScrollRectToVisibleInRemoteParent and mistakenly, we are passing |new_rect|.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/LayoutBox.cpp?rcl=80693a3b5a96e9e390bafb401cd46a0feb0a61fd&l=748
I am not sure if there are any other problems other than this.

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

commit 131b9d601fdd9b93a80ba0ce7d18c243384b4f5f
Author: Ehsan Karamad <ekaramad@chromium.org>
Date: Thu Mar 01 18:35:07 2018

Fix a bug in element.scrollIntoView() for OOPIFs

The initial patch [1] landing the feature was passing the wrong rectangle
to scroll to the LocalFrameView of the LocalRoot inside OOPIF process.
The passed rectangle did not include the scroll offset of the
LocalFrameView of the LocalFrameRoot.

This CL fixes the problem and modifies the original test so that
bounding client rectangle of each <iframe> in a nested <iframe>
scenario are precisely tested.

[1]- https://chromium-review.googlesource.com/c/chromium/src/+/679319

Bug:  810291 
Change-Id: Id8859dd9dc70f0bb25519a0c812f7b7a078a9922
Reviewed-on: https://chromium-review.googlesource.com/914871
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540224}
[modify] https://crrev.com/131b9d601fdd9b93a80ba0ce7d18c243384b4f5f/content/browser/site_per_process_browsertest.cc
[modify] https://crrev.com/131b9d601fdd9b93a80ba0ce7d18c243384b4f5f/content/test/data/iframe_out_of_view.html
[modify] https://crrev.com/131b9d601fdd9b93a80ba0ce7d18c243384b4f5f/third_party/WebKit/Source/core/exported/WebRemoteFrameImpl.cpp
[modify] https://crrev.com/131b9d601fdd9b93a80ba0ce7d18c243384b4f5f/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
[modify] https://crrev.com/131b9d601fdd9b93a80ba0ce7d18c243384b4f5f/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/131b9d601fdd9b93a80ba0ce7d18c243384b4f5f/third_party/WebKit/Source/core/layout/LayoutObject.cpp

Following comment #14 I am marking the bug as fixed. This is a small change so we could potentially merge into 65 but unfortunately we are too close to stable cut (Which is today).

I can repro the bug on 64.0.3282.167, 65.0.3325.106, and 66.0.3358.0 on Windows.  Since we haven't heard user reports and it's not a recent regression in Site Isolation, I think it's ok for this fix to go out in M66.  Thanks for checking!