De-dup PrefixSet code in SafeBrowsingDatabaseManager.

tools/metrics/histograms/histograms.xml

Index: tools/metrics/histograms/histograms.xml
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 024435dd002a22cd42b56d8971bde44cf4e2c1c9..c0336cb2ee2ad91692d0da8fa252b702813aae90 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -29765,12 +29765,10 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 </histogram>
 
 <histogram name="SB2.DatabaseKilobytes" units="KB">
-  <obsolete>
-    Replaced by SB2.BrowseDatabaseKilobytes.

[Mark P, 2015/01/02 19:56:20]

This doesn't seem right to me.  You're taking a histogram name that had
previously been used and then deprecated (marked as obsolete), then using it
again for something else.  I would encourage you to use an unused name (even if
you only plan to emit histograms with suffixes to this).

[gab, 2015/01/05 18:24:37]

Done, introduced SB2.DatabaseSizeKilobytes for this.

-  </obsolete>
-  <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
+  <owner>gab@chromium.org</owner>
   <summary>
-    The size of the SafeBrowsing database file on disk in kilobytes.
+    The size of one of the browsing SafeBrowsing database file on disk in

[Mark P, 2015/01/02 19:56:20]

nit: "browsing SafeBrowsing" seems redundant to me.

[gab, 2015/01/05 18:24:37]

Oops, definitely a typo.

+    kilobytes, after an update has occurred.

[Mark P, 2015/01/02 19:56:20]

(under the you-touched-it-you-must-improve-it rule)
Be more precise about when this histogram is emitted.

Ditto all the (non-deprecated) histograms below that you touch in this
changelist.
(Though I'm especially concerned here because I don't know exactly what this
caveat "after an update has occurred" means in turns of restriction what times
it would otherwise be omitted.)

[gab, 2015/01/05 18:24:37]

Added more details about when an update occurs.

As for the histograms below I was mostly fixing the fact that they now are
reported for many files rather than a single one. I do not think this calls for
digging into and adding more details to each of these histograms.

   </summary>
 </histogram>
 
@@ -30216,12 +30214,12 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 
 <histogram name="SB2.PrefixSetKilobytes" units="KB">
   <owner>shess@chromium.org</owner>
-  <summary>The size of the PrefixSet file in kilobytes.</summary>
+  <summary>The size of one of the PrefixSet files in kilobytes.</summary>
 </histogram>
 
 <histogram name="SB2.PrefixSetLoad" units="ms">
   <owner>shess@chromium.org</owner>
-  <summary>Time to load the PrefixSet file.</summary>
+  <summary>Time to load one of the PrefixSet files.</summary>
 </histogram>
 
 <histogram name="SB2.PrefixSetRestoredExcess">
@@ -30289,12 +30287,12 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 
 <histogram name="SB2.PrefixSetVersionRead">
   <owner>shess@chromium.org</owner>
-  <summary>Version read from the PrefixSet file.</summary>
+  <summary>Version read from one of the PrefixSet files.</summary>
 </histogram>
 
 <histogram name="SB2.PrefixSetWrite" units="ms">
   <owner>shess@chromium.org</owner>
-  <summary>Time to store the PrefixSet file.</summary>
+  <summary>Time to store one of the PrefixSet files.</summary>
 </histogram>
 
 <histogram name="SB2.ReportingIsEnabled" enum="BooleanEnabled">
@@ -30328,7 +30326,23 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="SB2.SideEffectFreePrefixSetWrite" units="ms">
+  <obsolete>
+    Deprecated 12/2014. Merged into SB2.PrefixSetWrite.
+  </obsolete>
+  <owner>shess@chromium.org</owner>
+  <summary>
+    Time to store the Side Effect Free Whitelist PrefixSet file. Note: this
+    histogram was intended to be stored as
+    SB2.SideEffectFreeWhitelistPrefixSetWrite but was actually reported as
+    SB2.SideEffectFreePrefixSetWrite from its inception to its deprecation...
+  </summary>
+</histogram>
+
 <histogram name="SB2.SideEffectFreeWhitelistDatabaseKilobytes" units="KB">
+  <obsolete>
+    Deprecated 12/2014. Merged into SB2.DatabaseKilobytes.
+  </obsolete>
   <owner>shess@chromium.org</owner>
   <summary>
     The size of the Side Effect Free Whitelist SaafeBrowsing database file on
@@ -30337,6 +30351,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 </histogram>
 
 <histogram name="SB2.SideEffectFreeWhitelistPrefixSetKilobytes" units="KB">
+  <obsolete>
+    Deprecated 12/2014. Merged into SB2.PrefixSetKilobytes.
+  </obsolete>
   <owner>shess@chromium.org</owner>
   <summary>
     The size of the Side Effect Free Whitelist PrefixSet file in kilobytes,
@@ -30345,17 +30362,13 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 </histogram>
 
 <histogram name="SB2.SideEffectFreeWhitelistPrefixSetLoad" units="ms">
+  <obsolete>
+    Deprecated 12/2014. Merged into SB2.PrefixSetLoad.
+  </obsolete>
   <owner>shess@chromium.org</owner>
   <summary>Time to load the Side Effect Free Whitelist PrefixSet file.</summary>
 </histogram>
 
-<histogram name="SB2.SideEffectFreeWhitelistPrefixSetWrite" units="ms">
-  <owner>shess@chromium.org</owner>
-  <summary>
-    Time to store the Side Effect Free Whitelist PrefixSet file.
-  </summary>
-</histogram>

[Ilya Sherman, 2015/01/06 00:21:17]

Please mark this obsolete rather than removing it entirely.

[gab, 2015/01/06 13:37:30]

It was never ever reported, instead the code was reporting
"SB2.SideEffectFreePrefixSetWrite" which I added (as obsolete) above.

Is that fine? Or does some histograms infra depend on this histogram being
marked as obsolete despite having never received any reports for it?

[Ilya Sherman, 2015/01/06 22:49:02]

I see -- in that case, there is indeed no reason to mark it as <obsolete>. 
Thanks for the clarification.

-
 <histogram name="SB2.SideEffectFreeWhitelistStatus"
     enum="SB2SideEffectFreeWhitelistStatus">
   <owner>shess@chromium.org</owner>