Add network service method to clear NetworkErrorLogging data.

Minor cleanup in NetworkErrorLoggingService by making non-filtered
cleanup an explicit separate method.

Bug: 824997
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Iaa7b9fa25a7463a7294d93759dca0b0ee06187a0
Reviewed-on: https://chromium-review.googlesource.com/1044359
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557195}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -1090,6 +1090,11 @@ class MockNetworkErrorLoggingService : public net::NetworkErrorLoggingService {
     last_origin_filter_ = origin_filter;
   }
 
+  void RemoveAllBrowsingData() override {
+    ++remove_calls_;
+    last_origin_filter_ = base::RepeatingCallback<bool(const GURL&)>();
+  }
+
   int remove_calls() const { return remove_calls_; }
   const base::RepeatingCallback<bool(const GURL&)>& last_origin_filter() const {
     return last_origin_filter_;

net/network_error_logging/network_error_logging_service.cc

@@ -276,12 +276,6 @@ class NetworkErrorLoggingServiceImpl : public NetworkErrorLoggingService {
 
   void RemoveBrowsingData(const base::RepeatingCallback<bool(const GURL&)>&
                               origin_filter) override {
-    if (origin_filter.is_null()) {
-      wildcard_policies_.clear();
-      policies_.clear();
-      return;
-    }
-
     std::vector<url::Origin> origins_to_remove;
 
     for (auto it = policies_.begin(); it != policies_.end(); ++it) {
@@ -296,6 +290,11 @@ class NetworkErrorLoggingServiceImpl : public NetworkErrorLoggingService {
     }
   }
 
+  void RemoveAllBrowsingData() override {
+    wildcard_policies_.clear();
+    policies_.clear();
+  }
+
   base::Value StatusAsValue() const override {
     base::Value dict(base::Value::Type::DICTIONARY);
     std::vector<base::Value> policy_list;
@@ -321,6 +320,14 @@ class NetworkErrorLoggingServiceImpl : public NetworkErrorLoggingService {
     return dict;
   }
 
+  std::set<url::Origin> GetPolicyOriginsForTesting() override {
+    std::set<url::Origin> origins;
+    for (const auto& entry : policies_) {
+      origins.insert(entry.first);
+    }
+    return origins;
+  }
+
  private:
   // NEL Policy set by an origin.
   struct OriginPolicy {
@@ -592,6 +599,11 @@ base::Value NetworkErrorLoggingService::StatusAsValue() const {
   return base::Value();
 }
 
+std::set<url::Origin> NetworkErrorLoggingService::GetPolicyOriginsForTesting() {
+  NOTIMPLEMENTED();
+  return std::set<url::Origin>();
+}
+
 NetworkErrorLoggingService::NetworkErrorLoggingService()
     : tick_clock_(base::DefaultTickClock::GetInstance()),
       reporting_service_(nullptr) {}

net/network_error_logging/network_error_logging_service.h

@@ -109,11 +109,14 @@ class NET_EXPORT NetworkErrorLoggingService {
   virtual void OnRequest(const RequestDetails& details) = 0;
 
   // Removes browsing data (origin policies) associated with any origin for
-  // which |origin_filter| returns true, or for all origins if
-  // |origin_filter.is_null()|.
+  // which |origin_filter| returns true.
   virtual void RemoveBrowsingData(
       const base::RepeatingCallback<bool(const GURL&)>& origin_filter) = 0;
 
+  // Removes browsing data (origin policies) for all origins. Allows slight
+  // optimization over passing an always-true filter to RemoveBrowsingData.
+  virtual void RemoveAllBrowsingData() = 0;
+
   // Sets the ReportingService that will be used to queue network error reports.
   // If |nullptr| is passed, reports will be queued locally or discarded.
   // |reporting_service| must outlive the NetworkErrorLoggingService.
@@ -126,6 +129,8 @@ class NET_EXPORT NetworkErrorLoggingService {
 
   virtual base::Value StatusAsValue() const;
 
+  virtual std::set<url::Origin> GetPolicyOriginsForTesting();
+
  protected:
   NetworkErrorLoggingService();
 

net/network_error_logging/network_error_logging_service_unittest.cc

@@ -494,7 +494,7 @@ TEST_F(NetworkErrorLoggingServiceTest,
 TEST_F(NetworkErrorLoggingServiceTest, RemoveAllBrowsingData) {
   service()->OnHeader(kOrigin_, kHeader_);
 
-  service()->RemoveBrowsingData(base::RepeatingCallback<bool(const GURL&)>());
+  service()->RemoveAllBrowsingData();
 
   service()->OnRequest(MakeRequestDetails(kUrl_, ERR_CONNECTION_REFUSED));
 

net/url_request/url_request_unittest.cc

@@ -7608,6 +7608,8 @@ class TestNetworkErrorLoggingService : public NetworkErrorLoggingService {
     NOTREACHED();
   }
 
+  void RemoveAllBrowsingData() override { NOTREACHED(); }
+
  private:
   std::vector<Header> headers_;
   std::vector<RequestDetails> errors_;

services/network/network_context.cc

@@ -77,6 +77,7 @@
 
 #if BUILDFLAG(ENABLE_REPORTING)
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/network_error_logging/network_error_logging_service.h"
 #include "net/reporting/reporting_browsing_data_remover.h"
 #include "net/reporting/reporting_policy.h"
 #include "net/reporting/reporting_service.h"
@@ -762,6 +763,22 @@ void NetworkContext::ClearReportingCacheClients(
 
   std::move(callback).Run();
 }
+
+void NetworkContext::ClearNetworkErrorLogging(
+    mojom::ClearDataFilterPtr filter,
+    ClearNetworkErrorLoggingCallback callback) {
+  net::NetworkErrorLoggingService* logging_service =
+      url_request_context_->network_error_logging_service();
+  if (logging_service) {
+    if (filter) {
+      logging_service->RemoveBrowsingData(BuildUrlFilter(std::move(filter)));
+    } else {
+      logging_service->RemoveAllBrowsingData();
+    }
+  }
+
+  std::move(callback).Run();
+}
 #else   // BUILDFLAG(ENABLE_REPORTING)
 void NetworkContext::ClearReportingCacheReports(
     mojom::ClearDataFilterPtr filter,
@@ -774,6 +791,12 @@ void NetworkContext::ClearReportingCacheClients(
     ClearReportingCacheClientsCallback callback) {
   NOTREACHED();
 }
+
+void NetworkContext::ClearNetworkErrorLogging(
+    mojom::ClearDataFilterPtr filter,
+    ClearNetworkErrorLoggingCallback callback) {
+  NOTREACHED();
+}
 #endif  // BUILDFLAG(ENABLE_REPORTING)
 
 void NetworkContext::SetNetworkConditions(

services/network/network_context.h

@@ -134,6 +134,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
   void ClearReportingCacheClients(
       mojom::ClearDataFilterPtr filter,
       ClearReportingCacheClientsCallback callback) override;
+  void ClearNetworkErrorLogging(
+      mojom::ClearDataFilterPtr filter,
+      ClearNetworkErrorLoggingCallback callback) override;
   void SetNetworkConditions(const std::string& profile_id,
                             mojom::NetworkConditionsPtr conditions) override;
   void SetAcceptLanguage(const std::string& new_accept_language) override;

services/network/network_context_unittest.cc

@@ -76,6 +76,7 @@
 #include "url/url_constants.h"
 
 #if BUILDFLAG(ENABLE_REPORTING)
+#include "net/network_error_logging/network_error_logging_service.h"
 #include "net/reporting/reporting_cache.h"
 #include "net/reporting/reporting_report.h"
 #include "net/reporting/reporting_service.h"
@@ -1267,6 +1268,98 @@ TEST_F(NetworkContextTest, ClearReportingCacheClientsWithNoService) {
                                               run_loop.QuitClosure());
   run_loop.Run();
 }
+
+TEST_F(NetworkContextTest, ClearNetworkErrorLogging) {
+  base::test::ScopedFeatureList scoped_feature_list_;
+  scoped_feature_list_.InitAndEnableFeature(features::kNetworkErrorLogging);
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+
+  net::NetworkErrorLoggingService* logging_service =
+      network_context->url_request_context()->network_error_logging_service();
+  ASSERT_TRUE(logging_service);
+
+  GURL domain("https://google.com");
+  logging_service->OnHeader(url::Origin::Create(domain),
+                            "{\"report-to\":\"group\",\"max-age\":86400}");
+
+  ASSERT_EQ(1u, logging_service->GetPolicyOriginsForTesting().size());
+
+  base::RunLoop run_loop;
+  network_context->ClearNetworkErrorLogging(nullptr /* filter */,
+                                            run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_TRUE(logging_service->GetPolicyOriginsForTesting().empty());
+}
+
+TEST_F(NetworkContextTest, ClearNetworkErrorLoggingWithFilter) {
+  base::test::ScopedFeatureList scoped_feature_list_;
+  scoped_feature_list_.InitAndEnableFeature(features::kNetworkErrorLogging);
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+
+  net::NetworkErrorLoggingService* logging_service =
+      network_context->url_request_context()->network_error_logging_service();
+  ASSERT_TRUE(logging_service);
+
+  GURL domain1("https://google.com");
+  logging_service->OnHeader(url::Origin::Create(domain1),
+                            "{\"report-to\":\"group\",\"max-age\":86400}");
+  GURL domain2("https://chromium.org");
+  logging_service->OnHeader(url::Origin::Create(domain2),
+                            "{\"report-to\":\"group\",\"max-age\":86400}");
+
+  ASSERT_EQ(2u, logging_service->GetPolicyOriginsForTesting().size());
+
+  mojom::ClearDataFilterPtr filter = mojom::ClearDataFilter::New();
+  filter->type = mojom::ClearDataFilter_Type::KEEP_MATCHES;
+  filter->domains.push_back("chromium.org");
+
+  base::RunLoop run_loop;
+  network_context->ClearNetworkErrorLogging(std::move(filter),
+                                            run_loop.QuitClosure());
+  run_loop.Run();
+
+  std::set<url::Origin> policy_origins =
+      logging_service->GetPolicyOriginsForTesting();
+  EXPECT_EQ(1u, policy_origins.size());
+  EXPECT_NE(policy_origins.end(),
+            policy_origins.find(url::Origin::Create(domain2)));
+}
+
+TEST_F(NetworkContextTest, ClearEmptyNetworkErrorLogging) {
+  base::test::ScopedFeatureList scoped_feature_list_;
+  scoped_feature_list_.InitAndEnableFeature(features::kNetworkErrorLogging);
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+
+  net::NetworkErrorLoggingService* logging_service =
+      network_context->url_request_context()->network_error_logging_service();
+  ASSERT_TRUE(logging_service);
+
+  ASSERT_TRUE(logging_service->GetPolicyOriginsForTesting().empty());
+
+  base::RunLoop run_loop;
+  network_context->ClearNetworkErrorLogging(nullptr /* filter */,
+                                            run_loop.QuitClosure());
+  run_loop.Run();
+
+  EXPECT_TRUE(logging_service->GetPolicyOriginsForTesting().empty());
+}
+
+TEST_F(NetworkContextTest, ClearEmptyNetworkErrorLoggingWithNoService) {
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+
+  ASSERT_FALSE(
+      network_context->url_request_context()->network_error_logging_service());
+
+  base::RunLoop run_loop;
+  network_context->ClearNetworkErrorLogging(nullptr /* filter */,
+                                            run_loop.QuitClosure());
+  run_loop.Run();
+}
 #endif  // BUILDFLAG(ENABLE_REPORTING)
 
 void SetCookieCallback(base::RunLoop* run_loop, bool* result_out, bool result) {

services/network/public/mojom/network_service.mojom

@@ -304,6 +304,13 @@ interface NetworkContext {
   // filter.
   ClearReportingCacheClients(ClearDataFilter? filter) => ();
 
+  // Clears policy entries from the NetworkErrorLoggingService. Should not be
+  // called if the ENABLE_REPORTING build flag is false.
+  //
+  // If a non-null |filter| is specified, will clear only entries matching the
+  // filter.
+  ClearNetworkErrorLogging(ClearDataFilter? filter) => ();
+
   // Configures network conditions for the specified throttling profile.
   // The throttling will be applied only to requests that have
   // X-DevTools-Emulate-Network-Conditions-Client-Id: <profile_id>

services/network/test/test_network_context.h

@@ -59,6 +59,9 @@ class TestNetworkContext : public mojom::NetworkContext {
   void ClearReportingCacheClients(
       mojom::ClearDataFilterPtr filter,
       ClearReportingCacheClientsCallback callback) override {}
+  void ClearNetworkErrorLogging(
+      mojom::ClearDataFilterPtr filter,
+      ClearNetworkErrorLoggingCallback callback) override {}
   void SetNetworkConditions(const std::string& profile_id,
                             mojom::NetworkConditionsPtr conditions) override {}
   void SetAcceptLanguage(const std::string& new_accept_language) override {}