Create CrashReportDatabase interface, a test, and a Mac implementation.

client/crash_report_database.h

+// Copyright 2015 The Crashpad Authors. All rights reserved.

[Mark Mentovai, 2015/01/08 22:38:09]

Top-level question: what do you do about reports that we collect while uploads
are turned off?

[Robert Sesek, 2015/01/13 16:18:22]

They can be retrieved with GetReportsNotUploaded(), and using the (newly added)
creation time, the uploader can choose to upload recent ones.

+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_CLIENT_CRASH_REPORT_DATABASE_H_
+#define CRASHPAD_CLIENT_CRASH_REPORT_DATABASE_H_
+
+#include <time.h>
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "util/misc/uuid.h"
+
+namespace crashpad {
+
+//! \brief An interface for managing a collection of crash report files and
+//!     metadata associated with the crash reports.
+//!
+//! All \a Report objects that are returned by this class are logically const.

[Mark Mentovai, 2015/01/08 22:38:09]

You don’t need the \a, this gets linked automatically and that’s all of the
special formatting we need. Same on line 41.
http://www.doxygen.org/manual/autolink.html#linkclass

\a is best for parameters, which aren’t detected in prose normally, and need
special treatment to be formatted differently.

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+//! They are snapshots of the database at the time the query was ran, and the

[Mark Mentovai, 2015/01/08 22:38:09]

run

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+//! data returned is liable to change after the query is executed.
+class CrashReportDatabase {
+ public:
+  //! \brief A crash report record.
+  //!
+  //! This represents the metadata for a crash report, as well as the location
+  //! of the report itself. A \a CrashReportDatabase maintains at least this

[Mark Mentovai, 2015/01/08 22:38:09]

No \a, it gets linked automatically.

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+  //! information.
+  struct Report {
+    Report() : uuid(), file_path(), id(), uploaded(false),

[Mark Mentovai, 2015/01/08 22:38:09]

I wouldn’t inline this.

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+               last_upload_attempt_time(0), upload_attempts(0) {}
+
+    // A unique identifier by which this report will always be known to the

[Mark Mentovai, 2015/01/08 22:38:09]

//! doxygen me, throughout this struct.

Looks like you meant to do that, because I see \a in here (which should be
changed to #, read on…)

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+    // database.
+    UUID uuid;
+
+    // The current location of the crash report on the client's filesystem.

[Mark Mentovai, 2015/01/08 22:38:09]

Your quotes aren’t too smart (but if you find it hard to type ’em smart, it’s
OK.)

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+    // The location of a crash report may change over time, so the UUID should
+    // be used as the canonical identifier.
+    base::FilePath file_path;
+
+    // An identifier issued to this crash report by a collection server.
+    std::string id;
+
+    // Whether this crash report was successfully uploaded to a collection
+    // server.
+    bool uploaded;
+
+    // The last timestamp at which an attempt was made to submit this crash
+    // report to a collection server. If this is zero, then the report has never
+    // been uploaded. If \a uploaded is true, then this timestamp is the time

[Mark Mentovai, 2015/01/08 22:38:09]

#uploaded

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+    // at which the report was uploaded, and no other attempts to upload this
+    // report will be made.
+    time_t last_upload_attempt_time;
+
+    // The number of times an attempt was made to submit this report to
+    // a collection server. If this is more than zero, then \a

[Mark Mentovai, 2015/01/08 22:38:09]

#last_upload_attempt_time.

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+    // last_upload_attempt_time will be set to the timestamp of the most
+    // recent attempt.
+    int upload_attempts;
+  };
+
+  //! \brief The result code for operations performed on a database.
+  enum OperationStatus {
+    //! \brief No error occurred.
+    kNoError = 0,
+
+    //! \brief The report that was requested could not be located.
+    kReportNotFound,
+
+    //! \brief An error occured while performing a file operation on a crash
+    //!     report.
+    //!
+    //! A database is responsible for managing both the metadata about a report
+    //! and the actual crash report itself. This error is returned when an
+    //! error occurred when managing the report file. Additional information
+    //! will be logged.
+    kFileSystemError,
+
+    //! \brief An error occured while recording metadata for a crash report.
+    //!
+    //! A database is responsible for managing both the metadata about a report
+    //! and the actual crash report itself. This error is returned when an
+    //! error occurred when managing the metadata about a crash report.
+    //! Additional information will be logged.
+    kDatabaseError,
+  };
+
+  virtual ~CrashReportDatabase() {}
+
+  //! \brief Initializes a database of crash reports.
+  //!
+  //! \param[in] path A path to a writable directory, where the database can

[Mark Mentovai, 2015/01/08 22:38:09]

It’s kinda weird for the interface to take the parent pathname instead of the
actual pathname.

Think about it from the perspective of other hypothetical implementations of
this interface, like one that stuffs everything in a sqlite3 database. You’d
probably want to say “use the crash report database at /home/rsesek/crashes” and
if you were using the dir/xattr-based implementation, have that mean that
/home/rsesek/crashes should be the directory that has the three subdirectories.
If you were using the sqlite3 one, you’d probably want it to mean
/home/rsesek/crashes.db.

I don’t see a good reason to mandate a directory named “Crashpad Database” at
the level of this interface or the dir/xattr-based implementation.

[Mark Mentovai, 2015/01/08 22:38:09]

The database doesn’t have to be a directory as far as this interface is
concerned, does it?

[Robert Sesek, 2015/01/13 16:18:22]

No.

[Robert Sesek, 2015/01/13 16:18:22]

I disagree. The caller does not know the structure nor name of the database, so
it should pass the directory to where it wants the database set up. In Chrome,
this would be the user data dir, for example. Even if the database were
SQLite-backed, the database may still require a directory for files.

+  //!     be created or opened.
+  //!
+  //! \return A database object on success, NULL on failure with an error

[Mark Mentovai, 2015/01/08 22:38:09]

`nullptr`

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+  //!     logged.
+  static scoped_ptr<CrashReportDatabase> Initialize(const base::FilePath& path);
+
+  //! \brief Creates a record of a new crash report.
+  //!
+  //! Callers can then write the crash report at the file path provided in the
+  //! record. The file at the Report::file_path will not be created as a result
+  //! of this call.
+  //!
+  //! \param[out] report A crash report record.

[Mark Mentovai, 2015/01/08 22:38:09]

…only valid if this returns OperationStatus::kNoError. (Right?) Same for several
other methods.

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+  //!
+  //! \return The operation status code.
+  virtual OperationStatus PrepareNewCrashReport(Report* report) = 0;
+
+  //! \brief Informs the database that a crash report has been written.
+  //!
+  //! After calling this method, the database is permitted to move and rename
+  //! the file at Record::file_path.
+  //!
+  //! \param[in] uuid The crash report record unique identifier. The identifier
+  //!     must be valid.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus FinishedWritingCrashReport(const UUID& uuid) = 0;
+
+  //! \brief Returns the crash report record for the unique identifier.
+  //!
+  //! \param[in] uuid The crash report record unique identifier.
+  //! \param[out] report A crash report record.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus LookUpCrashReport(const UUID& uuid,
+                                            Report* report) = 0;
+
+  //! \brief Returns a list of crash report records that have not been uploaded.
+  //!
+  //! \param[out] reports A list of crash report record objects. This must be
+  //!     empty.

[Mark Mentovai, 2015/01/08 22:38:09]

must be empty on entry.

Same for GetUploadedReports.

[Robert Sesek, 2015/01/13 16:18:22]

Done.

+  //!
+  //! \return The operation status code.
+  virtual OperationStatus GetNotUploadedReports(
+      std::vector<const Report>* reports) = 0;
+
+  //! \brief Returns a list of crash report records that have been uploaded.
+  //!
+  //! \param[out] reports A list of crash report record objects. This must be
+  //!     empty.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus GetUploadedReports(
+      std::vector<const Report>* reports) = 0;
+
+  //! \brief Adjusts a crash report record's metadata to account for an upload
+  //!     attempt.
+  //!
+  //! After calling this method, the database is permitted to move and rename
+  //! the file at Record::file_path.
+  //!
+  //! \param[in] uuid The unique identifier for the crash report record. The
+  //!     identifier must be valid.
+  //! \param[in] successful Whether the upload attempt was successful.
+  //! \param[in] id The identifier assigned to this crash report by the
+  //!     collection server. Must be empty if \a successful is `false`; may be
+  //!     empty if it is `true`.
+  //!
+  //! \return The operation status code.
+  virtual OperationStatus RecordUploadAttempt(const UUID& uuid,
+                                              bool successful,
+                                              const std::string& id) = 0;
+
+ protected:
+  CrashReportDatabase() {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(CrashReportDatabase);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_CLIENT_CRASH_REPORT_DATABASE_H_