Deferred frame loading stats v2

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

Index: third_party/WebKit/Source/core/frame/FrameView.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index 5cfb85f3f5f5fddc20055ed245de877eeb10987f..b1af08efbc6df9d67956fdf1861c59219b281b35 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -4311,11 +4311,29 @@ void FrameView::updateViewportIntersectionIfNeeded() {
   m_viewportIntersectionValid = true;
   FrameView* parent = parentFrameView();
   if (!parent) {
+    HTMLFrameOwnerElement* element = frame().deprecatedLocalOwner();
+    // Frames have no layout object approximately iff they are display:none.
+    // And out-of-process frames are the only other frames that have no parent.

[dgrogan, 2016/10/13 01:06:22]

Do you know if the line4316 comment is correct? Why might cross-origin frames
not have a parent?

+    if (element && !element->layoutObject())
+      frame().document()->wouldLoadBecause(WouldLoadDisplayNone);
+    else

[dcheng, 2016/10/13 23:00:43]

Right now, this will record WouldLoadOutOfProcess if an iframe's parent is
in-process and has a layout object (since it doesn't meet both of the first
conditions)

Let's restructure this to:

// Determining visibility uses the owner element's LayoutObject,
// which is not currently possible if the parent is
// remote.
if (!element)
  wouldLoadBecause(WouldLoadOutOfProcess)
// Having no layout object means the frame is not drawn.
else if (!element->layoutObject())
  wouldLoadBecause(WouldLoadDisplayNone)

which also puts each comment near the the clause it documents.

[dgrogan, 2016/10/14 21:45:08]

If an iframe's parent is in-process we wouldn't enter this block because of the
!parent check above?
Do we need to worry about the "else" case? What could hit it? I added "else
NOTREACHED()" and 3 layout tests hit it but I don't see what's common between
them except perhaps objects/iframes being unloaded.
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/dom/...
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/load...
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/imported/...

[dcheng, 2016/10/14 23:40:30]

Ah, I misread this originally. So I would expect parentFrameView() to only be
null for two cases:
- |this| is the main frame
- |this| is a child frame with a remote parent.

Taking that into account, I think the conditions we need to check for
display:none should be (I believe display:none iframes still have a FrameView):

if (parentFrameView() && !frame().deprecatedLocalOwner().layoutObject())
  wouldLoadBecause(DisplayNone);

And then for the remote frame case:

else if (frame->owner() && frame->owner()->isRemote()) {
  wouldLoadBecause(OutOfProcess);
}

Does this match the desired semantics?
probably not, but it's hard to say for sure =/

+      frame().document()->wouldLoadBecause(WouldLoadOutOfProcess);
     m_viewportIntersection = frameRect();
     return;
   }
   ASSERT(!parent->m_needsUpdateViewportIntersection);
 
+  bool parentLoaded = parent->frame().document()->wouldLoadReason() > Created;

[dgrogan, 2016/10/13 01:06:22]

I suspect ">" makes the meaning clearer at first glance.

+  // If the parent wasn't loaded, the children won't be either.
+  if (parentLoaded) {
+    if (frameRect().isEmpty())
+      frame().document()->wouldLoadBecause(WouldLoadZeroByZero);
+    else if (frameRect().maxY() < 0)
+      frame().document()->wouldLoadBecause(WouldLoadAbove);
+    else if (frameRect().maxX() < 0)
+      frame().document()->wouldLoadBecause(WouldLoadLeft);
+  }
+
   // If our parent is hidden, then we are too.
   if (parent->m_viewportIntersection.isEmpty()) {
     m_viewportIntersection = parent->m_viewportIntersection;
@@ -4335,6 +4353,9 @@ void FrameView::updateViewportIntersectionIfNeeded() {
   // content while scrolling.
   IntRect viewport = parent->m_viewportIntersection;
   m_viewportIntersection.intersect(viewport);
+
+  if (parentLoaded && !m_viewportIntersection.isEmpty())
+    frame().document()->wouldLoadBecause(WouldLoadVisible);
 }
 
 void FrameView::updateViewportIntersectionsForSubtree(
@@ -4420,8 +4441,6 @@ void FrameView::notifyRenderThrottlingObservers() {
 
   updateThrottlingStatus();
 
-  frame().document()->onVisibilityMaybeChanged(!m_hiddenForThrottling);
-
   bool becameThrottled = !wasThrottled && canThrottleRendering();
   bool becameUnthrottled = wasThrottled && !canThrottleRendering();
   ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator();