Blink changes to record interest rects for http://w3c.github.io/frame-timing/

Source/core/frame/FrameView.cpp

Index: Source/core/frame/FrameView.cpp
diff --git a/Source/core/frame/FrameView.cpp b/Source/core/frame/FrameView.cpp
index add17f97e6f65830268df93b0136af9dba331407..0ce09b6ed15dccd3b1169fcbd8c21ac0963a8050 100644
--- a/Source/core/frame/FrameView.cpp
+++ b/Source/core/frame/FrameView.cpp
@@ -2535,6 +2535,9 @@ void FrameView::updateLayoutAndStyleForPainting()
 
         updateCompositedSelectionIfNeeded();
 
+        if (RuntimeEnabledFeatures::frameTimingSupportEnabled())
+            updateFrameTimingRequestsIfNeeded();
+
         scrollContentsIfNeededRecursive();
 
         invalidateTreeIfNeededRecursive();
@@ -2545,6 +2548,17 @@ void FrameView::updateLayoutAndStyleForPainting()
     ASSERT(lifecycle().state() == DocumentLifecycle::PaintInvalidationClean);
 }
 
+void FrameView::updateFrameTimingRequestsIfNeeded()
+{
+    GraphicsLayerFrameTimingRequests graphicsLayerTimingRequests;

[chrishtr, 2015/05/08 16:52:04]

Add a TODO(mpb) to add the dirty bit.

[MikeB, 2015/05/08 18:37:34]

Done.

+    collectFrameTimingRequestsRecursive(graphicsLayerTimingRequests);
+
+    for (const auto& iter : graphicsLayerTimingRequests) {
+        const GraphicsLayer* graphicsLayer = iter.key;
+        graphicsLayer->platformLayer()->setFrameTimingRequests(iter.value);
+    }
+}
+
 void FrameView::updateLayoutAndStyleIfNeededRecursive()
 {
     // We have to crawl our entire tree looking for any FrameViews that need
@@ -4004,4 +4018,50 @@ LayoutObject* FrameView::viewportLayoutObject()
     return nullptr;
 }
 
+void FrameView::collectFrameTimingRequestsRecursive(
+    GraphicsLayerFrameTimingRequests& graphicsLayerTimingRequests)

[chrishtr, 2015/05/08 16:52:04]

Put this on the same line. Blink has no 80-column restriction..

[MikeB, 2015/05/08 18:37:34]

Done.

+{
+    for (Frame* frame = m_frame.get(); frame;
+        frame = frame->tree().traverseNext(m_frame.get())) {

[chrishtr, 2015/05/08 16:52:04]

same here IMO. Consider reducing lines in the rest of this method also.

[MikeB, 2015/05/08 18:37:34]

Done.

+        if (!frame->isLocalFrame())
+            continue;
+        LocalFrame* localFrame = toLocalFrame(frame);
+        LayoutRect rect = localFrame->contentLayoutObject()->viewRect();
+
+        DeprecatedPaintLayer::mapRectToPaintInvalidationBacking(
+            localFrame->contentLayoutObject(),
+            localFrame->contentLayoutObject()->containerForPaintInvalidation(),
+            rect);
+
+        const DeprecatedPaintLayer* layer = localFrame->contentLayoutObject()
+            ->enclosingLayer()
+            ->enclosingLayerForPaintInvalidationCrossingFrameBoundaries();

[chrishtr, 2015/05/08 16:52:04]

containerForPaintInvalidation() is the same as this, you can reuse the
computation from line 4033. In fact this method should probably be removed,
I'll make a note for myself.

[MikeB, 2015/05/08 18:37:34]

Done.

+        const GraphicsLayer* graphicsLayer = !layer ? nullptr :
+            layer->graphicsLayerBacking();
+
+        if (!graphicsLayer)
+            continue;
+
+        GraphicsLayerFrameTimingRequests::iterator requestIterator =
+            graphicsLayerTimingRequests.find(graphicsLayer);

[chrishtr, 2015/05/08 16:52:04]

I think you can just write:

graphicsLayerTimingRequests[graphicsLayer].push_back(std::make_pair(frame->frameID(),
enclosingIntRect(rect)));

the [] operator on a hash map makes a value if there was previously no entry
with the given key.

[MikeB, 2015/05/08 18:37:34]

error: no viable overloaded operator[] for type
'GraphicsLayerFrameTimingRequests' (aka 'HashMap<const blink::GraphicsLayer *,
std::vector<std::pair<int64_t, WebRect> > >')

I think that might be true of hash_map but not HashMap.

Although it looks like HashMap::add will do nothing if the key is already
present, so I think I can get away with the one-liner:

graphicsLayerTimingRequests.add(graphicsLayer,std::vector<std::pair<int64_t,
WebRect>>()).storedValue->value.push_back(std::make_pair(frame->frameID(),
enclosingIntRect(rect)));

+        std::vector<std::pair<int64_t, WebRect>> *graphicsLayerTimingRects;
+        if (requestIterator == graphicsLayerTimingRequests.end()) {
+            graphicsLayerTimingRects = &graphicsLayerTimingRequests.add(
+                graphicsLayer,
+                std::vector<std::pair<int64_t, WebRect>>()).storedValue->value;
+        } else {
+            graphicsLayerTimingRects = &requestIterator->value;
+        }
+        graphicsLayerTimingRects->push_back(
+            std::make_pair(frame->frameID(), enclosingIntRect(rect)));
+
+        if (frame != m_frame.get()) {

[chrishtr, 2015/05/08 16:52:04]

Factor into two methods, one which does the recursion and one which fills in
hash map values. There are other recursions just like that in this file, such as
invalidateTreeIfNeededRecursive that you can copy the pattern from.

[MikeB, 2015/05/08 18:37:34]

Done.

+            if (FrameView* view = localFrame->view()) {
+                view->collectFrameTimingRequestsRecursive(
+                    graphicsLayerTimingRequests);
+            }
+        }
+    }
+}
+
 } // namespace blink