Certificate Transparency: New component for obtaining fresh STHs.

chrome/browser/component_updater/sth_set_component_installer.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COMPONENT_UPDATER_STH_SET_COMPONENT_INSTALLER_H_
+#define CHROME_BROWSER_COMPONENT_UPDATER_STH_SET_COMPONENT_INSTALLER_H_
+
+#include <stdint.h>

[Ryan Sleevi, 2016/04/06 18:32:51]

STYLE: Newline between 8/9 -
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

[Eran Messeri, 2016/04/07 11:38:03]

Done.

+#include <string>
+#include <vector>
+
+#include "base/files/file_path.h"

[Ryan Sleevi, 2016/04/06 18:32:51]

Forward declarable

[Eran Messeri, 2016/04/07 11:38:03]

Done.

+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/values.h"

[Ryan Sleevi, 2016/04/06 18:32:51]

Don't include this if you're forward declaring on 20-23 (which you should be
able to, based on usage)

[Eran Messeri, 2016/04/07 11:38:03]

Done.

+#include "components/component_updater/default_component_installer.h"
+#include "net/cert/sth_observer.h"

[Ryan Sleevi, 2016/04/06 18:32:51]

Forward declarable

[Eran Messeri, 2016/04/07 11:38:03]

Done.

+
+namespace base {
+class FilePath;
+class Value;
+}  // namespace base
+
+namespace component_updater {
+
+class ComponentUpdateService;
+
+// Component for receiving Signed Tree Heads updates for Certificate
+// Transparency logs recognized in Chrome.
+// The STHs are in JSON format.
+// To identify the log each STH belongs to, the name of the file is
+// hex-encoded Log ID of the log that produced this STH.
+//
+// Unlike other components, once STHs are observed they are not placed
+// in some global object. Instead, a proxy class provided in the constructor

[Ryan Sleevi, 2016/04/06 18:32:51]

I would suggest this last sentence is a bit too vague a commitment/explanation.

"Instead, notifications of each of the new STHs are sent to the
net::ct::STHObserver, so that it can take appropriate steps, including possible
persistence."

[Eran Messeri, 2016/04/07 11:38:03]

Done.

+// sends notifications for each of the new STHs.
+class STHSetComponentInstallerTraits : public ComponentInstallerTraits {
+ public:
+  // The |sth_distributor| will be notified each time a new STH is observed.
+  explicit STHSetComponentInstallerTraits(
+      scoped_ptr<net::ct::STHObserver> sth_observer);
+  ~STHSetComponentInstallerTraits() override;
+
+ private:
+  friend class STHSetComponentInstallerTest;
+
+  // The following methods override ComponentInstallerTraits.
+  bool CanAutoUpdate() const override;
+  bool RequiresNetworkEncryption() const override;
+  bool OnCustomInstall(const base::DictionaryValue& manifest,
+                       const base::FilePath& install_dir) override;
+  bool VerifyInstallation(const base::DictionaryValue& manifest,
+                          const base::FilePath& install_dir) const override;
+  void ComponentReady(const base::Version& version,
+                      const base::FilePath& install_dir,
+                      scoped_ptr<base::DictionaryValue> manifest) override;
+  base::FilePath GetBaseDirectory() const override;
+  void GetHash(std::vector<uint8_t>* hash) const override;
+  std::string GetName() const override;
+
+  static base::FilePath GetInstalledPath(const base::FilePath& base);
+
+  // Reads and parses the on-disk json.
+  void LoadSTHsFromDisk(const base::FilePath& sths_file_path,
+                        const base::Version& version);
+
+  // Handle successful parsing of JSON by distributing the new STH.
+  void OnJsonParseSuccess(std::string log_id,
+                          scoped_ptr<base::Value> parsed_json);
+
+  // STH parsing failed - do nothing.
+  void OnJsonParseError(std::string log_id, const std::string& error);
+
+  scoped_ptr<net::ct::STHObserver> sth_observer_;
+
+  FRIEND_TEST_ALL_PREFIXES(STHSetComponentInstallerTest, CanLoadAllSTHs);
+  FRIEND_TEST_ALL_PREFIXES(STHSetComponentInstallerTest, VerifyInstallation);
+  FRIEND_TEST_ALL_PREFIXES(STHSetComponentInstallerTest, LoadSTHsFromDisk);

[Ryan Sleevi, 2016/04/06 18:32:51]

Could you place this at the start of the "private" section? We try to group such
friends together, in part, because this is a forward declaration (in effect),
and we group those at the start, per
https://google.github.io/styleguide/cppguide.html#Declaration_Order

This would then raise the question about "Why do you friend the class, but also
the specific tests" - generally, we try for one or the other (e.g. if
STHSetComponentInstallerTest is friend'd, then it can provide the access to the
tests as needed)

[Eran Messeri, 2016/04/07 11:38:03]

Done - moved that and removed the 'friend class STHSetComponentInstallerTest'
declaration.

+
+  DISALLOW_COPY_AND_ASSIGN(STHSetComponentInstallerTraits);
+};
+
+void RegisterSTHSetComponent(ComponentUpdateService* cus,
+                             const base::FilePath& user_data_dir,
+                             scoped_ptr<net::ct::STHObserver> sth_observer);
+
+}  // namespace component_updater
+
+#endif  // CHROME_BROWSER_COMPONENT_UPDATER_STH_SET_COMPONENT_INSTALLER_H_