Handle cross origin with src attribution for descendants

third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp

Index: third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
diff --git a/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp b/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
index 2ad9c42c205e34c630e4d334e17b14fefb103cdd..b2d5218457aa5759e009604b20ba6483d8b10899 100644
--- a/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
+++ b/third_party/WebKit/Source/core/inspector/InspectorWebPerfAgent.cpp
@@ -7,20 +7,25 @@
 #include "core/InstrumentingAgents.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExecutionContext.h"
+#include "core/frame/DOMWindow.h"
 #include "core/frame/Frame.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Location.h"
+#include "core/html/HTMLFrameOwnerElement.h"
 #include "core/inspector/InspectedFrames.h"
 #include "core/timing/DOMWindowPerformance.h"
 #include "core/timing/Performance.h"
 #include "public/platform/Platform.h"
 
+
 namespace blink {
 
 namespace {
 static const double kLongTaskThresholdMillis = 50.0;
 static const char* unknownAttribution = "unknown";
 static const char* ambiguousAttribution = "multiple-contexts";
+static const char* crossOriginAttribution = "cross-origin";

[pfeldman, 2016/09/30 23:38:50]

style: here, above and below: static const char[]

[panicker, 2016/10/01 00:25:28]

Done.

+static const char* ancestorAttribution = "ancestor";
 
 bool canAccessOrigin(Frame* frame1, Frame* frame2)
 {
@@ -101,8 +106,12 @@ void InspectorWebPerfAgent::ReportTaskTime(
         m_frameContextLocations, m_inspectedFrames->root()));
 }
 
+/**
+ * Report sanitized name based on cross-origin policy.
+ * See detailed Security doc here: http://bit.ly/2duD3F7
+ */
 String InspectorWebPerfAgent::sanitizedLongTaskName(
-    const HeapHashSet<Member<Location>>& frameContextLocations, Frame* rootFrame)
+    const HeapHashSet<Member<Location>>& frameContextLocations, Frame* observerFrame)
 {
     if (frameContextLocations.size() == 0) {
         // Unable to attribute as no script was involved.
@@ -115,11 +124,30 @@ String InspectorWebPerfAgent::sanitizedLongTaskName(
     // Exactly one culprit location, attribute based on origin boundary.
     DCHECK_EQ(1u, frameContextLocations.size());
     Location* culpritLocation = *frameContextLocations.begin();
-    if (canAccessOrigin(rootFrame, culpritLocation->frame())) {
+    if (canAccessOrigin(observerFrame, culpritLocation->frame())) {
         // For same origin, it's safe to to return culprit location URL.
         return culpritLocation->href();
     }
-    return "cross-origin";
+    if (culpritLocation->frame()->tree().isDescendantOf(observerFrame)) {
+        // If culprit is a descendant of the observer, then walk up the tree from culprit
+        // to observer, and report "src" for the *last* cross-origin (from observer) frame.
+        // If no intermediate cross-origin frame is found, then report the culprit directly.
+        Frame* lastCrossOriginFrame = culpritLocation->frame();
+        for (Frame* frame = culpritLocation->frame(); frame != observerFrame; frame = frame->tree().parent()) {
+            if (!canAccessOrigin(observerFrame, frame)) {
+                lastCrossOriginFrame = frame;
+            }
+        }
+        if (lastCrossOriginFrame->domWindow() && lastCrossOriginFrame->domWindow()->document()) {
+            HTMLFrameOwnerElement* localOwner = lastCrossOriginFrame->domWindow()->document()->localOwner();
+            if (localOwner && localOwner->hasAttribute(HTMLNames::srcAttr)) {
+                return localOwner->getAttribute(HTMLNames::srcAttr);

[skobes, 2016/09/30 21:26:31]

Is the src attribute updated on navigation?  I wonder if it would be better to
use something like lastCrossOriginFrame->domWindow()->location()->href().

[skobes, 2016/09/30 21:45:01]

Disregard; from reading the doc I see we avoid location.href on purpose to avoid
leaking information.  I'd suggest adding a comment explaining this.

[panicker, 2016/09/30 22:57:15]

Acknowledged.

+            }
+        }
+    } else if (observerFrame->tree().isDescendantOf(culpritLocation->frame())) {
+        return ancestorAttribution;
+    }
+    return crossOriginAttribution;
 }
 
 DEFINE_TRACE(InspectorWebPerfAgent)