[sql] Track uploads of diagnostic data to prevent duplication.

sql/connection.h

Index: sql/connection.h
diff --git a/sql/connection.h b/sql/connection.h
index 7d2ab6a355bfc9d448f4f58fd18480b2e3d1d722..b12e43f36be6dfe8a875ce1fc4d1e31b3b1cde40 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -470,6 +470,14 @@ class SQL_EXPORT Connection : public base::trace_event::MemoryDumpProvider {
       const base::trace_event::MemoryDumpArgs& args,
       base::trace_event::ProcessMemoryDump* process_memory_dump) override;
 
+  // Helper to determine if it would be appropriate to upload a diagnostic crash
+  // dump for this database.  Returns |false| if a previous dump was uploaded,
+  // or if the database directory is broken in a way which prevents recording
+  // the intention to upload (implying that the system is probably broken in a
+  // way which Chromium cannot address).  Should return |true| only once for
+  // each unique histogram tag.

[pkotwicz, 2015/10/15 18:02:30]

Nit: In the comment can you mention that calling ShouldUploadDiagnosticDump()
sets state and that calling ShouldUploadDiagnosticDump() twice will return true
at most once?

I am not a big fan of the method name but I cannot think of a better method
name.

[Scott Hess - ex-Googler, 2015/10/15 21:12:21]

Changed to RegisterIntentToUpload(), with the return value interpreted as
whether registration was successful.

Overhauled the comments.

[pkotwicz, 2015/10/15 23:31:06]

I like this function name better. Thanks for changing

+  bool ShouldUploadDiagnosticDump() const;
+
  private:
   // For recovery module.
   friend class Recovery;
@@ -504,7 +512,7 @@ class SQL_EXPORT Connection : public base::trace_event::MemoryDumpProvider {
   // Check whether the current thread is allowed to make IO calls, but only
   // if database wasn't open in memory. Function is inlined to be a no-op in
   // official build.
-  void AssertIOAllowed() {
+  void AssertIOAllowed() const {
     if (!in_memory_)
       base::ThreadRestrictions::AssertIOAllowed();
   }
@@ -653,6 +661,10 @@ class SQL_EXPORT Connection : public base::trace_event::MemoryDumpProvider {
   // which do not participate in the total-rows-changed tracking.
   void ReleaseCacheMemoryIfNeeded(bool implicit_change_performed);
 
+  // Return the results of sqlite3_db_filename(), which should match the path
+  // passed to Open().

[pkotwicz, 2015/10/15 18:02:30]

Nit: Return -> Returns

[Scott Hess - ex-Googler, 2015/10/15 21:12:21]

Acknowledged.  Also moved impl to after ReleaseCacheMemoryIfNeeded().

+  base::FilePath DbPath() const;
+
   // The actual sqlite database. Will be NULL before Init has been called or if
   // Init resulted in an error.
   sqlite3* db_;