Log to UMA when x-origin frames (1) are created and (2) become visible

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 4164 matching lines @@
 
     // Notify javascript IntersectionObservers
     if (targetState == DocumentLifecycle::PaintClean && frame().document()->intersectionObserverController())
         frame().document()->intersectionObserverController()->computeTrackedIntersectionObservations();
 
     // Adjust render throttling for iframes based on visibility
     bool shouldNotify = !hadValidIntersection || hadEmptyIntersection != m_viewportIntersection.isEmpty();
     if (shouldNotify && !m_renderThrottlingObserverNotificationFactory->isPending())
         m_frame->frameScheduler()->unthrottledTaskRunner()->postTask(BLINK_FROM_HERE, m_renderThrottlingObserverNotificationFactory->cancelAndCreate());
 
+    // TODO(dgrogan): Document why we need this check. For cycles in the frame tree?

[Sami, 2016/08/08 11:09:47]

This is so that we don't traverse the frame tree unnecessarily.

[dgrogan, 2016/08/08 17:45:48]

It what cases will it be triggered though?

[ojan, 2016/08/08 22:01:53]

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Fra...

It's setting dirty bits on the ancestor chain so it knows to walk down the
subtree to a specific frame. THis way we only traverse the subtrees of the frame
tree that have a dirty frame.

In either case, I'd leave out this TODO. I don't think committing a TODO to
leave a comment is very worthwhile. Feel free to add a comment by the member
variable definitions in the header, but I wouldn't bother adding a TODO.

     if (!m_needsUpdateViewportIntersectionInSubtree)
         return;
     m_needsUpdateViewportIntersectionInSubtree = false;
 
     for (Frame* child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling()) {
         if (!child->isLocalFrame())
             continue;
         if (FrameView* view = toLocalFrame(child)->view())
             view->updateViewportIntersectionsForSubtree(targetState);
     }
 }
 
 void FrameView::updateThrottlingStatus()
 {
     // Only offscreen frames can be throttled.
+    // TODO(dgrogan): Change this to DCHECK(m_viewportIntersectionValid)?

[Sami, 2016/08/08 11:09:47]

Right, we should be able to require that.

[ojan, 2016/08/08 22:01:54]

Same as above, either add the DCHECK or leave the whole thing out. Adding
comments like this clutters the code.

[dgrogan, 2016/08/09 00:30:15]

Sorry, it wasn't clear at all, but these comments weren't meant to survive
through to the final patchset. They're more placeholders for us to discuss
stuff. I spun this one out to its own tiny patch already.

     m_hiddenForThrottling = m_viewportIntersectionValid && m_viewportIntersection.isEmpty();
 
     // We only throttle the rendering pipeline in cross-origin frames. This is
     // to avoid a situation where an ancestor frame directly depends on the
     // pipeline timing of a descendant and breaks as a result of throttling.
     // The rationale is that cross-origin frames must already communicate with
     // asynchronous messages, so they should be able to tolerate some delay in
     // receiving replies from a throttled peer.
     //
     // Check if we can access our parent's security origin.
@@ skipping 18 matching lines @@
 
 void FrameView::notifyRenderThrottlingObservers()
 {
     TRACE_EVENT0("blink", "FrameView::notifyRenderThrottlingObservers");
     DCHECK(!isInPerformLayout());
     DCHECK(!m_frame->document() || !m_frame->document()->inStyleRecalc());
     bool wasThrottled = canThrottleRendering();
 
     updateThrottlingStatus();
 
+    frame().onVisibilityMaybeChanged(!m_hiddenForThrottling);
+
     bool becameThrottled = !wasThrottled && canThrottleRendering();
     bool becameUnthrottled = wasThrottled && !canThrottleRendering();
     ScrollingCoordinator* scrollingCoordinator = this->scrollingCoordinator();
     if (becameThrottled) {
         // If this FrameView became throttled, we must make sure all of its
         // children become throttled at the same time. Otherwise we might
         // attempt to paint one of the children with an out-of-date layout
         // before |notifyRenderThrottlingObservers| has made it throttled.
         forAllNonThrottledFrameViews([](FrameView& frameView) {
             frameView.m_subtreeThrottled = true;
@@ skipping 35 matching lines @@
 }
 
 bool FrameView::canThrottleRendering() const
 {
     if (!RuntimeEnabledFeatures::renderingPipelineThrottlingEnabled())
         return false;
     return m_subtreeThrottled || (m_hiddenForThrottling && m_crossOriginForThrottling);
 }
 
 } // namespace blink