Adding pingback reporting for blacklisted navigations

This adds a previews type state to the pingback that represents that no
preview was served due to the blacklist acting on one of its rules. The
states are still mutually exclusive.

Bug: 849328
Change-Id: I40235d318b68e9674b67ae40df30153c30aba178
Reviewed-on: https://chromium-review.googlesource.com/1086242
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564744}

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer.cc

@@ -23,6 +23,7 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_page_load_timing.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
+#include "components/previews/core/previews_user_data.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_data.h"
 #include "content/public/browser/navigation_handle.h"
@@ -147,6 +148,13 @@ DataReductionProxyMetricsObserver::OnCommit(
   if (!data || !data->used_data_reduction_proxy())
     return STOP_OBSERVING;
   data_ = data->DeepCopy();
+
+  previews::PreviewsUserData* previews_data =
+      chrome_navigation_data->previews_user_data();
+  if (previews_data) {
+    data_->set_black_listed(previews_data->black_listed_for_lite_page());
+  }
+
   process_id_ = navigation_handle->GetWebContents()
                     ->GetMainFrame()
                     ->GetProcess()

chrome/browser/page_load_metrics/observers/data_reduction_proxy_metrics_observer_unittest.cc

@@ -31,6 +31,7 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_data.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_page_load_timing.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
+#include "components/previews/core/previews_user_data.h"
 #include "content/public/test/web_contents_tester.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/memory_instrumentation.h"
 #include "services/resource_coordinator/public/mojom/memory_instrumentation/memory_instrumentation.mojom.h"
@@ -45,17 +46,30 @@ const int kMemoryKb = 1024;
 data_reduction_proxy::DataReductionProxyData* DataForNavigationHandle(
     content::WebContents* web_contents,
     content::NavigationHandle* navigation_handle) {
-  ChromeNavigationData* chrome_navigation_data = new ChromeNavigationData();
-  content::WebContentsTester::For(web_contents)
-      ->SetNavigationData(navigation_handle,
-                          base::WrapUnique(chrome_navigation_data));
-  data_reduction_proxy::DataReductionProxyData* data =
-      new data_reduction_proxy::DataReductionProxyData();
-  chrome_navigation_data->SetDataReductionProxyData(base::WrapUnique(data));
+  auto chrome_navigation_data = std::make_unique<ChromeNavigationData>();
+
+  auto drp_data =
+      std::make_unique<data_reduction_proxy::DataReductionProxyData>();
+  data_reduction_proxy::DataReductionProxyData* data = drp_data.get();
+  chrome_navigation_data->SetDataReductionProxyData(std::move(drp_data));
 
+  content::WebContentsTester::For(web_contents)
+      ->SetNavigationData(navigation_handle, std::move(chrome_navigation_data));
   return data;
 }
 
+previews::PreviewsUserData* PreviewsDataForNavigationHandle(
+    content::NavigationHandle* navigation_handle) {
+  ChromeNavigationData* chrome_navigation_data =
+      static_cast<ChromeNavigationData*>(
+          navigation_handle->GetNavigationData());
+  auto data = std::make_unique<previews::PreviewsUserData>(1);
+  auto* data_ptr = data.get();
+  chrome_navigation_data->set_previews_user_data(std::move(data));
+
+  return data_ptr;
+}
+
 // Pingback client responsible for recording the timing information it receives
 // from a SendPingback call.
 class TestPingbackClient
@@ -113,11 +127,13 @@ class TestDataReductionProxyMetricsObserver
   TestDataReductionProxyMetricsObserver(content::WebContents* web_contents,
                                         TestPingbackClient* pingback_client,
                                         bool data_reduction_proxy_used,
-                                        bool lite_page_used)
+                                        bool lite_page_used,
+                                        bool black_listed)
       : web_contents_(web_contents),
         pingback_client_(pingback_client),
         data_reduction_proxy_used_(data_reduction_proxy_used),
-        lite_page_used_(lite_page_used) {}
+        lite_page_used_(lite_page_used),
+        black_listed_(black_listed) {}
 
   ~TestDataReductionProxyMetricsObserver() override {}
 
