Log pref file size histogram on read rather than on write.

base/json/json_file_value_serializer.h

Index: base/json/json_file_value_serializer.h
diff --git a/base/json/json_file_value_serializer.h b/base/json/json_file_value_serializer.h
index f0f556c2dc68a23457ad2aa8ec3db7a8a209947d..69d6c7f93782b6bf7e1608c947a5e8da8198d3b8 100644
--- a/base/json/json_file_value_serializer.h
+++ b/base/json/json_file_value_serializer.h
@@ -14,13 +14,21 @@
 
 class BASE_EXPORT JSONFileValueSerializer : public base::ValueSerializer {
  public:
-  // json_file_patch is the path of a file that will be source of the
+  explicit JSONFileValueSerializer(const base::FilePath& json_file_path)
+    : JSONFileValueSerializer(json_file_path, std::string()) {}

[Alexei Svitkine (slow), 2015/01/30 13:49:03]

Nit: Indent 2 more. Maybe run git cl format?

[gab, 2015/01/30 17:33:37]

Good point, git cl formatted.

+
+  // |json_file_path_| is the path of a file that will be source of the
   // deserialization or the destination of the serialization.
   // When deserializing, the file should exist, but when serializing, the
   // serializer will attempt to create the file at the specified location.
-  explicit JSONFileValueSerializer(const base::FilePath& json_file_path)
+  // If |histogram_suffix| is non-empty, will log a
+  // "Settings.JsonDataSizeKilobytes.|histogram_suffix|" UMA histogram with the
+  // size of the content read from disk while deserializing.
+  JSONFileValueSerializer(const base::FilePath& json_file_path,
+                          const std::string& histogram_suffix)
     : json_file_path_(json_file_path),
-      allow_trailing_comma_(false) {}
+      allow_trailing_comma_(false),
+      histogram_suffix_(histogram_suffix) {}

[Alexei Svitkine (slow), 2015/01/30 13:49:03]

Nit: The ctors and dtor should probably be defined out of line.

[gab, 2015/01/30 17:33:37]

They were already inlined so I'm not sure this CL justifies changing that, I
think the reason clang allows this is that all members are POD-types (or made
only of POD types) -- i.e., all construction/destruction can be inlined.

[Alexei Svitkine (slow), 2015/01/30 17:41:16]

But they're not POD types... a std::string is backed by a buffer on the heap and
involves a string copy...

I think the motivation for the rule is that we don't want ctors/dtors that
generate a lot of machine code (e.g. in this case, the ctors for std::string can
be inlined in this one) to be in headers, as then they end up in multiple object
files and have to be de-duped by the linker, slowing the build. I think the
above still applies to this case.

Also, since you're adding a new ctor, this change is making this existing
problem worse. So I would prefer you do actually out of line them... but won't
block you on this if you really insist...

[gab, 2015/01/30 18:22:34]

Okay you convinced me, done.

 
   ~JSONFileValueSerializer() override {}
 
@@ -74,8 +82,9 @@ class BASE_EXPORT JSONFileValueSerializer : public base::ValueSerializer {
  private:
   bool SerializeInternal(const base::Value& root, bool omit_binary_values);
 
-  base::FilePath json_file_path_;
+  const base::FilePath json_file_path_;
   bool allow_trailing_comma_;
+  const std::string histogram_suffix_;
 
   // A wrapper for ReadFileToString which returns a non-zero JsonFileError if
   // there were file errors.