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

sql/connection.h

 // Copyright (c) 2012 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 SQL_CONNECTION_H_
 #define SQL_CONNECTION_H_
 
 #include <stdint.h>
 #include <map>
 #include <set>
@@ skipping 452 matching lines @@
   // error-handling at the end of OnSqliteError().  Expose to allow
   // those clients to work appropriately with ScopedErrorIgnorer in
   // tests.
   static bool ShouldIgnoreSqliteError(int error);
 
   // base::trace_event::MemoryDumpProvider implementation.
   bool OnMemoryDump(
       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;
 
   // Allow test-support code to set/reset error ignorer.
   friend class ScopedErrorIgnorer;
 
   // Statement accesses StatementRef which we don't want to expose to everybody
   // (they should go through Statement).
   friend class Statement;
@@ skipping 14 matching lines @@
   };
   bool OpenInternal(const std::string& file_name, Retry retry_flag);
 
   // Internal close function used by Close() and RazeAndClose().
   // |forced| indicates that orderly-shutdown checks should not apply.
   void CloseInternal(bool forced);
 
   // 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();
   }
 
   // Internal helper for DoesTableExist and DoesIndexExist.
   bool DoesTableOrIndexExist(const char* name, const char* type) const;
 
   // Accessors for global error-ignorer, for injecting behavior during tests.
   // See test/scoped_error_ignorer.h.
   typedef base::Callback<bool(int)> ErrorIgnorerCallback;
@@ skipping 128 matching lines @@
   base::TimeTicks Now() {
     return clock_->Now();
   }
 
   // Release page-cache memory if memory-mapped I/O is enabled and the database
   // was changed.  Passing true for |implicit_change_performed| allows
   // overriding the change detection for cases like DDL (CREATE, DROP, etc),
   // 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_;
 
   // Parameters we'll configure in sqlite before doing anything else. Zero means
   // use the default value.
   int page_size_;
   int cache_size_;
   bool exclusive_locking_;
   bool restrict_to_user_;
@@ skipping 63 matching lines @@
   // Source for timing information, provided to allow tests to inject time
   // changes.
   scoped_ptr<TimeSource> clock_;
 
   DISALLOW_COPY_AND_ASSIGN(Connection);
 };
 
 }  // namespace sql
 
 #endif  // SQL_CONNECTION_H_