@@ -129,6 +145,10 @@ class TestDataReductionProxyMetricsObserver
     data->set_used_data_reduction_proxy(data_reduction_proxy_used_);
     data->set_request_url(GURL(kDefaultTestUrl));
     data->set_lite_page_received(lite_page_used_);
+
+    auto* previews_data = PreviewsDataForNavigationHandle(navigation_handle);
+    previews_data->set_black_listed_for_lite_page(black_listed_);
+
     return DataReductionProxyMetricsObserver::OnCommit(navigation_handle,
                                                        source_id);
   }
@@ -161,6 +181,7 @@ class TestDataReductionProxyMetricsObserver
   TestPingbackClient* pingback_client_;
   bool data_reduction_proxy_used_;
   bool lite_page_used_;
+  bool black_listed_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDataReductionProxyMetricsObserver);
 };
@@ -172,7 +193,8 @@ class DataReductionProxyMetricsObserverTest
       : pingback_client_(new TestPingbackClient()),
         data_reduction_proxy_used_(false),
         is_using_lite_page_(false),
-        opt_out_expected_(false) {}
+        opt_out_expected_(false),
+        black_listed_(false) {}
 
   void ResetTest() {
     page_load_metrics::InitPageLoadTimingForTest(&timing_);
@@ -196,10 +218,12 @@ class DataReductionProxyMetricsObserverTest
 
   void RunTest(bool data_reduction_proxy_used,
                bool is_using_lite_page,
-               bool opt_out_expected) {
+               bool opt_out_expected,
+               bool black_listed) {
     data_reduction_proxy_used_ = data_reduction_proxy_used;
     is_using_lite_page_ = is_using_lite_page;
     opt_out_expected_ = opt_out_expected;
+    black_listed_ = black_listed;
     NavigateAndCommit(GURL(kDefaultTestUrl));
     SimulateTimingUpdate(timing_);
     pingback_client_->Reset();
@@ -208,7 +232,8 @@ class DataReductionProxyMetricsObserverTest
   void RunTestAndNavigateToUntrackedUrl(bool data_reduction_proxy_used,
                                         bool is_using_lite_page,
                                         bool opt_out_expected) {
-    RunTest(data_reduction_proxy_used, is_using_lite_page, opt_out_expected);
+    RunTest(data_reduction_proxy_used, is_using_lite_page, opt_out_expected,
+            false);
     NavigateToUntrackedUrl();
   }
 
@@ -256,6 +281,11 @@ class DataReductionProxyMetricsObserverTest
     EXPECT_EQ(lofi_expected, pingback_client_->data().lofi_received());
   }
 
+  void ValidateBlackListInPingback(bool black_listed) {
+    EXPECT_TRUE(pingback_client_->send_pingback_called());
+    EXPECT_EQ(black_listed, pingback_client_->data().black_listed());
+  }
+
   void ValidateRendererCrash(bool renderer_crashed) {
     EXPECT_TRUE(pingback_client_->send_pingback_called());
     EXPECT_EQ(renderer_crashed,
@@ -397,7 +427,7 @@ class DataReductionProxyMetricsObserverTest
     tracker->AddObserver(
         std::make_unique<TestDataReductionProxyMetricsObserver>(
             web_contents(), pingback_client_.get(), data_reduction_proxy_used_,
-            is_using_lite_page_));
+            is_using_lite_page_, black_listed_));
   }
 
   std::unique_ptr<TestPingbackClient> pingback_client_;
@@ -407,6 +437,7 @@ class DataReductionProxyMetricsObserverTest
   bool data_reduction_proxy_used_;
   bool is_using_lite_page_;
   bool opt_out_expected_;
+  bool black_listed_;
 
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyMetricsObserverTest);
 };
@@ -414,7 +445,7 @@ class DataReductionProxyMetricsObserverTest
 TEST_F(DataReductionProxyMetricsObserverTest, DataReductionProxyOff) {
   ResetTest();
   // Verify that when the data reduction proxy was not used, no UMA is reported.
-  RunTest(false, false, false);
+  RunTest(false, false, false, false);
   ValidateHistograms();
 }
 
