Do not create TreeStateTracker if kCTLogAuditing feature is disabled

This avoids distributing STHs and SCTs to it and spending time verifying STH signatures
when they won't be used for anything.

Bug: 828665
Change-Id: I8cb6b67fcd12d9e4f875331d1eae9c44d9e74925
Reviewed-on: https://chromium-review.googlesource.com/1009913Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: Rob Percival <robpercival@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550675}

chrome/browser/io_thread.cc

@@ -42,6 +42,7 @@
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
+#include "components/certificate_transparency/features.h"
 #include "components/certificate_transparency/sth_distributor.h"
 #include "components/certificate_transparency/sth_observer.h"
 #include "components/certificate_transparency/tree_state_tracker.h"
@@ -551,15 +552,17 @@ void IOThread::Init() {
 
   UpdateDnsClientEnabled();
 
-  ct_tree_tracker_ =
-      std::make_unique<certificate_transparency::TreeStateTracker>(
-          globals_->ct_logs, globals_->system_request_context->host_resolver(),
-          net_log_);
-  // Register the ct_tree_tracker_ as observer for new STHs.
-  RegisterSTHObserver(ct_tree_tracker_.get());
-  // Register the ct_tree_tracker_ as observer for verified SCTs.
-  globals_->system_request_context->cert_transparency_verifier()->SetObserver(
-      ct_tree_tracker_.get());
+  if (base::FeatureList::IsEnabled(certificate_transparency::kCTLogAuditing)) {
+    ct_tree_tracker_ =
+        std::make_unique<certificate_transparency::TreeStateTracker>(
+            globals_->ct_logs,
+            globals_->system_request_context->host_resolver(), net_log_);
+    // Register the ct_tree_tracker_ as observer for new STHs.
+    RegisterSTHObserver(ct_tree_tracker_.get());
+    // Register the ct_tree_tracker_ as observer for verified SCTs.
+    globals_->system_request_context->cert_transparency_verifier()->SetObserver(
+        ct_tree_tracker_.get());
+  }
 }
 
 void IOThread::CleanUp() {
@@ -567,13 +570,16 @@ void IOThread::CleanUp() {
 
   system_url_request_context_getter_ = nullptr;
 
-  // Unlink the ct_tree_tracker_ from the global cert_transparency_verifier
-  // and unregister it from new STH notifications so it will take no actions
-  // on anything observed during CleanUp process.
-  globals()->system_request_context->cert_transparency_verifier()->SetObserver(
-      nullptr);
-  UnregisterSTHObserver(ct_tree_tracker_.get());
-  ct_tree_tracker_.reset();
+  if (ct_tree_tracker_) {
+    // Unlink the ct_tree_tracker_ from the global cert_transparency_verifier
+    // and unregister it from new STH notifications so it will take no actions
+    // on anything observed during CleanUp process.
+    globals()
+        ->system_request_context->cert_transparency_verifier()
+        ->SetObserver(nullptr);
+    UnregisterSTHObserver(ct_tree_tracker_.get());
+    ct_tree_tracker_.reset();
+  }
 
   globals_->system_request_context->proxy_resolution_service()->OnShutdown();
 

chrome/browser/io_thread_browsertest.cc

@@ -20,6 +20,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/certificate_transparency/features.h"
 #include "components/certificate_transparency/tree_state_tracker.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
@@ -113,12 +114,20 @@ void CheckEffectiveConnectionType(IOThread* io_thread,
 }
 
 void CheckSCTsAreSentToTreeTracker(IOThread* io_thread) {
+  EXPECT_NE(io_thread->ct_tree_tracker(), nullptr);
   EXPECT_EQ(io_thread->ct_tree_tracker(),
             io_thread->globals()
                 ->system_request_context->cert_transparency_verifier()
                 ->GetObserver());
 }
 
+void CheckSCTsAreNotSentToTreeTracker(IOThread* io_thread) {
+  EXPECT_EQ(io_thread->globals()
+                ->system_request_context->cert_transparency_verifier()
+                ->GetObserver(),
+            nullptr);
+}
+
 class IOThreadBrowserTest : public InProcessBrowserTest {
  public:
   IOThreadBrowserTest() {}
@@ -203,12 +212,48 @@ IN_PROC_BROWSER_TEST_F(IOThreadBrowserTest, UpdateDelegateWhitelist) {
                  base::Unretained(g_browser_process->io_thread()), true, url));
 }
 
-IN_PROC_BROWSER_TEST_F(IOThreadBrowserTest, SCTsAreSentToTreeTracker) {
+class IOThreadCTBrowserTest : public IOThreadBrowserTest {
+ public:
+  IOThreadCTBrowserTest() {}
+  ~IOThreadCTBrowserTest() override {}
+
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(
+        certificate_transparency::kCTLogAuditing);
+    IOThreadBrowserTest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(IOThreadCTBrowserTest, SCTsAreSentToTreeTracker) {
   RunOnIOThreadBlocking(
       base::BindOnce(&CheckSCTsAreSentToTreeTracker,
                      base::Unretained(g_browser_process->io_thread())));
 }
 
+class IOThreadNoCTBrowserTest : public IOThreadBrowserTest {
+ public:
+  IOThreadNoCTBrowserTest() {}
+  ~IOThreadNoCTBrowserTest() override {}
+
+  void SetUp() override {
+    scoped_feature_list_.InitAndDisableFeature(
+        certificate_transparency::kCTLogAuditing);
+    IOThreadBrowserTest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(IOThreadNoCTBrowserTest, SCTsAreNotSentToTreeTracker) {
+  RunOnIOThreadBlocking(
+      base::BindOnce(&CheckSCTsAreNotSentToTreeTracker,
+                     base::Unretained(g_browser_process->io_thread())));
+}
+
 class IOThreadEctCommandLineBrowserTest : public IOThreadBrowserTest {
  public:
   IOThreadEctCommandLineBrowserTest() {}

components/certificate_transparency/tree_state_tracker.cc

@@ -54,9 +54,6 @@ TreeStateTracker::~TreeStateTracker() {}
 void TreeStateTracker::OnSCTVerified(base::StringPiece hostname,
                                      X509Certificate* cert,
                                      const SignedCertificateTimestamp* sct) {
-  if (!base::FeatureList::IsEnabled(kCTLogAuditing))
-    return;
-
   auto it = tree_trackers_.find(sct->log_id);
   // Ignore if the SCT is from an unknown log.
   if (it == tree_trackers_.end())