Re^4 land Add UKM Document.OutliveTimeAfterShutdown

This is the 4th trial to add the UKM Document.OutliveTimeAfterShutdown.
This change caused a problem that logs became huge, but now the problem
has been fixed by holte@'s fix (https://chromium-review.googlesource.com/c/chromium/src/+/795362).

This CL adds a new UKM Document.OutliveTimeAfterShutdown, that is
recorded when a Document object survives 5, 10, 20 or 50 garbage
collections after detached. If a document outlives such long time, the
document might be leaked. The UKM would be very useful to know where such
leaky documents exist and to fix them.

Design doc: https://docs.google.com/document/d/1fbs5smdd-pBLLMpq7u8EkyddZILtI7CZPJlo_AA1kak/edit?usp=sharing

Bug: 757374
Change-Id: I20c390c8f3856ed8a9cd733ab6ac18d667b12d95
Reviewed-on: https://chromium-review.googlesource.com/818676Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523708}

components/ukm/test_ukm_recorder.cc

@@ -84,14 +84,11 @@ std::set<ukm::SourceId> TestUkmRecorder::GetSourceIds() const {
 }
 
 const UkmSource* TestUkmRecorder::GetSourceForUrl(const char* url) const {
-  const UkmSource* source = nullptr;
   for (const auto& kv : sources()) {
-    if (kv.second->url() == url) {
-      DCHECK_EQ(nullptr, source);
-      source = kv.second.get();
-    }
+    if (kv.second->url() == url)
+      return kv.second.get();
   }
-  return source;
+  return nullptr;
 }
 
 std::vector<const UkmSource*> TestUkmRecorder::GetSourcesForUrl(

components/ukm/test_ukm_recorder.h

@@ -78,6 +78,9 @@ class TestUkmRecorder : public UkmRecorderImpl {
   // Get all SourceIds with any data associated with them.
   std::set<ukm::SourceId> GetSourceIds() const;
 
+  // Returns the UKM source for the given URL. If there are multiple sources for
+  // the given URL, this returns the first source that is created. If there is
+  // no source for the given URL, this returns nullptr.
   const UkmSource* GetSourceForUrl(const char* url) const;
   const UkmSource* GetSourceForUrl(const GURL& url) const {
     return GetSourceForUrl(url.spec().c_str());

third_party/WebKit/Source/core/dom/BUILD.gn

@@ -369,6 +369,7 @@ blink_core_sources("dom") {
 
   deps = [
     "//services/metrics/public/cpp:metrics_cpp",
+    "//services/metrics/public/cpp:ukm_builders",
     "//services/metrics/public/interfaces",
   ]
 }

third_party/WebKit/Source/core/dom/Document.cpp

@@ -264,6 +264,8 @@
 #include "public/platform/modules/insecure_input/insecure_input_service.mojom-blink.h"
 #include "public/platform/site_engagement.mojom-blink.h"
 #include "services/metrics/public/cpp/mojo_ukm_recorder.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
+#include "services/metrics/public/cpp/ukm_source_id.h"
 #include "services/metrics/public/interfaces/ukm_interface.mojom-shared.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/WebKit/common/page/page_visibility_state.mojom-blink.h"
@@ -315,6 +317,7 @@ class DocumentOutliveTimeReporter : public BlinkGCObserver {
     int outlive_time_count = GetOutliveTimeCount();
     if (outlive_time_count == 5 || outlive_time_count == 10) {
       const char* kUMAString = "Document.OutliveTimeAfterShutdown.GCCount";
+
       if (outlive_time_count == 5)
         UMA_HISTOGRAM_ENUMERATION(kUMAString, kGCCount5, kGCCountMax);
       else if (outlive_time_count == 10)
@@ -322,6 +325,11 @@ class DocumentOutliveTimeReporter : public BlinkGCObserver {
       else
         NOTREACHED();
     }
+
+    if (outlive_time_count == 5 || outlive_time_count == 10 ||
+        outlive_time_count == 20 || outlive_time_count == 50) {
+      document_->RecordUkmOutliveTimeAfterShutdown(outlive_time_count);
+    }
   }
 
  private:
@@ -659,7 +667,9 @@ Document::Document(const DocumentInit& initializer,
       password_count_(0),
       logged_field_edit_(false),
       engagement_level_(mojom::blink::EngagementLevel::NONE),
-      secure_context_state_(SecureContextState::kUnknown) {
+      secure_context_state_(SecureContextState::kUnknown),
+      ukm_source_id_(ukm::kInvalidSourceId),
+      needs_to_record_ukm_outlive_time_(false) {
   if (frame_) {
     DCHECK(frame_->GetPage());
     ProvideContextFeaturesToDocumentFrom(*this, *frame_->GetPage());
@@ -2788,6 +2798,12 @@ void Document::Shutdown() {
   // TODO(crbug.com/729196): Trace why LocalFrameView::DetachFromLayout crashes.
   CHECK(!View()->IsAttached());
 
+  needs_to_record_ukm_outlive_time_ = IsInMainFrame();
+  if (needs_to_record_ukm_outlive_time_) {
+    // Ensure |ukm_recorder_| and |ukm_source_id_|.
+    UkmRecorder();
+  }
+
   // This is required, as our LocalFrame might delete itself as soon as it
   // detaches us. However, this violates Node::detachLayoutTree() semantics, as
   // it's never possible to re-attach. Eventually Document::detachLayoutTree()
@@ -7278,6 +7294,18 @@ void Document::RecordDeferredLoadReason(WouldLoadReason reason) {
   would_load_reason_ = reason;
 }
 
+void Document::RecordUkmOutliveTimeAfterShutdown(int outlive_time_count) {
+  if (!needs_to_record_ukm_outlive_time_)
+    return;
+
+  DCHECK(ukm_recorder_);
+  DCHECK(ukm_source_id_ != ukm::kInvalidSourceId);
+
+  ukm::builders::Document_OutliveTimeAfterShutdown(ukm_source_id_)
+      .SetGCCount(outlive_time_count)
+      .Record(ukm_recorder_.get());
+}
+
 void Document::TraceWrappers(const ScriptWrappableVisitor* visitor) const {
   // node_lists_ are traced in their corresponding NodeListsNodeData, keeping
   // them only alive for live nodes. Otherwise we would keep lists of dead

third_party/WebKit/Source/core/dom/Document.h

@@ -1420,6 +1420,8 @@ class CORE_EXPORT Document : public ContainerNode,
 
   scoped_refptr<WebTaskRunner> GetTaskRunner(TaskType) override;
 
+  void RecordUkmOutliveTimeAfterShutdown(int outlive_time_count);
+
  protected:
   Document(const DocumentInit&, DocumentClassFlags = kDefaultDocumentClass);
 
@@ -1807,6 +1809,8 @@ class CORE_EXPORT Document : public ContainerNode,
   // the document to recorde UKM.
   std::unique_ptr<ukm::UkmRecorder> ukm_recorder_;
   int64_t ukm_source_id_;
+
+  bool needs_to_record_ukm_outlive_time_;
 };
 
 extern template class CORE_EXTERN_TEMPLATE_EXPORT Supplement<Document>;

third_party/WebKit/Tools/Scripts/audit-non-blink-usage.py

@@ -96,6 +96,9 @@ _CONFIG = [
             # but still needed for interop with WebKit/common. Note that other
             # STL types such as std::unique_ptr are encouraged.
             'std::.+',
+
+            # Blink uses UKM for logging e.g. always-on leak detection (crbug/757374)
+            'ukm::.+',
         ],
         'disallowed': ['.+'],
     },

tools/metrics/ukm/ukm.xml

@@ -492,6 +492,24 @@ be describing additional metrics about the same event.
   </metric>
 </event>
 
+<event name="Document.OutliveTimeAfterShutdown">
+  <owner>hajimehoshi@chromium.org</owner>
+  <owner>keishi@chromium.org</owner>
+  <summary>
+    Recorded when a Document object survives certain number of garbage
+    collections after detached. It is expected that regular Document objects are
+    destroyed soon after detached, and if a document outlives longer, probably
+    this can be leaked.
+  </summary>
+  <metric name="GCCount">
+    <summary>
+      Measures the numbers of garbarge collection after the document is
+      detached. This is recorded when the number reached certain numbers like 5
+      or 10.
+    </summary>
+  </metric>
+</event>
+
 <event name="Event.ScrollUpdate.Touch">
   <owner>nzolghadr@chromium.org</owner>
   <summary>