Add functionality to prune old crash reports from the database.

client/prune_crash_reports.h

Index: client/prune_crash_reports.h
diff --git a/client/prune_crash_reports.h b/client/prune_crash_reports.h
new file mode 100644
index 0000000000000000000000000000000000000000..70a4cffbedaaa42dee14176ac62562135e48faad
--- /dev/null
+++ b/client/prune_crash_reports.h
@@ -0,0 +1,134 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// 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_PRUNE_CRASH_REPORTS_H_
+#define CRASHPAD_CLIENT_PRUNE_CRASH_REPORTS_H_
+
+#include <time.h>
+
+#include "base/memory/scoped_ptr.h"
+#include "client/crash_report_database.h"
+
+namespace crashpad {
+
+class PruneCondition;
+
+//! \brief Deletes crash reports from \a database that match \a condition.
+//!
+//! This function can be used to remove old or large reports from the database.
+//! The \a condition will be evaluated against each report in the \a database,
+//! sorted in descending by CrashReportDatabase::Report::creation_time. This

[Mark Mentovai, 2015/10/07 03:54:28]

in descending what?

[Robert Sesek, 2015/10/07 16:24:35]

Done.

+//! guarantee allows conditions to be stateful.
+//!
+//! \param[in] database The database from which crash reports will be deleted.
+//! \param[in] condition The condition by which all reports in the database

[Mark Mentovai, 2015/10/07 03:54:28]

by → against?

[Robert Sesek, 2015/10/07 16:24:34]

Done.

+//!     will be evaluated.
+void PruneCrashReportDatabase(CrashReportDatabase* database,
+                              PruneCondition* condition);
+
+//! \brief Returns a sensible default condition for removing obsolete crash

[Mark Mentovai, 2015/10/07 03:54:28]

This should actually say what the condition is in the body (doesn’t have to be
in the \brief).

[Robert Sesek, 2015/10/07 16:24:35]

Didn't know if we wanted to define that in the API docs or not. Done.

+//!     reports.
+//!
+//! \return A PruneCondition for use with PruneCrashReportDatabase().
+scoped_ptr<PruneCondition> GetDefaultDatabasePruneCondition();

[Mark Mentovai, 2015/10/07 03:54:28]

Maybe make this a static function, PruneCondition::GetDefault()?

[Robert Sesek, 2015/10/07 16:24:35]

Done.

+
+//! \brief An abstract base class for evaluating crash reports for deletion.
+//!
+//! When passed to PruneCrashReportDatabase(), each crash report in the
+//! database will be evaluated according to ShouldPruneReport(). The reports
+//! are evaluated serially in descending sort order by Report::creation_time.
+class PruneCondition {
+ public:
+  virtual ~PruneCondition() {}
+
+  //! \brief Evaluates a crash report for deletion.
+  //!
+  //! \param[in] report The crash report to evaluate.
+  //!
+  //! \return `true` if the crash report should be deleted, `false` if it
+  //!     should be kept.
+  virtual bool ShouldPruneReport(const CrashReportDatabase::Report& report) = 0;
+};
+
+//! \brief A PruneCondition that deletes reports older than the specified number
+//!     days.
+class AgePruneCondition : public PruneCondition {
+ public:
+  //! \brief Creates a PruneCondition based on Report::creation_time.

[Mark Mentovai, 2015/10/07 03:54:28]

CrashReportDatabase::Report::creation_time

[Robert Sesek, 2015/10/07 16:24:35]

Done.

+  //!
+  //! \param[in] max_age_in_days Reports created more than this many days ago
+  //!     will be deleted.
+  explicit AgePruneCondition(int max_age_in_days);
+  ~AgePruneCondition();
+
+  bool ShouldPruneReport(const CrashReportDatabase::Report& report) override;
+
+ private:
+  time_t oldest_report_time_;

[Mark Mentovai, 2015/10/07 03:54:28]

const

[Robert Sesek, 2015/10/07 16:24:35]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(AgePruneCondition);
+};
+
+//! \brief A PruneCondition that deletes older reports to keep the total
+//!     Crashpad database size under the specified limit.
+class DatabaseSizePruneCondition : public PruneCondition {
+ public:
+  //! \brief Creates a PruneCondition that will keep newer reports, until the
+  //!     sum of the size of all reports is not smaller than
+  //!     \a max_size_in_bytes.

[Mark Mentovai, 2015/10/07 03:54:28]

Clarify that after that, it will prune.

[Robert Sesek, 2015/10/07 16:24:34]

Done.

+  //!
+  //! \param[in] max_size_in_bytes The maximum number of bytes that all crash
+  //!     crash reports should consume.

[Mark Mentovai, 2015/10/07 03:54:28]

Duplicate “crash crash.”

[Robert Sesek, 2015/10/07 16:24:34]

Done.

+  explicit DatabaseSizePruneCondition(size_t max_size_in_bytes);

[Mark Mentovai, 2015/10/07 03:54:28]

<sys/types.h> for size_t.

[Mark Mentovai, 2015/10/07 03:54:28]

“Bytes” is probably not the best choice for a unit here, it’s too easy to
overflow (in 32-bit-land, at least). You used “days” for AgePruneCondition, how
about kilobytes here?

[Robert Sesek, 2015/10/07 16:24:35]

Done.

[Robert Sesek, 2015/10/07 16:24:35]

Done.

+  ~DatabaseSizePruneCondition();
+
+  bool ShouldPruneReport(const CrashReportDatabase::Report& report) override;
+
+ private:
+  const size_t max_size_in_bytes_;
+  size_t measured_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(DatabaseSizePruneCondition);
+};
+
+//! \breif A PruneCondition that conjoins two other PruneConditions.
+class BinaryPruneCondition : public PruneCondition {
+ public:
+  enum Operator {

[Mark Mentovai, 2015/10/07 03:54:28]

enum class is hot and should work just fine in this application.

[Robert Sesek, 2015/10/07 16:24:34]

Sure, but I think it'd be more verbose than necessary here.

+    AND,
+    OR,
+  };
+
+  //! \brief Evaluates two sub-conditions according to the specified logical
+  //!     operator.
+  //!
+  //! \param[in] op The logical operator to apply on \a lhs and \a rhs.

[Mark Mentovai, 2015/10/07 03:54:28]

Specify whether short-circuiting is implemented.

It is. (Is that what you want? Seems like it could mess up tabulation of
DatabaseSizePruneCondition::measured_size_. The exact behavior seems like
something that you ought to have a test for, too.)

[Robert Sesek, 2015/10/07 16:24:34]

Done.

+  //! \param[in] lhs The left-hand side of \a op. This class takes ownership.
+  //! \param[in] rhs The right-hand side of \a op. This class takes ownership.
+  BinaryPruneCondition(Operator op, PruneCondition* lhs, PruneCondition* rhs);
+  ~BinaryPruneCondition();
+
+  bool ShouldPruneReport(const CrashReportDatabase::Report& report) override;
+
+ private:
+  Operator op_;

[Mark Mentovai, 2015/10/07 03:54:28]

These data members can be const. Well, maybe lhs_ and rhs_ can’t be. But op_
can.

[Robert Sesek, 2015/10/07 16:24:35]

Done.

+  scoped_ptr<PruneCondition> lhs_;
+  scoped_ptr<PruneCondition> rhs_;
+
+  DISALLOW_COPY_AND_ASSIGN(BinaryPruneCondition);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_CLIENT_PRUNE_CRASH_REPORTS_H_