Skip to content

Commit

Permalink
sql: Add BuiltInRecovery::IsSupported method
Browse files Browse the repository at this point in the history
BuiltInRecovery is not yet supported on Fuchsia. Consumers can call this
static helper method rather than manually checking buildflags themselves

Bug: 1385500
Change-Id: I1cc4eb3b95fb827bc16ab8ee89619c99d0068427
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4544906
Reviewed-by: Evan Stade <estade@chromium.org>
Auto-Submit: Austin Sullivan <asully@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146280}
  • Loading branch information
a-sully authored and Chromium LUCI CQ committed May 19, 2023
1 parent 0f7a32e commit 44dafca
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 17 deletions.
6 changes: 1 addition & 5 deletions sql/recovery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/types/pass_key.h"
#include "build/build_config.h"
#include "sql/database.h"
#include "sql/error_delegate_util.h"
#include "sql/internal_api_token.h"
Expand Down Expand Up @@ -65,10 +64,7 @@ BuiltInRecovery::BuiltInRecovery(Database* database, Strategy strategy)
.page_size = database ? database->page_size() : 0,
.cache_size = 0,
}) {
// TODO(https://crbug.com/1385500): Make built-in recovery work on Fuchsia.
#if BUILDFLAG(IS_FUCHSIA)
NOTIMPLEMENTED();
#endif // BUILDFLAG(IS_FUCHSIA)
CHECK(IsSupported());
CHECK(db_);
CHECK(db_->is_open());
// Recovery is likely to be used in error handling. To prevent re-entry due to
Expand Down
13 changes: 11 additions & 2 deletions sql/recovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/component_export.h"
#include "base/memory/raw_ptr.h"
#include "build/build_config.h"
#include "sql/database.h"
#include "sql/internal_api_token.h"
#include "sql/sqlite_result_code_values.h"
Expand All @@ -25,8 +26,6 @@ namespace sql {
// recovery to your feature that uses SQLite, use the `Recovery` class below...
// for now! See https://crbug.com/1385500.
//
// TODO(https://crbug.com/1385500): This currently does not work on Fuchsia.
//
// Uses SQLite's built-in corruption recovery module to recover the database.
// See https://www.sqlite.org/recovery.html
class COMPONENT_EXPORT(SQL) BuiltInRecovery {
Expand All @@ -45,6 +44,16 @@ class COMPONENT_EXPORT(SQL) BuiltInRecovery {
// successfully-recovered, but unsuccessfully-restored database if needed.
};

// TODO(https://crbug.com/1385500): `BuiltInRecovery` is not yet supported on
// Fuchsia.
static bool IsSupported() {
#if BUILDFLAG(IS_FUCHSIA)
return false;
#else
return true;
#endif // BUILDFLAG(IS_FUCHSIA)
}

// Returns true if `RecoverDatabase()` can plausibly fix `database` given this
// `extended_error`. This does not guarantee that `RecoverDatabase()` will
// successfully recover the database.
Expand Down
11 changes: 1 addition & 10 deletions sql/recovery_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
#include "base/strings/string_number_conversions.h"
#include "base/test/bind.h"
#include "base/test/gtest_util.h"
#include "base/types/pass_key.h"
#include "build/build_config.h"
#include "sql/database.h"
#include "sql/meta_table.h"
#include "sql/sqlite_result_code_values.h"
Expand Down Expand Up @@ -82,14 +80,7 @@ class SqlRecoveryTest : public testing::TestWithParam<bool> {
return file.Write(0, kText, kTextBytes) == kTextBytes;
}

bool UseBuiltIn() {
#if BUILDFLAG(IS_FUCHSIA)
// TODO(https://crbug.com/1385500): Make built-in recovery work on Fuchsia.
return false;
#else
return GetParam();
#endif // BUILDFLAG(IS_FUCHSIA)
}
bool UseBuiltIn() { return GetParam() && BuiltInRecovery::IsSupported(); }

protected:
base::ScopedTempDir temp_dir_;
Expand Down

0 comments on commit 44dafca

Please sign in to comment.