[sql] Test mmap operation based on SQLite capabilities.

[sql] Test mmap operation based on SQLite capabilities.

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
Committed: https://crrev.com/53adf16ef292bc721a65b47628b1a3008b502363
Cr-Commit-Position: refs/heads/master@{#365904}

[Scott Hess - ex-Googler, 2015-12-15 19:05:46]

shess@chromium.org changed reviewers:
+ vichang@chromium.org

[Scott Hess - ex-Googler, 2015-12-15 19:05:46]

vichang@, with this change setting SQLITE_MAX_MMAP_SIZE=0 in sqlite.gyp (and
corresponding BUILD.gn) will do the right thing.  That's the preferred way to
disable because then it can't be accidentally enabled.

erg@, previously Mojo skipped the test in this area.  I have verified that this
test works right with a VFS which works like MojoVFS, but I have been unable to
run mojo:sql_apptest for reasons beyond my ability to analyze (it doesn't seem
related to this change at least).  So FYI, I guess.

https://codereview.chromium.org/1529693002/diff/20001/sql/sqlite_features_uni...
File sql/sqlite_features_unittest.cc (right):

https://codereview.chromium.org/1529693002/diff/20001/sql/sqlite_features_uni...
sql/sqlite_features_unittest.cc:192: db().Close();
From here this code is just a move from connection_unittest.cc.

[vichang1, 2015-12-16 03:16:04]

On 2015/12/15 19:05:46, Scott Hess wrote:
> vichang@, with this change setting SQLITE_MAX_MMAP_SIZE=0 in sqlite.gyp (and
> corresponding BUILD.gn) will do the right thing.  That's the preferred way to
> disable because then it can't be accidentally enabled.
> 
> erg@, previously Mojo skipped the test in this area.  I have verified that
this
> test works right with a VFS which works like MojoVFS, but I have been unable
to
> run mojo:sql_apptest for reasons beyond my ability to analyze (it doesn't seem
> related to this change at least).  So FYI, I guess.
> 
>
https://codereview.chromium.org/1529693002/diff/20001/sql/sqlite_features_uni...
> File sql/sqlite_features_unittest.cc (right):
> 
>
https://codereview.chromium.org/1529693002/diff/20001/sql/sqlite_features_uni...
> sql/sqlite_features_unittest.cc:192: db().Close();
> From here this code is just a move from connection_unittest.cc.

Thanks for notifying me. I believe someone else should review the code, as I am
not familiar the code base here. 

One question. In which part of release/daily process does this unit test run? I
am not aware that this test runs on my build target. (Sorry, I am from Android
team)

[Scott Hess - ex-Googler, 2015-12-16 06:56:18]

On 2015/12/16 03:16:04, vichang1 wrote:
> One question. In which part of release/daily process does this unit test run?
I
> am not aware that this test runs on my build target. (Sorry, I am from Android
> team)

sql_unittests.  I have no idea what you mean by "release/daily process", these
tests run constantly as part of the Chromium commit-queue and trybot
infrastructure for the supported platforms (Win, Linux, OSX, iOS, Android,
ChromeOS).

[Scott Hess - ex-Googler, 2015-12-16 06:57:31]

On 2015/12/16 03:16:04, vichang1 wrote:
> Thanks for notifying me. I believe someone else should review the code, as I
am
> not familiar the code base here. 

Elliot, would you mind particularly to review?  There really aren't that many
people familiar with the code base, and as long as I'm FYI'ing you because of
Mojo, I'm already bothering you.

[Scott Hess - ex-Googler, 2015-12-16 06:57:55]

Description was changed from

==========
[sql] Test mmap operation based on SQLite capabilities.

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
==========

to

==========
[sql] Test mmap operation based on SQLite capabilities.

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
==========

[Scott Hess - ex-Googler, 2015-12-16 06:57:55]

shess@chromium.org changed reviewers:
+ erg@chromium.org
- vichang@chromium.org

[Elliot Glaysher, 2015-12-17 21:15:49]

lgtm

[Scott Hess - ex-Googler, 2015-12-17 21:19:43]

The CQ bit was checked by shess@chromium.org

[Scott Hess - ex-Googler, 2015-12-17 21:19:51]

On 2015/12/17 21:15:49, Elliot Glaysher wrote:
> lgtm

Thanks!

[commit-bot: I haz the power, 2015-12-17 21:21:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1529693002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1529693002/20001

[commit-bot: I haz the power, 2015-12-17 22:07:33]

Description was changed from

==========
[sql] Test mmap operation based on SQLite capabilities.

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
==========

to

==========
[sql] Test mmap operation based on SQLite capabilities.

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
==========

[commit-bot: I haz the power, 2015-12-17 22:07:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-12-17 22:08:16]

Description was changed from

==========
[sql] Test mmap operation based on SQLite capabilities.

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
==========

to

==========
[sql] Test mmap operation based on SQLite capabilities.

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

Committed: https://crrev.com/53adf16ef292bc721a65b47628b1a3008b502363
Cr-Commit-Position: refs/heads/master@{#365904}
==========

[commit-bot: I haz the power, 2015-12-17 22:08:17]

Patchset 2 (id:??) landed as
https://crrev.com/53adf16ef292bc721a65b47628b1a3008b502363
Cr-Commit-Position: refs/heads/master@{#365904}