Move SSL config change notification after channel ID clearing to network service.

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

content/browser/browsing_data/browsing_data_remover_impl.cc

@@ -119,33 +119,6 @@ void ClearHttpAuthCacheOnIOThread(
   http_session->CloseAllConnections();
 }
 
-void OnClearedChannelIDsOnIOThread(
-    net::URLRequestContextGetter* request_context,
-    base::OnceClosure callback) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-  // Need to close open SSL connections which may be using the channel ids we
-  // are deleting.
-  // TODO(mattm): http://crbug.com/166069 Make the server bound cert
-  // service/store have observers that can notify relevant things directly.
-  // TODO(ericorth): http://crbug.com/824970 Move this over to the network
-  // service and handle within ClearChannelIds().
-  request_context->GetURLRequestContext()
-      ->ssl_config_service()
-      ->NotifySSLConfigChange();
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, std::move(callback));
-}
-
-void OnClearedChannelIDs(
-    scoped_refptr<net::URLRequestContextGetter> request_context,
-    base::OnceClosure callback) {
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::BindOnce(&OnClearedChannelIDsOnIOThread,
-                     base::RetainedRef(std::move(request_context)),
-                     std::move(callback)));
-}
-
 }  // namespace
 
 BrowsingDataRemoverImpl::BrowsingDataRemoverImpl(
@@ -356,16 +329,10 @@ void BrowsingDataRemoverImpl::RemoveImpl(
     DCHECK(service_filter->origins.empty())
         << "Origin-based deletion is not suitable for channel IDs.";
 
-    // Since we are running on the UI thread don't call GetURLRequestContext().
-    scoped_refptr<net::URLRequestContextGetter> request_context =
-        BrowserContext::GetDefaultStoragePartition(browser_context_)
-            ->GetURLRequestContext();
     BrowserContext::GetDefaultStoragePartition(browser_context_)
         ->GetNetworkContext()
-        ->ClearChannelIds(
-            delete_begin, delete_end, std::move(service_filter),
-            base::BindOnce(&OnClearedChannelIDs, std::move(request_context),
-                           CreatePendingTaskCompletionClosureForMojo()));
+        ->ClearChannelIds(delete_begin, delete_end, std::move(service_filter),
+                          CreatePendingTaskCompletionClosureForMojo());
   }
 
   //////////////////////////////////////////////////////////////////////////////

services/network/network_context.cc

@@ -109,6 +109,16 @@ bool MatchesClearChannelIdFilter(mojom::ClearDataFilter_Type filter_type,
          found_domain;
 }
 
+void OnClearedChannelIds(net::SSLConfigService* ssl_config_service,
+                         base::OnceClosure callback) {
+  // Need to close open SSL connections which may be using the channel ids we
+  // deleted.
+  // TODO(mattm): http://crbug.com/166069 Make the server bound cert
+  // service/store have observers that can notify relevant things directly.
+  ssl_config_service->NotifySSLConfigChange();
+  std::move(callback).Run();
+}
+
 }  // namespace
 
 constexpr bool NetworkContext::enable_resource_scheduler_;
@@ -555,8 +565,12 @@ void NetworkContext::ClearChannelIds(base::Time start_time,
 
   url_request_context_->channel_id_service()
       ->GetChannelIDStore()
-      ->DeleteForDomainsCreatedBetween(std::move(filter_predicate), start_time,
-                                       end_time, std::move(callback));
+      ->DeleteForDomainsCreatedBetween(
+          std::move(filter_predicate), start_time, end_time,
+          base::BindOnce(
+              &OnClearedChannelIds,
+              base::RetainedRef(url_request_context_->ssl_config_service()),
+              std::move(callback)));
 }
 
 void NetworkContext::SetNetworkConditions(

services/network/network_context_unittest.cc

@@ -78,7 +78,8 @@ mojom::NetworkContextParamsPtr CreateContextParams() {
   return params;
 }
 
-class NetworkContextTest : public testing::Test {
+class NetworkContextTest : public testing::Test,
+                           public net::SSLConfigService::Observer {
  public:
   NetworkContextTest()
       : scoped_task_environment_(
@@ -118,6 +119,8 @@ class NetworkContextTest : public testing::Test {
     return network_service_.get();
   }
 
+  void OnSSLConfigChanged() override { ++ssl_config_changed_count_; }
+
  protected:
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   std::unique_ptr<NetworkService> network_service_;
@@ -126,6 +129,7 @@ class NetworkContextTest : public testing::Test {
   // the NetworkContext. These tests are probably fine anyways, since the
   // message loop must be spun for that to happen.
   mojom::NetworkContextPtr network_context_ptr_;
+  int ssl_config_changed_count_ = 0;
 };
 
 TEST_F(NetworkContextTest, DestroyContextWithLiveRequest) {
@@ -876,6 +880,24 @@ TEST_F(NetworkContextTest, ClearChannelIdsWithTimeRange) {
               testing::UnorderedElementsAre("google.com", "gmail.com"));
 }
 
+TEST_F(NetworkContextTest, ClearChannelIdTriggersSslChangeNotification) {
+  std::unique_ptr<NetworkContext> network_context =
+      CreateContextWithParams(CreateContextParams());
+
+  network_context->url_request_context()->ssl_config_service()->AddObserver(
+      this);
+
+  ASSERT_EQ(0, ssl_config_changed_count_);
+
+  base::RunLoop run_loop;
+  network_context->ClearChannelIds(base::Time(), base::Time(),
+                                   nullptr /* filter */,
+                                   base::BindOnce(run_loop.QuitClosure()));
+  run_loop.Run();
+
+  EXPECT_EQ(1, ssl_config_changed_count_);
+}
+
 void SetCookieCallback(base::RunLoop* run_loop, bool* result_out, bool result) {
   *result_out = result;
   run_loop->Quit();