Window disposition tracking for popups in UKM.

This adds a new boolean to the Popup.Closed UKM: WindowOpenDisposition.
This records the disposition used to create the popup, recorded
at the time the pop-up tracker is created.

This will be used to better understand how pop-ups are opened and
perceived by users in order to identify more abusive pop-up
experiences and improve the pop up blocker.

BUG=1044601

Change-Id: Ie32bff8f5753eb4210054569058e6df75d5f5258
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2075138
Commit-Queue: Justin Miron <justinmiron@google.com>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748252}

chrome/browser/android/tab_web_contents_delegate_android.cc

@@ -402,7 +402,7 @@ void TabWebContentsDelegateAndroid::AddNewContents(
   // At this point the |new_contents| is beyond the popup blocker, but we use
   // the same logic for determining if the popup tracker needs to be attached.
   if (source && ConsiderForPopupBlocking(disposition))
-    PopupTracker::CreateForWebContents(new_contents.get(), source);
+    PopupTracker::CreateForWebContents(new_contents.get(), source, disposition);
 
   TabHelpers::AttachTabHelpers(new_contents.get());
 

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

@@ -141,7 +141,8 @@ void PopupBlockerTabHelper::ShowBlockedPopup(
 #endif
   if (popup->params.navigated_or_inserted_contents) {
     auto* tracker = PopupTracker::CreateForWebContents(
-        popup->params.navigated_or_inserted_contents, web_contents());
+        popup->params.navigated_or_inserted_contents, web_contents(),
+        popup->params.disposition);
     tracker->set_is_trusted(true);
 
     if (popup->params.disposition == WindowOpenDisposition::NEW_POPUP) {

chrome/browser/ui/blocked_content/popup_opener_tab_helper_unittest.cc

@@ -35,6 +35,7 @@
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_source.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/window_open_disposition.h"
 #include "url/gurl.h"
 
 #if defined(OS_ANDROID)
@@ -96,7 +97,8 @@ class PopupOpenerTabHelperTest : public ChromeRenderViewHostTestHarness {
     content::WebContents* raw_popup = popup.get();
     popups_.push_back(std::move(popup));
 
-    PopupTracker::CreateForWebContents(raw_popup, web_contents() /* opener */);
+    PopupTracker::CreateForWebContents(raw_popup, web_contents() /* opener */,
+                                       WindowOpenDisposition::NEW_POPUP);
     web_contents()->WasHidden();
     raw_popup->WasShown();
     return raw_popup;

chrome/browser/ui/blocked_content/popup_tracker.cc

@@ -16,13 +16,15 @@
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
 
-PopupTracker* PopupTracker::CreateForWebContents(content::WebContents* contents,
-                                                 content::WebContents* opener) {
+PopupTracker* PopupTracker::CreateForWebContents(
+    content::WebContents* contents,
+    content::WebContents* opener,
+    WindowOpenDisposition disposition) {
   DCHECK(contents);
   DCHECK(opener);
   auto* tracker = FromWebContents(contents);
   if (!tracker) {
-    tracker = new PopupTracker(contents, opener);
+    tracker = new PopupTracker(contents, opener, disposition);
     contents->SetUserData(UserDataKey(), base::WrapUnique(tracker));
   }
   return tracker;
@@ -31,13 +33,15 @@ PopupTracker* PopupTracker::CreateForWebContents(content::WebContents* contents,
 PopupTracker::~PopupTracker() = default;
 
 PopupTracker::PopupTracker(content::WebContents* contents,
-                           content::WebContents* opener)
+                           content::WebContents* opener,
+                           WindowOpenDisposition disposition)
     : content::WebContentsObserver(contents),
       scoped_observer_(this),
       visibility_tracker_(
           base::DefaultTickClock::GetInstance(),
           contents->GetVisibility() != content::Visibility::HIDDEN),
-      opener_source_id_(ukm::GetSourceIdForWebContentsDocument(opener)) {
+      opener_source_id_(ukm::GetSourceIdForWebContentsDocument(opener)),
+      window_open_disposition_(disposition) {
   if (auto* popup_opener = PopupOpenerTabHelper::FromWebContents(opener))
     popup_opener->OnOpenedPopup(this);
 
@@ -83,6 +87,7 @@ void PopupTracker::WebContentsDestroyed() {
         .SetTrusted(is_trusted_)
         .SetNumInteractions(capped_interactions)
         .SetSafeBrowsingStatus(static_cast<int>(safe_browsing_status_))
+        .SetWindowOpenDisposition(static_cast<int>(window_open_disposition_))
         .Record(ukm::UkmRecorder::Get());
   }
 }

chrome/browser/ui/blocked_content/popup_tracker.h

@@ -15,6 +15,7 @@
 #include "content/public/browser/web_contents_user_data.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
 #include "ui/base/scoped_visibility_tracker.h"
+#include "ui/base/window_open_disposition.h"
 
 namespace content {
 class WebContents;
@@ -38,7 +39,8 @@ class PopupTracker : public content::WebContentsObserver,
   };
 
   static PopupTracker* CreateForWebContents(content::WebContents* contents,
-                                            content::WebContents* opener);
+                                            content::WebContents* opener,
+                                            WindowOpenDisposition disposition);
   ~PopupTracker() override;
 
   void set_is_trusted(bool is_trusted) { is_trusted_ = is_trusted; }
@@ -46,7 +48,9 @@ class PopupTracker : public content::WebContentsObserver,
  private:
   friend class content::WebContentsUserData<PopupTracker>;
 
-  PopupTracker(content::WebContents* contents, content::WebContents* opener);
+  PopupTracker(content::WebContents* contents,
+               content::WebContents* opener,
+               WindowOpenDisposition disposition);
 
   // content::WebContentsObserver:
   void WebContentsDestroyed() override;
@@ -89,6 +93,9 @@ class PopupTracker : public content::WebContentsObserver,
   PopupSafeBrowsingStatus safe_browsing_status_ =
       PopupSafeBrowsingStatus::kNoValue;
 
+  // The window open disposition used when creating the popup.
+  const WindowOpenDisposition window_open_disposition_;
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 
   DISALLOW_COPY_AND_ASSIGN(PopupTracker);

chrome/browser/ui/blocked_content/popup_tracker_browsertest.cc

@@ -50,6 +50,7 @@ const char kUkmUserInitiatedClose[] = "UserInitiatedClose";
 const char kUkmTrusted[] = "Trusted";
 const char kUkmNumInteractions[] = "NumInteractions";
 const char kUkmSafeBrowsingStatus[] = "SafeBrowsingStatus";
+const char kUkmWindowOpenDisposition[] = "WindowOpenDisposition";
 }  // namespace
 
 using UkmEntry = ukm::builders::Popup_Closed;
@@ -476,3 +477,66 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingPopupTrackerBrowserTest,
         static_cast<int>(PopupTracker::PopupSafeBrowsingStatus::kUnsafe));
   }
 }
+
+IN_PROC_BROWSER_TEST_F(PopupTrackerBrowserTest, PopupInTab_IsWindowFalse) {
+  const GURL first_url = embedded_test_server()->GetURL("/title1.html");
+  ui_test_utils::NavigateToURL(browser(), first_url);
+
+  content::TestNavigationObserver navigation_observer(nullptr, 1);
+  navigation_observer.StartWatchingNewWebContents();
+
+  EXPECT_TRUE(
+      content::ExecJs(browser()->tab_strip_model()->GetActiveWebContents(),
+                      "window.open('/title1.html')"));
+  navigation_observer.Wait();
+
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  content::WebContents* popup =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(PopupTracker::FromWebContents(popup));
+
+  // Close the popup and check metric.
+  int active_index = browser()->tab_strip_model()->active_index();
+  content::WebContentsDestroyedWatcher destroyed_watcher(popup);
+  browser()->tab_strip_model()->CloseWebContentsAt(
+      active_index, TabStripModel::CLOSE_USER_GESTURE);
+  destroyed_watcher.Wait();
+
+  auto* entry = ExpectAndGetEntry(first_url);
+  test_ukm_recorder_->ExpectEntryMetric(
+      entry, kUkmWindowOpenDisposition,
+      static_cast<int>(WindowOpenDisposition::NEW_FOREGROUND_TAB));
+}
+
+IN_PROC_BROWSER_TEST_F(PopupTrackerBrowserTest, PopupInWindow_IsWindowTrue) {
+  const GURL first_url = embedded_test_server()->GetURL("/title1.html");
+  ui_test_utils::NavigateToURL(browser(), first_url);
+
+  content::TestNavigationObserver navigation_observer(nullptr, 1);
+  navigation_observer.StartWatchingNewWebContents();
+  EXPECT_TRUE(content::ExecJs(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "window.open('/title1.html', 'new_window', "
+      "'location=yes,height=570,width=520,scrollbars=yes,status=yes')"));
+  navigation_observer.Wait();
+  EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
+
+  Browser* created_browser = chrome::FindLastActive();
+
+  EXPECT_EQ(1, created_browser->tab_strip_model()->count());
+  content::WebContents* popup =
+      created_browser->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(PopupTracker::FromWebContents(popup));
+
+  // Close the popup and check metric.
+  int active_index = created_browser->tab_strip_model()->active_index();
+  content::WebContentsDestroyedWatcher destroyed_watcher(popup);
+  created_browser->tab_strip_model()->CloseWebContentsAt(
+      active_index, TabStripModel::CLOSE_USER_GESTURE);
+  destroyed_watcher.Wait();
+
+  auto* entry = ExpectAndGetEntry(first_url);
+  test_ukm_recorder_->ExpectEntryMetric(
+      entry, kUkmWindowOpenDisposition,
+      static_cast<int>(WindowOpenDisposition::NEW_POPUP));
+}

chrome/browser/ui/browser.cc

@@ -1663,7 +1663,7 @@ WebContents* Browser::OpenURLFromTab(WebContents* source,
 
   if (is_popup && nav_params.navigated_or_inserted_contents) {
     auto* tracker = PopupTracker::CreateForWebContents(
-        nav_params.navigated_or_inserted_contents, source);
+        nav_params.navigated_or_inserted_contents, source, params.disposition);
     tracker->set_is_trusted(params.triggering_event_info !=
                             blink::TriggeringEventInfo::kFromUntrustedEvent);
   }
@@ -1725,7 +1725,7 @@ void Browser::AddNewContents(WebContents* source,
   // At this point the |new_contents| is beyond the popup blocker, but we use
   // the same logic for determining if the popup tracker needs to be attached.
   if (source && ConsiderForPopupBlocking(disposition))
-    PopupTracker::CreateForWebContents(new_contents.get(), source);
+    PopupTracker::CreateForWebContents(new_contents.get(), source, disposition);
 
   chrome::AddWebContents(this, source, std::move(new_contents), disposition,
                          initial_rect);

tools/metrics/ukm/ukm.xml

@@ -8375,6 +8375,12 @@ be describing additional metrics about the same event.
       Boolean value to represent whether the popup was closed by user gesture.
     </summary>
   </metric>
+  <metric name="WindowOpenDisposition" enum="WindowOpenDisposition">
+    <summary>
+      The UI window open disposition used when creating the web contents of the
+      pop-up. Set by the browser process when the pop up tracker is created.
+    </summary>
+  </metric>
 </event>
 
 <event name="Popup.Page">