Emit "function calls in detached window" counters via DetachedWindows UKM event

Privacy review:
https://docs.google.com/document/d/1cgwwc8jY7A3MLCHAh7yvONTAWTUTlsjnBsHp2oIuMv0/edit?usp=sharing

Change-Id: Ic7ee5a3664d592230e1e120391724dd4e7a0c5dd
Bug: chromium:1018156
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1965747Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727045}

components/ukm/content/source_url_recorder.cc

@@ -80,6 +80,7 @@ class SourceUrlRecorderWebContentsObserver
 
   // blink::mojom::UkmSourceIdFrameHost
   void SetDocumentSourceId(int64_t source_id) override;
+  void GetNavigationSourceId(GetNavigationSourceIdCallback callback) override;
 
  private:
   explicit SourceUrlRecorderWebContentsObserver(
@@ -317,6 +318,11 @@ ukm::SourceId SourceUrlRecorderWebContentsObserver::
   return last_committed_full_navigation_or_same_document_source_id_;
 }
 
+void SourceUrlRecorderWebContentsObserver::GetNavigationSourceId(
+    GetNavigationSourceIdCallback callback) {
+  std::move(callback).Run(last_committed_full_navigation_source_id_);
+}
+
 void SourceUrlRecorderWebContentsObserver::SetDocumentSourceId(
     int64_t source_id) {
   content::RenderFrameHost* main_frame = web_contents()->GetMainFrame();

third_party/blink/public/mojom/ukm/ukm.mojom

@@ -5,8 +5,14 @@
 module blink.mojom;
 
 // This interface is associated with a frame.
+// TODO(bartekn): Browser should generate source ID and navigation source ID,
+// and push them to the renderer.
 interface UkmSourceIdFrameHost {
   // The renderer tells the browser the source id of the document. This is
   // called once per document when the document's loading is completed.
   SetDocumentSourceId(int64 source_id);
+
+  // Returns the source id of the last navigation.
+  // This is called once per document when the document's loading is completed.
+  GetNavigationSourceId() => (int64 navigation_source_id);
 };

third_party/blink/renderer/bindings/core/v8/use_counter_callback.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/use_counter_callback.h"
 
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
+#include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/frame/deprecation.h"
 #include "third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h"
@@ -20,6 +21,7 @@ void UseCounterCallback(v8::Isolate* isolate,
 
   WebFeature blink_feature;
   bool deprecated = false;
+  bool detached_window_call = false;
   switch (feature) {
     case v8::Isolate::kUseAsm:
       blink_feature = WebFeature::kUseAsm;
@@ -245,56 +247,69 @@ void UseCounterCallback(v8::Isolate* isolate,
       blink_feature = WebFeature::kV8SharedArrayBufferConstructed;
       break;
     // The following 9 counters differ from the rest, because they're reported
-    // via UKM using the instance counter mechanism. Using the typical use
-    // counter infrastructure won't work, because it doesn't work on detached
-    // windows.
+    // to UKM using alternative mechanisms. The use counter logic doesn't work
+    // on detached windows.
     // TODO(bartekn,chromium:1018156): Remove once not needed.
     case v8::Isolate::kCallInDetachedWindowByNavigation:
       InstanceCounters::IncrementCounter(
           InstanceCounters::kV8CallInDetachedWindowByNavigationCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByNavigationAfter10s:
       InstanceCounters::IncrementCounter(
           InstanceCounters::kV8CallInDetachedWindowByNavigationAfter10sCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByNavigationAfter1min:
       InstanceCounters::IncrementCounter(
           InstanceCounters::
               kV8CallInDetachedWindowByNavigationAfter1minCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByClosing:
       InstanceCounters::IncrementCounter(
           InstanceCounters::kV8CallInDetachedWindowByClosingCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByClosingAfter10s:
       InstanceCounters::IncrementCounter(
           InstanceCounters::kV8CallInDetachedWindowByClosingAfter10sCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByClosingAfter1min:
       InstanceCounters::IncrementCounter(
           InstanceCounters::kV8CallInDetachedWindowByClosingAfter1minCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByOtherReason:
       InstanceCounters::IncrementCounter(
           InstanceCounters::kV8CallInDetachedWindowByOtherReasonCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByOtherReasonAfter10s:
       InstanceCounters::IncrementCounter(
           InstanceCounters::
               kV8CallInDetachedWindowByOtherReasonAfter10sCounter);
-      return;
+      detached_window_call = true;
+      break;
     case v8::Isolate::kCallInDetachedWindowByOtherReasonAfter1min:
       InstanceCounters::IncrementCounter(
           InstanceCounters::
               kV8CallInDetachedWindowByOtherReasonAfter1minCounter);
-      return;
+      detached_window_call = true;
+      break;
     // End of special case.
     default:
       // This can happen if V8 has added counters that this version of Blink
       // does not know about. It's harmless.
       return;
   }
-  if (deprecated) {
+  if (detached_window_call) {
+    // Detached window call counters are interesting only in the Document case.
+    Document* document = DynamicTo<Document>(CurrentExecutionContext(isolate));
+    if (document)
+      document->RecordCallInDetachedWindow(feature);
+  } else if (deprecated) {
     Deprecation::CountDeprecation(CurrentExecutionContext(isolate),
                                   blink_feature);
   } else {

third_party/blink/renderer/core/dom/document.cc

@@ -4177,6 +4177,17 @@ bool Document::CheckCompletedInternal() {
       return false;
   }
 
+  if (frame_ && frame_->Client()->GetRemoteNavigationAssociatedInterfaces()) {
+    ukm_binding_ = std::make_unique<
+        mojo::AssociatedRemote<mojom::blink::UkmSourceIdFrameHost>>();
+    frame_->Client()->GetRemoteNavigationAssociatedInterfaces()->GetInterface(
+        ukm_binding_.get());
+    DCHECK(ukm_binding_->is_bound());
+    auto callback =
+        WTF::Bind(&Document::SetNavigationSourceId, WrapPersistent(this));
+    (*ukm_binding_.get())->GetNavigationSourceId(std::move(callback));
+  }
+
   // OK, completed. Fire load completion events as needed.
   SetReadyState(kComplete);
   if (LoadEventStillNeeded())
@@ -4203,11 +4214,8 @@ bool Document::CheckCompletedInternal() {
 
     // Send the source ID of the document to the browser.
     if (frame_->Client()->GetRemoteNavigationAssociatedInterfaces()) {
-      mojo::AssociatedRemote<mojom::blink::UkmSourceIdFrameHost> ukm_binding;
-      frame_->Client()->GetRemoteNavigationAssociatedInterfaces()->GetInterface(
-          &ukm_binding);
-      DCHECK(ukm_binding.is_bound());
-      ukm_binding->SetDocumentSourceId(ukm_source_id_);
+      DCHECK(ukm_binding_->is_bound());
+      (*ukm_binding_.get())->SetDocumentSourceId(ukm_source_id_);
     }
 
     frame_->GetFrameScheduler()->RegisterStickyFeature(
@@ -8685,6 +8693,104 @@ void Document::CountUse(mojom::WebFeature feature) {
   }
 }
 
+void Document::RecordCallInDetachedWindow(
+    v8::Isolate::UseCounterFeature reason) {
+  // Emit each reason only once (max twice, in the case explained below).
+  // We're mainly interested if this kind of call occurred in a page or not.
+  // It would be nice to count how many times, but that would flood the UKM
+  // infrastructure with too many events.
+  if (calls_in_detached_window_emitted_.Contains(reason))
+    return;
+
+  if (navigation_source_id_ == ukm::kInvalidSourceId) {
+    // It's possible that navigation_source_id_ isn't set yet. Emit the event
+    // with invalid ID (at most once) so that we could potentially make use of
+    // it in combination with DocumentCreated event, using document's source ID.
+    // However, save it to give it a chance to be emitted again with valid
+    // navigation_source_id_.
+    if (calls_in_detached_window_orphaned_.Contains(reason))
+      return;
+    calls_in_detached_window_orphaned_.insert(reason);
+  }
+
+  HashSet<v8::Isolate::UseCounterFeature> reasons;
+  reasons.insert(reason);
+  EmitDetachedWindowsUkmEvent(reasons);
+}
+
+void Document::SetNavigationSourceId(int64_t source_id) {
+  navigation_source_id_ = source_id;
+  if (navigation_source_id_ != ukm::kInvalidSourceId) {
+    // Now that a valid navigation_source_id_ is set, re-emit the DetacheWindows
+    // event for cases that were emitted with invalid ID.
+    EmitDetachedWindowsUkmEvent(calls_in_detached_window_orphaned_);
+    calls_in_detached_window_orphaned_.clear();
+  }
+}
+
+void Document::EmitDetachedWindowsUkmEvent(
+    const HashSet<v8::Isolate::UseCounterFeature>& reasons) {
+  if (reasons.IsEmpty())
+    return;
+
+  DCHECK_NE(ukm_source_id_, ukm::kInvalidSourceId);
+  ukm::builders::DetachedWindows_Experimental builder(ukm_source_id_);
+  // Invalid ID should be 0 to take advantage of the protobuf default.
+  DCHECK_EQ(ukm::kInvalidSourceId, 0);
+  if (navigation_source_id_ != ukm::kInvalidSourceId)
+    builder.SetNavigationSourceId(navigation_source_id_);
+  for (auto reason : reasons) {
+    if (navigation_source_id_ != ukm::kInvalidSourceId)
+      calls_in_detached_window_emitted_.insert(reason);
+
+    switch (reason) {
+      case v8::Isolate::kCallInDetachedWindowByNavigation:
+        builder.SetNumberOfCallsInDetachedWindowByNavigation(1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByNavigationAfter10s:
+        builder
+            .SetNumberOfCallsInDetachedWindowByNavigation_After10sSinceDetaching(
+                1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByNavigationAfter1min:
+        builder
+            .SetNumberOfCallsInDetachedWindowByNavigation_After1minSinceDetaching(
+                1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByClosing:
+        builder.SetNumberOfCallsInDetachedWindowByClosing(1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByClosingAfter10s:
+        builder
+            .SetNumberOfCallsInDetachedWindowByClosing_After10sSinceDetaching(
+                1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByClosingAfter1min:
+        builder
+            .SetNumberOfCallsInDetachedWindowByClosing_After1minSinceDetaching(
+                1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByOtherReason:
+        builder.SetNumberOfCallsInDetachedWindowByOtherReason(1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByOtherReasonAfter10s:
+        builder
+            .SetNumberOfCallsInDetachedWindowByOtherReason_After10sSinceDetaching(
+                1);
+        break;
+      case v8::Isolate::kCallInDetachedWindowByOtherReasonAfter1min:
+        builder
+            .SetNumberOfCallsInDetachedWindowByOtherReason_After1minSinceDetaching(
+                1);
+        break;
+      default:
+        LOG(DFATAL) << "Use counter not related to detached windows: "
+                    << reason;
+    }
+  }
+  builder.Record(UkmRecorder());
+}
+
 void Document::CountDeprecation(mojom::WebFeature feature) {
   // TODO(yoichio): We should remove these counters when v0 APIs are removed.
   // crbug.com/946875.

third_party/blink/renderer/core/dom/document.h

@@ -37,6 +37,8 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/timer/elapsed_timer.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
+#include "third_party/blink/public/mojom/ukm/ukm.mojom-blink.h"
 #include "third_party/blink/public/platform/web_focus_type.h"
 #include "third_party/blink/public/platform/web_insecure_request_policy.h"
 #include "third_party/blink/renderer/core/accessibility/axid.h"
@@ -1583,6 +1585,8 @@ class CORE_EXPORT Document : public ContainerNode,
   bool IsAnimatedPropertyCounted(CSSPropertyID property) const;
   void ClearUseCounterForTesting(mojom::WebFeature);
 
+  void RecordCallInDetachedWindow(v8::Isolate::UseCounterFeature reason);
+
   // Bind Content Security Policy to this document. This will cause the
   // CSP to resolve the 'self' attribute and all policies will then be
   // applied to this document.
@@ -1783,6 +1787,12 @@ class CORE_EXPORT Document : public ContainerNode,
   void ProcessDisplayLockActivationObservation(
       const HeapVector<Member<IntersectionObserverEntry>>&);
 
+  void SetNavigationSourceId(int64_t source_id);
+
+  // TODO(bartekn): Remove after investigation is completed.
+  void EmitDetachedWindowsUkmEvent(
+      const HashSet<v8::Isolate::UseCounterFeature>& reasons);
+
   DocumentLifecycle lifecycle_;
 
   bool evaluate_media_queries_on_style_recalc_;
@@ -2056,6 +2066,10 @@ class CORE_EXPORT Document : public ContainerNode,
   int64_t ukm_source_id_;
   bool needs_to_record_ukm_outlive_time_;
 
+  std::unique_ptr<mojo::AssociatedRemote<mojom::blink::UkmSourceIdFrameHost>>
+      ukm_binding_;
+  uint64_t navigation_source_id_ = ukm::kInvalidSourceId;
+
   // Tracks and reports UKM metrics of the number of attempted font family match
   // attempts (both successful and not successful) by the page.
   std::unique_ptr<FontMatchingMetrics> font_matching_metrics_;
@@ -2154,6 +2168,9 @@ class CORE_EXPORT Document : public ContainerNode,
       element_explicitly_set_attr_elements_map_;
 
   Member<IntersectionObserver> display_lock_activation_observer_;
+
+  HashSet<v8::Isolate::UseCounterFeature> calls_in_detached_window_orphaned_;
+  HashSet<v8::Isolate::UseCounterFeature> calls_in_detached_window_emitted_;
 };
 
 extern template class CORE_EXTERN_TEMPLATE_EXPORT Supplement<Document>;

tools/metrics/ukm/ukm.xml

@@ -3038,6 +3038,73 @@ be describing additional metrics about the same event.
   </metric>
 </event>
 
+<event name="DetachedWindows.Experimental">
+  <owner>bartekn@chromium.org</owner>
+  <summary>
+    Metrics related to detached windows.
+  </summary>
+  <metric name="NavigationSourceId">
+    <summary>
+      Contains the UKM source id of the navigation as an integer.
+    </summary>
+  </metric>
+  <metric name="NumberOfCallsInDetachedWindowByClosing">
+    <summary>
+      The number of function calls in a window detached by closing/removal.
+    </summary>
+  </metric>
+  <metric name="NumberOfCallsInDetachedWindowByClosing_After10sSinceDetaching">
+    <summary>
+      The number of function calls in a window detached by closing/removal, that
+      happened at least 10 seconds after detaching.
+    </summary>
+  </metric>
+  <metric name="NumberOfCallsInDetachedWindowByClosing_After1minSinceDetaching">
+    <summary>
+      The number of function calls in a window detached by closing/removal, that
+      happened at least 1 minute after detaching.
+    </summary>
+  </metric>
+  <metric name="NumberOfCallsInDetachedWindowByNavigation">
+    <summary>
+      The number of function calls in a window detached by navigating away.
+    </summary>
+  </metric>
+  <metric
+      name="NumberOfCallsInDetachedWindowByNavigation_After10sSinceDetaching">
+    <summary>
+      The number of function calls in a window detached by navigating away, that
+      happened at least 10 seconds after detaching.
+    </summary>
+  </metric>
+  <metric
+      name="NumberOfCallsInDetachedWindowByNavigation_After1minSinceDetaching">
+    <summary>
+      The number of function calls in a window detached by navigating away, that
+      happened at least 1 minute after detaching.
+    </summary>
+  </metric>
+  <metric name="NumberOfCallsInDetachedWindowByOtherReason">
+    <summary>
+      The number of function calls in a window detached by another reason.
+    </summary>
+  </metric>
+  <metric
+      name="NumberOfCallsInDetachedWindowByOtherReason_After10sSinceDetaching">
+    <summary>
+      The number of function calls in a window detached by another reason, that
+      happened at least 10 seconds after detaching.
+    </summary>
+  </metric>
+  <metric
+      name="NumberOfCallsInDetachedWindowByOtherReason_After1minSinceDetaching">
+    <summary>
+      The number of function calls in a window detached by another reason, that
+      happened at least 1 minute after detaching.
+    </summary>
+  </metric>
+</event>
+
 <event name="Document.OutliveTimeAfterShutdown">
   <owner>hajimehoshi@chromium.org</owner>
   <owner>keishi@chromium.org</owner>