0
Commit Graph

12 Commits

Author SHA1 Message Date
Anthony Vallée-Dubois
9adf0bbf4b Builder pattern in DatabaseOptions
DatabaseOptions is getting too large for the "explicit out of line
constructor for complex types" presubmit. Adding a constructor to it
prevents it from being an aggregate type, which is how most of the
callers currently use it.

This Cl makes DatabaseOptions members private and adds builder-type
setters for each member. It also updates all callers, and adds an out of
line constructor.

A future improvement could be to add a passkey to the setters for
discouraged options.

Bug: None
Change-Id: I63562f43c8b290247878d194039487b240e958c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216099
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Commit-Queue: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1414974}
2025-02-03 09:01:41 -08:00
Anthony Vallée-Dubois
e3c94919c5 Make database tag mandatory
This CL removes the default value of the `tag` argument on the 2
`sql::Database` constructors, and updates all callers that didn't
explicitly pass a tag to do so.

As a convenience, this CL also defines a common tag for unit tests.

This will allow the implementation and monitoring of per-database
performance metrics (time to open, statement execution time, VMSteps,
etc) without the possibility of having some of the databases
uninstrumented. This is useful for diagnosing issues such as crbug.com/369635654 in the wild, and required for some performance investigations that we have in the pipeline.

The last step of this work item (asserting that the tag is correctly defined in histograms.xml variants) is implemented in https://chromium-review.googlesource.com/c/chromium/src/+/6055279.

Bug: 40949392
Change-Id: I6dec0fb86a5e7b98cd42ac3a9db18e23eaf9e9bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6039025
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Commit-Queue: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1395446}
2024-12-12 08:47:47 -08:00
Andrew Paseltiner
ab20e8b9cd Remove ability to enable virtual tables from sql::DatabaseOptions
This change almost allows us to remove FTS3 support from the SQLite
build (saving ~64kb of binary size on Android), as it is only needed by
WebSQL, which is in the process of being removed, but WebSQL is still
enabled for WebView.

As a result, in order to retain test coverage of FTS3 availability,
we add a test-only private field to allow sql::Database to continue
enabling virtual tables. This field will be removed once we are sure
that WebSQL and therefore FTS3 support can be removed as well.

Bug: 340805983
Change-Id: I7ac2f7aeb35bd230194ac224a9413a47a7343988
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5539299
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1301504}
2024-05-15 19:29:45 +00:00
Takuto Ikuta
2eb6134247 sql: apply clang-tidy's misc-include-cleaner
Bug: 336474469
Change-Id: I3378c04b5b88c202885e412bbe2190af6824433f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5504232
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1299145}
2024-05-10 09:05:35 +00:00
Arthur Sonzogni
5bc3326c91 Cleanup: Format <optional> include order.
This is an automated patch. Please avoid assigning unrelated bug.
 #cleanup

This is a follow-up to @thestig comment:
https://chromium-review.googlesource.com/c/chromium/src/+/5009410/7/pdf/accessibility_helper.cc#11

Several developers, including myself rewrote optional without properly
reordering the #includes at the beginning.
This patches automatically fixes it.

Script:
-------

```
function replace {
  echo "Replacing $1 by $2"
  git grep -l "$1" \
    | cut -f1 -d: \
    | grep -v \
      -e "*win*" \
      -e "third_party/*" \
      -e "tools/*" \
    | grep \
      -e "\.h" \
      -e "\.cc" \
    | sort \
    | uniq \
    | xargs sed -i "s/$1/$2/g"
}

replace "#include <optional>" ""
git add -u
git commit -m "remove optional"
git revert HEAD --no-commit
git add -u
echo "Formatting":
echo "IncludeBlocks: Regroup" >> ".clang-format"
echo "IncludeIsMainRegex: \"(_(android|apple|chromeos|freebsd|fuchsia|fuzzer|ios|linux|mac|nacl|openbsd|posix|stubs?|win))?(_(unit|browser|perf)?tests?)?$\"" >> ".clang-format"
git add -u
git cl format --upstream=HEAD
git checkout origin/main -- ".clang-format"
git add -u
git commit -m "revert with format"
git reset --soft origin/main
git add -u
git commit -m "Automated format"
```

cleanup: This is a no-op patch formatting includes.
Bug: 40288126
Change-Id: I5f61b1207c097a4c6b20a034f9d1b323975b1851
AX-Relnotes: n/a.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5335142
Owners-Override: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1267143}
2024-02-29 19:39:05 +00:00
Arthur Sonzogni
59ac8227c5 Rename {absl => std}::optional in minor top-level dirs
Automated patch, intended to be effectively a no-op.

This patch gather the changes for every top-level directories with less
than 150 files modified:
 #  directory
--- ---------------
150 remoting
98  gpu
87  chromecast
79  mojo
70  storage
65  fuchsia_web
46  sandbox
44  android_webview
38  google_apis
27  pdf
25  printing
20  headless
13  ipc
11  crypto
10  sql
3   dbus
2   testing
2   skia
2   gin
2   apps
1   rlz
1   codelabs

Context:
https://groups.google.com/a/chromium.org/g/cxx/c/nBD_1LaanTc/m/ghh-ZZhWAwAJ?utm_medium=email&utm_source=footer

