The last remaining SQLite patch is
0001-Custom-shell.c-helpers-to-load-Chromium-s-ICU-data. This CL adds
tests ensuring that SQLite uses ICU, creating a safety net for the
removal of the SQLite patch.
Bug: 945204
Change-Id: I2b5661d78fcd00e7c088fc234eeef0f981f2bb62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1621845
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#661968}
Previously unix-dotlock VFS was used by default on Fuchsia. dotlock
creates .lock directory to lock files. It doesn't work correctly because
the lock is not removed if the process dies before releasing the lock.
On Fuchsia profile directories are not expected to be shared between
processes, so cross-process file locks are not necessary.
This CL switches Chromium's VFS wrapper to use unix-none as a base VFS
(which uses no-op locking) and adds custom in-process locking for
Fuchsia in the wrapper.
Bug: 936672
Change-Id: Ibb0a84ba0e540b21551d5f174ec32f0d9077cf98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504048
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#639257}
This CL:
1) Re-enables MacUtilTest.TestExcludeFileFromBackups, which was disabled
due to failures on a bot that does not exist anymore.
2) Extracts the logic used to check base::mac::SetFileBackupExclusion()
into base::mac::GetFileBackupExclusion(). This logic was duplicated
in tests.
3) Uses the approach in //sql for converting a base::FilePath into the
CFURLRef needed by CSBackup{Is,Set}ItemExcluded. This approach does
not rely on an autorelease pool. The conversion logic is extracted in
base::mac::FilePathToCFURL().
Change-Id: I6ed2a3ded285e6904dccf7e21ade7ff5f68167a2
Reviewed-on: https://chromium-review.googlesource.com/c/1438035
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626702}
SQLite patch 0007-Allow-auto-vacuum-to-work-with-chunks.patch
introduces a PRAGMA that is not used outside of tests. The PRAGMA was
added for an experiment that was discontinued, referenced on the bug
above.
This CL removes the patch and the associated tests, reducing the total
amount of patches we carry on top of SQLite.
No functional changes are expected, because the patch is inert in
production. Specifically, the newly introduced autoVacuumSlack member of
the BtShared structure is zero-initialized by default, and can only be
changed by issuing the auto_vacuum_slack_pages PRAGMA. The new btree
code is gated by an if (... && pBt->autoVacuumSlack). The PRAGMA is not
used outside of testing.
Bug: 698010
Change-Id: Id2a40badccb96389c6ff9ce1780ffa703dedce6c
Reviewed-on: https://chromium-review.googlesource.com/c/1362222
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613782}
This reverts commit 7202f4dc1f.
Reason for revert: CL is only needed in 72.0.3620.0 Windows Canary to diagnose SQLite crashes.
Original change's description:
> sqlite: Disable mmap usage on Windows.
>
> This is an experiment to see if the crashes in the referenced bug are
> related to mmap changes on Windows. This CL will be reverted after it
> goes live in 1-2 canaries.
>
> TBR=cmumford
>
> Bug: 897576
> Change-Id: Ic44836c4ed848f8713d16b553f038c63119f0aaf
> Reviewed-on: https://chromium-review.googlesource.com/c/1348736
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Victor Costan <pwnall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610551}
TBR=cmumford@chromium.org,pwnall@chromium.org
Change-Id: I4d79a63c2a428e1e8ab04143a4d8280e274db1fd
Bug: 897576
Reviewed-on: https://chromium-review.googlesource.com/c/1349789
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610700}
This is an experiment to see if the crashes in the referenced bug are
related to mmap changes on Windows. This CL will be reverted after it
goes live in 1-2 canaries.
TBR=cmumford
Bug: 897576
Change-Id: Ic44836c4ed848f8713d16b553f038c63119f0aaf
Reviewed-on: https://chromium-review.googlesource.com/c/1348736
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610551}
sql::Connection represents an opened SQLite database. Pedantically, the
name is correct. At the same time, it is inconsistent with the naming
convention of similar software -- IndexedDB, LevelDB, and WebSQL all use
a derivative of "Database" (IDBDatabase, DB, WindowDatabase) to name
this concept. Furthermore, most sql::Connection instances have names
like "db" and "database".
This CL renames the class to sql::Database, reducing the cognitive load
caused by dissonances like "sql::Connection db_".
TBR=mpearson
Bug: none
Change-Id: I8abf34016bca236b6e38a772f5848e7ea9bf1c59
Reviewed-on: https://chromium-review.googlesource.com/1157876
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579990}
Chrome's SQLite currently uses permissive (0644) POSIX access controls
for newly created databases, and //sql offers a method for opting into
restrictive (0600) permissions. This method is only used by the login
database. However, all Chrome databases are likely to have private data.
For example, the cookies database may contain long-lived OAuth tokens,
and can be just as valuable as the login database. The same argument
applies for the DOMStorage database.
This CL configures SQLite to use restrictive permissions by default, and
removes the method for opting into the restrictive permissions.
Change-Id: I5f0ce9e7f038081fad515cfc30c45ccccf7ff1b6
Reviewed-on: https://chromium-review.googlesource.com/1146295
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577476}
USE_SYSTEM_SQLITE used to be supported on iOS, and now is a no-op.
This CL removes the references to it from //sql, to avoid confusing
readers.
The CL also breaks down an unnecessarily large unit test in
//sql/connection_unittest.cc that happend to be near the changes in this
CL.
Change-Id: I47c324300b54362529364a5ef26ee45b45fe3b76
Reviewed-on: https://chromium-review.googlesource.com/1137921
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576213}
The feature is used by //components/webdata. This test will help make
sure the feature keeps working as we upgrade SQLite.
Bug: 829893
Change-Id: Ie94439e5abd2cad4e0e15bad92ec3af462850b4c
Reviewed-on: https://chromium-review.googlesource.com/1015206
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552419}
This updates //sql to use base::RepeatingCallback instead of legacy
base::Callback. No intended functional change is included.
This reduces the number of 'base::Bind' in //sql from 19 to 1
as tracked at http://goo.gl/LUVhDj
Bug: 714018
Change-Id: I4b6b321e25f2b3db7ab7ac3e09d9ee74761db230
Reviewed-on: https://chromium-review.googlesource.com/952116
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541388}
We currently use the system-supplied SQLite library on iOS.
This is not the end of the world, because we require iOS 10.0+, which
means SQLite 3.14+, which has the features that Chrome uses. iOS is also
special in that we don't ship Blink, so we don't expose SQLite via
WebSQL -- we control all the other SQL queries we send to SQLite, so
SQLite security bugs aren't easily exploitable.
At the same time, USE_SYSTEM_SQLITE means Chrome for iOS uses an
entirely different SQLite build that most developers don't run. This
makes it more likely that we'll introduce bugs or crashes, and makes it
more difficult to investigate existing bugs/crashes.
In theory, USE_SYSTEM_SQLITE seems appealing on Android and Linux. In
practice, the idea isn't workable on either platform.
* On Android, not shipping our own SQLite would cut down APK size by a few
hundred KB. However, Android doesn't have a great upgrade story before
Lollipop (5.0), and we still support KitKat (4.4). SQLite is exposed
via WebSQL on Android, so we must be in control of SQLite upgrades.
* Linux distributions would like to ship Chromium builds linked against
their SQLite copies. However, https://crbug.com/807487 suggests that
USE_SYSTEM_SQLITE on Linux would result in unacceptable stability. The
root cause of the bug was that NSS can load SQLite on Linux, resulting
in SQLite getting initialized outside our control. Even if we could
fix that bug (via a dependency from //crypto to //sql), we'd still
have to chase all the libraries that might potentially initialize
SQLite. Today we believe that NSS is the only library in that set, but
we simply don't have the bandwidth to justify investigating the bug
reports that will come if we miss something.
In conclusion, there is no platform where USE_SYSTEM_SQLITE is desirable
and viable. So, this CL removes USE_SYSTEM_SQLITE from the codebase.
This will cause the iOS build to increase by a few hundred KB. This
price buys us one less difference between iOS and every other platform,
and increased OS stability in the long run, as iOS bugs / crashes will
be easier to diagnose.
Bug: 22208, 299684, 372584, 807093, 807487
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-device
Change-Id: I1bd64bd93527624e197bd082c725a952c8b3c430
Reviewed-on: https://chromium-review.googlesource.com/898223
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535148}
Chrome currently only supports iOS10 and above. iOS 10 ships with SQLite
3.14.0+, so we can remove some special case handling (and cognitive
burden) needed to support older iOS versions.
Bug: 807093
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-device
Change-Id: I5d19af5eeaffdf587fb43216fc9cf97e9297d15b
Reviewed-on: https://chromium-review.googlesource.com/898564
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534591}
Dot-file locking (which we're currently using on Fuchsia) specifies
"version 1" of VFS. This doesn't include SHM functions, so disable the
mmap-requiring tests.
Bug: 764423
Change-Id: I2bb24f4130bebdb3371aca4ee4169dd573d9cbb1
Reviewed-on: https://chromium-review.googlesource.com/665301
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502096}
SQLite WAL mode by default checkpoints the WAL to the main database at
close time. Chromium closes databases at shutdown, which causes
shutdown slowdowns due to buffered data being flushed. WAL
checkpointing would effectively increase the amount of buffered data
being flushed.
[WAL mode is currently not in use by Chromium.]
BUG=675264
Review-Url: https://codereview.chromium.org/2827673006
Cr-Commit-Position: refs/heads/master@{#465772}
SQLITE_FCNTL_CHUNK_SIZE can advise the VFS to resize files in quantum
amounts, to reduce fragmentation from tiny appends. This change allows
a new PRAGMA auto_vacuum_slack_pages to provide auto_vacuum with a hint
to only rearrange pages when an entire quantum can be released.
BUG=698010
Review-Url: https://codereview.chromium.org/2732553002
Cr-Commit-Position: refs/heads/master@{#459847}
Chromium's SQLite was modified to propagate OSX Time-Machine exclusions
from the main database file to any associated journal files.
Re-implement this using a VFS which wraps the default VFS and makes the
check when opening journal files.
BUG=679941
Review-Url: https://codereview.chromium.org/2623083002
Cr-Commit-Position: refs/heads/master@{#445601}
When the bug was logged, SQLite had a flaw where the regular
expression (Y in X REGEXP Y or REGEXP(Y,X)) effectively had the
compiled results cached with the prepared statement. At some point
Chromium pulled a version of SQLite which fixed this, this test
verifies that the SQLite currently in use has the fix.
BUG=248608
TBR=nyquist@chromium.org
Review-Url: https://chromiumcodereview.appspot.com/2440653002
Cr-Commit-Position: refs/heads/master@{#426398}
The mmap mitigation for bug 537742 meant that the test wasn't running in
cases where mmap could potentially be enabled but wasn't enabled by
default. Change the test to instead run when the platform allows SQLite
mmap to be enabled.
Add a test to verify that mmap cannot be enabled in cases where it is
expected not to work, so that platforms must make an explicit decision
about whether to allow mmap.
BUG=537742, 554269
Review URL: https://codereview.chromium.org/1529693002
Cr-Commit-Position: refs/heads/master@{#365904}
This adds a //sql/mojo library which can be linked into preexisting
sqlite3 code which adds a new VFS which transparently proxies filesystem
usage to the mojo:filesystem application.
We create a new sql_apptests.mojo target, which currently has all the
sql connection_unittests.cc (minus 2 hard ones), all statement and
transaction unit tests and refactor the sql testing stuff so that we
have two implementations of an SQLTestBase class: one that uses files
raw and one that proxies to the filesystem process.
Notably, this patch does not implement file locking, which will have to
be implemented before we can safely use this, but will be a large enough
patch in and of itself that I'm punting on it for this patch.
BUG=493311
Review URL: https://codereview.chromium.org/1176653002
Cr-Commit-Position: refs/heads/master@{#335415}
Long ago, Chromium used fts2 for history full-text search. It was
later replaced by fts3, and even later that feature was deleted
entirely. fts2 is no longer used in the browser at all, so stop
compiling it.
Since SQLite is used by WebSQL, in theory this could affect web
authors, but WebSQL uses an authorizer to allow only specific virtual
table types. fts2 is not one of those types, I have verified manually
that fts2 tables cannot be created using WebSQL.
BUG=455817
Review URL: https://codereview.chromium.org/999573003
Cr-Commit-Position: refs/heads/master@{#320135}
Ran through the import script in third_party/sqlite/README.Chromium,
including the SQLite test suite. There are a few pager errors which
are because of a change required for WebDatabase support (documented
in README).
SQLite changes are at http://www.sqlite.org/changes.html , Chromium
previously used 3.7.6.3.
All patches were applied and the results reviewed to make sure
backported patches were safe to remove, and retained patches were still
covering what was necessary.
Keep fts4 disabled, and also the new fts3 virtual table and unicode61
tokenizer. Once enabled, these are very hard to disable, and there
doesn't seem to be any pressure to enable them. Other SQLITE_* flags
were reviewed for applicability, none looked essential.
Fixes to Chromium:
- In recovery.cc, pk_column now follows the documentation.
- Short garbage files now see SQLITE_NOTADB rather than
SQLITE_IOERR_SHORT_READ.
- Adjust to allow clients to use ScopedErrorIgnore without adding
dependencies.
- More-specific SQLITE_CONSTRAINT_* errors aren't necessary.
- Force recovery test to scan table rather than index.
BUG=340757
TEST=*EVERYTHING* continues to work.
R=michaeln@chromium.org
Review URL: https://codereview.chromium.org/901033002
Cr-Commit-Position: refs/heads/master@{#315646}
This low-level test specifically ensures that foreign key support will not break upon importing a new SQLite version or changing compile-time flags.
BUG=437865
Review URL: https://codereview.chromium.org/856343002
Cr-Commit-Position: refs/heads/master@{#312596}
HAVE_USLEEP affects SQLite's sleep implementation, which is used when
SQLite waits for a lock to clear. Browser databases are generally
uncontended, but this case could happen for WebDatabase in the renderer.
BUG=none
Review URL: https://codereview.chromium.org/827523004
Cr-Commit-Position: refs/heads/master@{#312460}
In working on a new import of SQLite, I found the patch resulting from:
http://codereview.chromium.org/14176
This only touches fts2, not fts3. These tests verify that both versions
work in the way the patch implies.
BUG=340757
Review URL: https://codereview.chromium.org/844443002
Cr-Commit-Position: refs/heads/master@{#310922}
The Google C++ style guide states:
Explicitly annotate overrides of virtual functions or virtual
destructors with an override or (less frequently) final specifier.
Older (pre-C++11) code will use the virtual keyword as an inferior
alternative annotation. For clarity, use exactly one of override,
final, or virtual when declaring an override.
To better conform to these guidelines, the following constructs have
been rewritten:
- if a base class has a virtual destructor, then:
virtual ~Foo(); -> ~Foo() override;
- virtual void Foo() override; -> void Foo() override;
- virtual void Foo() override final; -> void Foo() final;
This patch was automatically generated. The clang plugin can generate
fixit hints, which are suggested edits when it is 100% sure it knows how
to fix a problem. The hints from the clang plugin were applied to the
source tree using the tool in https://codereview.chromium.org/598073004.
Several formatting edits by clang-format were manually reverted, due to
mangling of some of the more complicate IPC macros.
BUG=417463
Review URL: https://codereview.chromium.org/817403002
Cr-Commit-Position: refs/heads/master@{#309480}
(Also delete two dead methods in other files.)
BUG=31877
NOTRY=true
Review URL: https://codereview.chromium.org/218133002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260569 0039d316-1c4b-4281-b951-d872f2087c98
Automated clean up of style checker errors that were missed due to the plugin
not being executed on implementation files.
BUG=115047
Review URL: https://chromiumcodereview.appspot.com/12211033
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@181004 0039d316-1c4b-4281-b951-d872f2087c98
Also add to base namespace.
BUG=
Review URL: https://codereview.chromium.org/11359217
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168281 0039d316-1c4b-4281-b951-d872f2087c98
Patch from Pavel Ivanov <paivanof@gmail.com>
BUG=130498
TEST=On Linux /tmp/SQLStatementTest.db-journal is not left after execution.
Review URL: https://chromiumcodereview.appspot.com/10542096
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141849 0039d316-1c4b-4281-b951-d872f2087c98
Pulls out the core of gbillock's http://codereview.chromium.org/8283002/
- Move NOTREACHED and similar checks into the sql:: implementation code.
- Add malformed SQL checks to Connection::Execute.
- Add SQL-checking convenience methods to Connection.
The general idea is that the sql:: framework assumes valid statements,
rather than having client code contain scattered ad-hoc (and thus
inconsistent) checks.
This version puts back Statement operator overloading and loosy-goosy
Execute() calls to allow other code to be updated in small batches.
R=gbillock@chromium.org,jhawkins@chromium.org,dhollowa@chromium.org
BUG=none
TEST=sql_unittests,unit_tests:*Table*.*
Review URL: http://codereview.chromium.org/8899012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114118 0039d316-1c4b-4281-b951-d872f2087c98
I can't remove app/app.gyp and app/app_base.gypi yet because they are referenced
by third_party gyp files :(
BUG=72317
TEST=None
R=rsesek@chromium.org
move app/sql to sql
Review URL: http://codereview.chromium.org/7353026
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@93069 0039d316-1c4b-4281-b951-d872f2087c98
