Propagate viewport intersection across OOPIFs at the correct time

third_party/WebKit/Source/core/frame/FrameView.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Dirk Mueller <mueller@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
  *           (C) 2006 Graham Dennis (graham.dennis@gmail.com)
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
@@ skipping 4919 matching lines @@
       !owner_element->GetLayoutObject()) {
     // No need to notify children because descendants of display:none frames
     // should remain throttled.
     UpdateRenderThrottlingStatus(hidden_for_throttling_, subtree_throttled_,
                                  kDontForceThrottlingInvalidation,
                                  kDontNotifyChildren);
   }
 
   for (Frame* child = frame_->Tree().FirstChild(); child;
        child = child->Tree().NextSibling()) {
-    if (!child->IsLocalFrame())
+    DCHECK(child->IsRemoteFrame() || child->IsLocalFrame());

[dcheng, 2017/05/05 18:25:14]

Isn't this DCHECK always going to be true?

[kenrb, 2017/05/05 19:01:31]

Aren't *all* DCHECKs supposed to be always true? :)

I see your point, though. Violating that would be a significant change to the
Frame code and would break Blink in a lot of ways. I'll take it out.

+    if (child->IsRemoteFrame()) {
+      if (RemoteFrameView* view = ToRemoteFrame(child)->View())
+        view->UpdateRemoteViewportIntersection();

[dcheng, 2017/05/05 18:25:14]

It almost feels like View() and <update intersections> should be virtuals.
*shrug*

[kenrb, 2017/05/05 19:01:31]

That might be reasonable for View(). For UpdateViewportIntersections I think the
difference in function arguments makes it more trouble than it's worth.

[dcheng, 2017/05/05 22:47:53]

Do you mind poking at this in a followup?

FWIW, I don't think combining the methods for RemoteFrameView/FrameView is that
bad: it's just one parameter to ignore, and it removes explicit downcasts from
the codebase. I think the explicit downcasts have been a pretty big headache
overall, and using virtual dispatch will result in overall simpler to reason
about code.

       continue;
+    }
     if (FrameView* view = ToLocalFrame(child)->View())
       view->UpdateViewportIntersectionsForSubtree(target_state);
   }
 }
 
 void FrameView::UpdateRenderThrottlingStatusForTesting() {
   visibility_observer_->DeliverObservationsForTesting();
 }
 
 void FrameView::CrossOriginStatusChanged() {
@@ skipping 442 matching lines @@
 void FrameView::SetAnimationHost(
     std::unique_ptr<CompositorAnimationHost> host) {
   animation_host_ = std::move(host);
 }
 
 LayoutUnit FrameView::CaretWidth() const {
   return LayoutUnit(GetChromeClient()->WindowToViewportScalar(1));
 }
 
 }  // namespace blink