As of https://crrev.com/1204351, absl::optional is now a type alias for
std::optional. We should migrate toward it.

Script:
```

function replace {
  echo "Replacing $1 by $2"
  git grep -l "$1" \
		| cut -f1 -d: \
		| grep \
			-e "^codelabs" \
			-e "^rlz" \
			-e "^apps" \
			-e "^gin" \
			-e "^skia" \
			-e "^testing" \
			-e "^dbus" \
			-e "^sql" \
			-e "^crypto" \
			-e "^ipc" \
			-e "^headless" \
			-e "^printing" \
			-e "^pdf" \
			-e "^google_apis" \
			-e "^android_webview" \
			-e "^sandbox" \
			-e "^fuchsia_web" \
			-e "^storage" \
			-e "^mojo" \
			-e "^chromecast" \
			-e "^gpu" \
			-e "^remoting" \
		| sort \
		| uniq \
		| grep \
			-e "\.h" \
			-e "\.cc" \
			-e "\.mm" \
			-e "\.py" \
	  | xargs sed -i "s/$1/$2/g"
}

replace "absl::make_optional" "std::make_optional"
replace "absl::optional" "std::optional"
replace "absl::nullopt" "std::nullopt"
replace "absl::in_place" "std::in_place"
replace "absl::in_place_t" "std::in_place_t"
replace "\"third_party\/abseil-cpp\/absl\/types\/optional.h\"" "<optional>"
git cl format
```

CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-betty-pi-arc-chrome

Bug: chromium:1500249
Change-Id: I0eca8ff96f5712ba746ac8d8da93d03a86d8292c
AX-Relnotes: n/a.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5009410
Reviewed-by: danakj <danakj@chromium.org>
Owners-Override: danakj <danakj@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1222826}
2023-11-10 09:46:54 +00:00
Avi Drissman
2e85357a8b Update header includes for /base/functional in /s*
bind.h, callback.h, callback_forward.h, and callback_helpers.h
moved into /base/functional/. Update the include paths to
directly include them in their new location.

Bug: 1364441
Change-Id: I9576f9ae63e93053a6ce2c8cc64ebf6c85c40b1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4159511
Auto-Submit: Avi Drissman <avi@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1092060}
2023-01-12 21:57:32 +00:00
Avi Drissman
69b874fee0 Update copyright headers in sql/, storage/
The methodology used to generate this CL is documented in
https://crbug.com/1098010#c95.

No-Try: true
Bug: 1098010
Change-Id: I68bb81a4dcae37f944f4d8cd39d82ed540364615
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3899461
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Owners-Override: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1047607}
2022-09-15 19:11:14 +00:00
Victor Costan
335b822ecc sql: Check for on-disk page size in DatabaseOptions test.
This CL moves ReadPageSize() from an anonymous namespace to

to ensure that SQLite is indeed using the indicated page size.

sql: :test:ReadDatabasePageSize() and uses it in DatabaseOptions's test
Change-Id: If037ed19e96a88d7a199bb98685e7bd6b561730a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3541048
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983717}
2022-03-22 08:15:11 +00:00
Victor Costan
ab7a24586c sql: Introduce strongly typed enums for SQLite result codes.
This CL introduces the following strongly typed enums, which are meant
to replace raw integers in Chrome's layer on top of SQLite.

* SqliteResultCode represents all result codes.
* SqliteErrorCode represent a result code that's known to be an error.

This CL integrates each new type in one place, to give an idea of what
our code will look like once we adopt these new types.

* SqliteResultCode is integrated in
  Database::ExecuteAndReturnResultCode().
* SqliteErrorCode is integrated in Database::OnSqliteError().

The new enums are declared in //sql/sqlite_result_code.h, together with
helper functions for converting raw integers returned by the SQLite API.
The values for the new enums are defined in a separate header,
//sql/sqlite_result_code_values.h. This is intended to help readability,
as sqlite_result_code.h has everything developers need to start using
the types, while the boilerplate is relegated to
sqlite_result_code_values.h.

Last, this CL moves the SqliteLoggedResultCode logic from
error_metrics.{h,cc} into the newly introduced files. This is done in
part because this type is also a strongly typed replacement for SQLite's
result codes, and in part because we want all the SQLite result code
conversion logic to share one mapping table.

Bug: 1308290
Change-Id: I580ae4649cabccaab74c1d4749b5f349cf42f047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3538180
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#983543}
2022-03-21 23:06:08 +00:00
Victor Costan
f2ed41f738 sql: Add DatabaseOption for flushing transaction data to media.
Change-Id: I7f5800d6107a8b54afd3f8abcd2be68c2d64fe13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3527340
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#981482}
2022-03-16 04:04:38 +00:00
Victor Costan
5f5104901a sql: Add dedicated tests for DatabaseOptions.
The tests ensure that our sql::Database::Open*() implementations
configure SQLite with the values in sql::DatabaseOptions. Some tests in
this CL are new, while some tests are moved over from sql::Database's
unit tests.

Change-Id: I709c5380726e1a3f6cd9022cce0734b6d2cbe713
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3526264
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/main@{#981393}
2022-03-15 23:50:23 +00:00