Separate independent events in AbusiveExperienceHeuristic

Change-Id: I0deb1e45f2e76ac887e9a03d6e48d151211b1c97
Reviewed-on: https://chromium-review.googlesource.com/1191726
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587866}

chrome/browser/subresource_filter/ad_tagging_browsertest.cc

@@ -342,12 +342,12 @@ const ukm::mojom::UkmEntry* FindDocumentCreatedEntry(
 
 void ExpectLatestUkmEntry(const ukm::TestUkmRecorder& ukm_recorder,
                           size_t expected_num_entries,
-                          base::StringPiece metric_name,
                           bool from_main_frame,
                           const GURL& main_frame_url,
-                          int64_t expected_value) {
+                          bool from_ad_subframe,
+                          bool from_ad_script) {
   auto entries = ukm_recorder.GetEntriesByName(
-      ukm::builders::AbusiveExperienceHeuristic::kEntryName);
+      ukm::builders::AbusiveExperienceHeuristic_WindowOpen::kEntryName);
   EXPECT_EQ(expected_num_entries, entries.size());
 
   // Check that the event is keyed to |main_frame_url| only if it was from the
@@ -379,7 +379,14 @@ void ExpectLatestUkmEntry(const ukm::TestUkmRecorder& ukm_recorder,
             *ukm_recorder.GetEntryMetric(
                 dc_entry, ukm::builders::DocumentCreated::kIsMainFrameName));
 
-  ukm_recorder.ExpectEntryMetric(entries.back(), metric_name, expected_value);
+  ukm_recorder.ExpectEntryMetric(
+      entries.back(),
+      ukm::builders::AbusiveExperienceHeuristic_WindowOpen::kFromAdSubframeName,
+      from_ad_subframe);
+  ukm_recorder.ExpectEntryMetric(
+      entries.back(),
+      ukm::builders::AbusiveExperienceHeuristic_WindowOpen::kFromAdScriptName,
+      from_ad_script);
 }
 
 IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, WindowOpenFromSubframe) {
@@ -402,10 +409,9 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, WindowOpenFromSubframe) {
                         hostname, "/ad_tagging/frame_factory.html?1" + suffix));
       EXPECT_TRUE(content::ExecuteScript(child, "window.open();"));
       ExpectLatestUkmEntry(ukm_recorder, ++expected_num_entries,
-                           ukm::builders::AbusiveExperienceHeuristic::
-                               kDidWindowOpenFromAdSubframeName,
                            false /* from_main_frame */, main_frame_url,
-                           ad_frame);
+                           ad_frame /* from_ad_subframe */,
+                           false /* from_ad_script */);
     }
   }
 }
@@ -418,15 +424,13 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, WindowOpenWithScriptInStack) {
 
   EXPECT_TRUE(content::ExecuteScript(main_tab, "windowOpenFromNonAdScript();"));
   ExpectLatestUkmEntry(
-      ukm_recorder, 1 /* expected_num_entries */,
-      ukm::builders::AbusiveExperienceHeuristic::kDidWindowOpenFromAdScriptName,
-      true /* from_main_frame */, main_frame_url, false /* expected_value */);
+      ukm_recorder, 1 /* expected_num_entries */, true /* from_main_frame */,
+      main_frame_url, false /* from_ad_subframe */, false /* from_ad_script */);
 
   EXPECT_TRUE(content::ExecuteScript(main_tab, "windowOpenFromAdScript();"));
-  ExpectLatestUkmEntry(
-      ukm_recorder, 2 /* expected_num_entries */,
-      ukm::builders::AbusiveExperienceHeuristic::kDidWindowOpenFromAdScriptName,
-      true /* from_main_frame */, main_frame_url, true /* expected_value */);
+  ExpectLatestUkmEntry(ukm_recorder, 2 /* expected_num_entries */,
+                       true /* from_main_frame */, main_frame_url,
+                       false /* from_ad_subframe */, true /* from_ad_script */);
 }
 
 }  // namespace

chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc

@@ -614,7 +614,7 @@ TEST_P(BlockTabUnderIncognitoTest, DisableFeature_LogsDidTabUnder) {
 }
 
 TEST_F(BlockTabUnderTest, LogsUkm) {
-  using UkmEntry = ukm::builders::AbusiveExperienceHeuristic;
+  using UkmEntry = ukm::builders::AbusiveExperienceHeuristic_TabUnder;
 
   ukm::InitializeSourceUrlRecorderForWebContents(web_contents());
   ukm::TestAutoSetUkmRecorder test_ukm_recorder;

chrome/browser/ui/blocked_content/tab_under_navigation_throttle.cc

@@ -102,7 +102,7 @@ void LogTabUnderAttempt(content::NavigationHandle* handle,
   ukm::SourceId opener_source_id =
       ukm::GetSourceIdForWebContentsDocument(handle->GetWebContents());
   if (opener_source_id != ukm::kInvalidSourceId && ukm_recorder) {
-    ukm::builders::AbusiveExperienceHeuristic(opener_source_id)
+    ukm::builders::AbusiveExperienceHeuristic_TabUnder(opener_source_id)
         .SetDidTabUnder(true)
         .Record(ukm_recorder);
   }

third_party/blink/renderer/core/page/create_window.cc

@@ -208,9 +208,9 @@ static void MaybeLogWindowOpenUKM(LocalFrame& opener_frame) {
   bool is_ad_subframe = opener_frame.IsAdSubframe();
   bool is_ad_script_in_stack = ad_tracker->IsAdScriptInStack();
   if (source_id != ukm::kInvalidSourceId) {
-    ukm::builders::AbusiveExperienceHeuristic(source_id)
-        .SetDidWindowOpenFromAdSubframe(is_ad_subframe)
-        .SetDidWindowOpenFromAdScript(is_ad_script_in_stack)
+    ukm::builders::AbusiveExperienceHeuristic_WindowOpen(source_id)
+        .SetFromAdSubframe(is_ad_subframe)
+        .SetFromAdScript(is_ad_script_in_stack)
         .Record(ukm_recorder);
   }
 }

tools/metrics/ukm/ukm.xml

@@ -22,8 +22,8 @@ be describing additional metrics about the same event.
   <owner>csharrison@chromium.org</owner>
   <owner>yaoxia@chromium.org</owner>
   <summary>
-    Various metrics recording experiences which are commonly used for abusive
-    purposes.
+    Obsolete. Various metrics recording experiences which are commonly used for
+    abusive purposes.
   </summary>
   <metric name="DidTabUnder">
     <summary>
@@ -42,6 +42,32 @@ be describing additional metrics about the same event.
   </metric>
 </event>
 
+<event name="AbusiveExperienceHeuristic.TabUnder">
+  <owner>csharrison@chromium.org</owner>
+  <metric name="DidTabUnder">
+    <summary>
+      True if the page attempted a tab-under navigation.
+    </summary>
+  </metric>
+</event>
+
+<event name="AbusiveExperienceHeuristic.WindowOpen">
+  <owner>yaoxia@chromium.org</owner>
+  <summary>
+    Recorded whenever window.open() is called when AdTagging is enabled.
+  </summary>
+  <metric name="FromAdScript">
+    <summary>
+      True if the page called window.open() with an ad script in the stack.
+    </summary>
+  </metric>
+  <metric name="FromAdSubframe">
+    <summary>
+      True if the page called window.open() from an ad subframe.
+    </summary>
+  </metric>
+</event>
+
 <event name="AbusiveExperienceHeuristic.JavaScriptDialog">
   <owner>yaoxia@chromium.org</owner>
   <metric name="DismissalCause">