@@ -422,7 +453,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, DataReductionProxyOn) {
   ResetTest();
   // Verify that when the data reduction proxy was used, but lite page was not
   // used, the correpsonding UMA is reported.
-  RunTest(true, false, false);
+  RunTest(true, false, false, false);
   ValidateHistograms();
 }
 
@@ -430,7 +461,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, LitePageEnabled) {
   ResetTest();
   // Verify that when the data reduction proxy was used and lite page was used,
   // both histograms are reported.
-  RunTest(true, true, false);
+  RunTest(true, true, false, false);
   ValidateHistograms();
 }
 
@@ -473,7 +504,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, OnCompletePingback) {
   ResetTest();
   // Verify that when an opt out occurs, that it is reported in the pingback.
   timing_.document_timing->load_event_start = base::nullopt;
-  RunTest(true, true, true);
+  RunTest(true, true, true, false);
   observer()->BroadcastEventToObservers(
       PreviewsInfoBarDelegate::OptOutEventKey());
   NavigateToUntrackedUrl();
@@ -500,11 +531,17 @@ TEST_F(DataReductionProxyMetricsObserverTest, OnCompletePingback) {
       0,
       {} /* load_timing_info */};
 
-  RunTest(true, false, false);
+  RunTest(true, false, false, false);
   SimulateLoadedResource(resource);
   NavigateToUntrackedUrl();
   ValidateTimes();
   ValidateLoFiInPingback(true);
+  ValidateBlackListInPingback(false);
+
+  ResetTest();
+  RunTest(true, false, false, true);
+  NavigateToUntrackedUrl();
+  ValidateBlackListInPingback(true);
 
   ResetTest();
   // Verify that when data reduction proxy was not used, SendPingback is not
@@ -524,7 +561,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, OnCompletePingback) {
 TEST_F(DataReductionProxyMetricsObserverTest, ByteInformationCompression) {
   ResetTest();
 
-  RunTest(true, false, false);
+  RunTest(true, false, false, false);
 
   std::unique_ptr<DataReductionProxyData> data =
       std::make_unique<DataReductionProxyData>();
@@ -615,7 +652,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, ByteInformationCompression) {
 TEST_F(DataReductionProxyMetricsObserverTest, ByteInformationInflation) {
   ResetTest();
 
-  RunTest(true, false, false);
+  RunTest(true, false, false, false);
 
   std::unique_ptr<DataReductionProxyData> data =
       std::make_unique<DataReductionProxyData>();
@@ -711,7 +748,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, ByteInformationInflation) {
 
 TEST_F(DataReductionProxyMetricsObserverTest, ProcessIdSentOnRendererCrash) {
   ResetTest();
-  RunTest(true, false, false);
+  RunTest(true, false, false, false);
   std::unique_ptr<DataReductionProxyData> data =
       std::make_unique<DataReductionProxyData>();
   data->set_used_data_reduction_proxy(true);
@@ -722,7 +759,7 @@ TEST_F(DataReductionProxyMetricsObserverTest, ProcessIdSentOnRendererCrash) {
   ValidateRendererCrash(true);
 
   ResetTest();
-  RunTest(true, false, false);
+  RunTest(true, false, false, false);
   data = std::make_unique<DataReductionProxyData>();
   data->set_used_data_reduction_proxy(true);
   data->set_request_url(GURL(kDefaultTestUrl));

components/data_reduction_proxy/content/browser/data_reduction_proxy_pingback_client_impl.cc

@@ -149,6 +149,9 @@ void AddDataToPageloadMetrics(const DataReductionProxyData& request_data,
   } else if (request_data.lite_page_received()) {
     request->set_previews_type(PageloadMetrics_PreviewsType_LITE_PAGE);
     was_preview_shown = true;
+  } else if (request_data.black_listed()) {
+    request->set_previews_type(
+        PageloadMetrics_PreviewsType_CLIENT_BLACKLIST_PREVENTED_PREVIEW);
   } else {
     request->set_previews_type(PageloadMetrics_PreviewsType_NONE);
   }

components/data_reduction_proxy/core/browser/data_reduction_proxy_data.cc

@@ -19,6 +19,7 @@ DataReductionProxyData::DataReductionProxyData()
       lite_page_received_(false),
       lofi_policy_received_(false),
       lofi_received_(false),
+      black_listed_(false),
       effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
       connection_type_(net::NetworkChangeNotifier::CONNECTION_UNKNOWN) {}
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_data.h

@@ -111,6 +111,10 @@ class DataReductionProxyData : public base::SupportsUserData::Data {
   const base::Optional<uint64_t>& page_id() const { return page_id_; }
   void set_page_id(uint64_t page_id) { page_id_ = page_id; }
 
+  // Whether the blacklist prevented a preview.
+  bool black_listed() const { return black_listed_; }
+  void set_black_listed(bool black_listed) { black_listed_ = black_listed; }
+
   // Removes |this| from |request|.
   static void ClearData(net::URLRequest* request);
 
@@ -153,6 +157,9 @@ class DataReductionProxyData : public base::SupportsUserData::Data {
   // Whether a lite page response was seen for the request or navigation.
   bool lofi_received_;
 
+  // Whether the blacklist prevented a preview.
+  bool black_listed_;
+
   // The session key used for this request or navigation.
   std::string session_key_;
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_data_unittest.cc

@@ -56,6 +56,12 @@ TEST_F(DataReductionProxyDataTest, BasicSettersAndGetters) {
   data->set_lofi_received(false);
   EXPECT_FALSE(data->lofi_received());
 
+  EXPECT_FALSE(data->black_listed());
+  data->set_black_listed(true);
+  EXPECT_TRUE(data->black_listed());
+  data->set_black_listed(false);
+  EXPECT_FALSE(data->black_listed());
+
   EXPECT_EQ(std::string(), data->session_key());
   std::string session_key = "test-key";
   data->set_session_key(session_key);
@@ -122,6 +128,7 @@ TEST_F(DataReductionProxyDataTest, DeepCopy) {
     data->set_lofi_requested(tests[i].lofi_test_value);
     data->set_lite_page_received(tests[i].lofi_test_value);
     data->set_lofi_received(tests[i].lofi_test_value);
+    data->set_black_listed(tests[i].lofi_test_value);
     data->set_session_key(kSessionKey);
     data->set_request_url(kTestURL);
     data->set_effective_connection_type(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
@@ -130,6 +137,7 @@ TEST_F(DataReductionProxyDataTest, DeepCopy) {
     EXPECT_EQ(tests[i].lofi_test_value, copy->lofi_requested());
     EXPECT_EQ(tests[i].lofi_test_value, copy->lite_page_received());
     EXPECT_EQ(tests[i].lofi_test_value, copy->lofi_received());
+    EXPECT_EQ(tests[i].lofi_test_value, copy->black_listed());
     EXPECT_EQ(tests[i].data_reduction_used, copy->used_data_reduction_proxy());
     EXPECT_EQ(kSessionKey, copy->session_key());
     EXPECT_EQ(kTestURL, copy->request_url());

components/data_reduction_proxy/proto/pageload_metrics.proto

@@ -69,12 +69,17 @@ message PageloadMetrics {
 
   // The various server previews that can be shown.
   enum PreviewsType {
-    // No server preview was applied.
+    // No server preview was applied, but the URL/navigation was not
+    // blacklisted.
     NONE = 0;
     // Image placeholders were used on the page.
     LOFI = 1;
     // The main resource was a lite page.
     LITE_PAGE = 2;
+    // Blacklisting rules caused no server preview to be requested. It's
+    // possible in the future that client previews might be shown when server
+    // previews are blacklisted, currently this is not possible.
+    CLIENT_BLACKLIST_PREVENTED_PREVIEW = 3;
   }
 
   // What type of crash occured on the page.

components/previews/content/previews_io_data.cc

@@ -245,6 +245,10 @@ bool PreviewsIOData::ShouldAllowPreviewAtECT(
     PreviewsEligibilityReason status = previews_black_list_->IsLoadedAndAllowed(
         request.url(), type, &passed_reasons);
     if (status != PreviewsEligibilityReason::ALLOWED) {
+      if (type == PreviewsType::LITE_PAGE) {
+        PreviewsUserData::GetData(request)->set_black_listed_for_lite_page(
+            true);
+      }
       LogPreviewDecisionMade(status, request.url(), base::Time::Now(), type,
                              std::move(passed_reasons), page_id);
       return false;
@@ -346,6 +350,10 @@ bool PreviewsIOData::IsURLAllowedForPreview(const net::URLRequest& request,
     PreviewsEligibilityReason status = previews_black_list_->IsLoadedAndAllowed(
         request.url(), type, &passed_reasons);
     if (status != PreviewsEligibilityReason::ALLOWED) {
+      if (type == PreviewsType::LITE_PAGE) {
+        PreviewsUserData::GetData(request)->set_black_listed_for_lite_page(
+            true);
+      }
       LogPreviewDecisionMade(status, request.url(), base::Time::Now(), type,
                              std::move(passed_reasons),
                              PreviewsUserData::GetData(request)->page_id());

components/previews/content/previews_io_data_unittest.cc

@@ -480,6 +480,25 @@ TEST_F(PreviewsIODataTest, TestDisallowPreviewBecauseOfBlackListState) {
   variations::testing::ClearAllVariationParams();
 }
 
+TEST_F(PreviewsIODataTest, TestSetBlacklistBoolDueToBlackListState) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kPreviews);
+  std::unique_ptr<net::URLRequest> request = CreateRequest();
+  base::HistogramTester histogram_tester;
+  InitializeUIServiceWithoutWaitingForBlackList();
+  base::RunLoop().RunUntilIdle();
+  io_data()->AddPreviewNavigation(GURL(request->url()), true,
+                                  PreviewsType::LITE_PAGE, 1);
+
+  auto* data =
+      PreviewsUserData::Create(request.get(), 54321 /* page_id, not used */);
+  EXPECT_FALSE(data->black_listed_for_lite_page());
+  EXPECT_FALSE(io_data()->ShouldAllowPreviewAtECT(
+      *request, PreviewsType::LITE_PAGE, net::EFFECTIVE_CONNECTION_TYPE_2G,
+      {}));
+  EXPECT_TRUE(data->black_listed_for_lite_page());
+}
+
 TEST_F(PreviewsIODataTest, TestDisallowOfflineWhenNetworkQualityUnavailable) {
   base::test::ScopedFeatureList scoped_feature_list;
   scoped_feature_list.InitAndEnableFeature(features::kPreviews);

components/previews/core/previews_user_data.h

@@ -50,6 +50,15 @@ class PreviewsUserData : public base::SupportsUserData::Data {
     return data_savings_inflation_percent_;
   }
 
+  // Whether a lite page preview was prevented from being shown due to the
+  // blacklist.
+  bool black_listed_for_lite_page() const {
+    return black_listed_for_lite_page_;
+  }
+  void set_black_listed_for_lite_page(bool black_listed_for_lite_page) {
+    black_listed_for_lite_page_ = black_listed_for_lite_page;
+  }
+
   // Sets that the page load received the Cache-Control:no-transform
   // directive. Expected to be set upon receiving a committed response.
   void SetCacheControlNoTransformDirective() {
@@ -84,6 +93,10 @@ class PreviewsUserData : public base::SupportsUserData::Data {
   // Whether the origin provided a no-transform directive.
   bool cache_control_no_transform_directive_ = false;
 
+  // Whether a lite page preview was prevented from being shown due to the
+  // blacklist.
+  bool black_listed_for_lite_page_ = false;
+
   // The committed previews type, if any.
   previews::PreviewsType committed_previews_type_ = PreviewsType::NONE;