Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194
Committed: https://crrev.com/38d9efa1374c0cb319450f965d09f4ac83dc4a09
Cr-Commit-Position: refs/heads/master@{#440886}

[Marijn Kruisselbrink, 2016-12-22 23:12:43]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 23:13:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-22 23:17:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-22 23:17:22]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Marijn Kruisselbrink, 2016-12-22 23:23:57]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 23:24:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-22 23:48:46]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-22 23:49:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-22 23:52:07]

mek@chromium.org changed reviewers:
+ jam@chromium.org

[Marijn Kruisselbrink, 2016-12-22 23:52:08]

This is my attempt to add some unit tests that test how LocalStorageContextMojo
behaves when it is using the actual file service. The good part, the tests seem
to work (and found some bugs). The bad part, they cause content_unittests to
quit on exit, with a stack trace like:

Received signal 11 <unknown> 000000000000
#0 0x7fd885d2372e base::debug::StackTrace::StackTrace()
#1 0x7fd885d2326f base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7fd889101330 <unknown>
#3 0x7fd87bc43cd6
_ZN4base8internal13FunctorTraitsIMN4mojo3edk14NodeControllerEFvvEvE6InvokeIPS4_JEEEvS6_OT_DpOT0_
#4 0x7fd87bc72071
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN4mojo3edk15ProcessDelegateEFvvEJPS6_EEEvOT_DpOT0_
#5 0x7fd87bc72017
_ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk15ProcessDelegateEFvvEJNS0_17UnretainedWrapperIS5_EEEEEFvvEE7RunImplIRKS7_RKSt5tupleIJS9_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#6 0x7fd87bc71f5c
_ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk15ProcessDelegateEFvvEJNS0_17UnretainedWrapperIS5_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#7 0x7fd87bc3ef6b base::internal::RunMixin<>::Run()
#8 0x7fd87bc3185b mojo::edk::NodeController::AttemptShutdownIfRequested()
#9 0x7fd87bc3156a mojo::edk::NodeController::RequestShutdown()
#10 0x7fd87bc03a75 mojo::edk::Core::RequestShutdown()
#11 0x7fd87bc718af mojo::edk::ShutdownIPCSupport()
#12 0x00000218953d mojo::edk::test::ScopedIPCSupport::~ScopedIPCSupport()
#13 0x000002189719 mojo::edk::test::ScopedIPCSupport::~ScopedIPCSupport()
#14 0x0000007059bf std::default_delete<>::operator()()
#15 0x0000017664cc std::unique_ptr<>::reset()
#16 0x000001766599 std::unique_ptr<>::~unique_ptr()
#17 0x0000017663cf main
#18 0x7fd874bc3f45 __libc_start_main
#19 0x0000006837e5 <unknown>

So apparently/possibly there is something incompatible between how
content_unittests sets up mojo, and how service_manager::test::ServiceTest
expects things to be setup. But well, I really have no idea how all the low
level mojo bits work. Also it's very possible that I should just not be using
ServiceTest in content_unittests at all (or that the way I'm using it is wrong).
But the tests themselves do seem to actually work, so I don't know.

[commit-bot: I haz the power, 2016-12-23 00:19:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-23 00:19:44]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[jam, 2016-12-27 18:23:20]

Description was changed from

==========
WIP: Add tests for how LocalStorageContextMojo interacts with file/leveldb
services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194
==========

to

==========
WIP: Add tests for how LocalStorageContextMojo interacts with file/leveldb
services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194
==========

[jam, 2016-12-27 18:23:20]

jam@chromium.org changed reviewers:
+ rockot@chromium.org

[jam, 2016-12-27 18:23:37]

On 2016/12/22 23:52:08, Marijn Kruisselbrink wrote:
> This is my attempt to add some unit tests that test how
LocalStorageContextMojo
> behaves when it is using the actual file service. The good part, the tests
seem
> to work (and found some bugs). The bad part, they cause content_unittests to
> quit on exit, with a stack trace like:
> 
> Received signal 11 <unknown> 000000000000
> #0 0x7fd885d2372e base::debug::StackTrace::StackTrace()
> #1 0x7fd885d2326f base::debug::(anonymous namespace)::StackDumpSignalHandler()
> #2 0x7fd889101330 <unknown>
> #3 0x7fd87bc43cd6
>
_ZN4base8internal13FunctorTraitsIMN4mojo3edk14NodeControllerEFvvEvE6InvokeIPS4_JEEEvS6_OT_DpOT0_
> #4 0x7fd87bc72071
>
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN4mojo3edk15ProcessDelegateEFvvEJPS6_EEEvOT_DpOT0_
> #5 0x7fd87bc72017
>
_ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk15ProcessDelegateEFvvEJNS0_17UnretainedWrapperIS5_EEEEEFvvEE7RunImplIRKS7_RKSt5tupleIJS9_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
> #6 0x7fd87bc71f5c
>
_ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk15ProcessDelegateEFvvEJNS0_17UnretainedWrapperIS5_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
> #7 0x7fd87bc3ef6b base::internal::RunMixin<>::Run()
> #8 0x7fd87bc3185b mojo::edk::NodeController::AttemptShutdownIfRequested()
> #9 0x7fd87bc3156a mojo::edk::NodeController::RequestShutdown()
> #10 0x7fd87bc03a75 mojo::edk::Core::RequestShutdown()
> #11 0x7fd87bc718af mojo::edk::ShutdownIPCSupport()
> #12 0x00000218953d mojo::edk::test::ScopedIPCSupport::~ScopedIPCSupport()
> #13 0x000002189719 mojo::edk::test::ScopedIPCSupport::~ScopedIPCSupport()
> #14 0x0000007059bf std::default_delete<>::operator()()
> #15 0x0000017664cc std::unique_ptr<>::reset()
> #16 0x000001766599 std::unique_ptr<>::~unique_ptr()
> #17 0x0000017663cf main
> #18 0x7fd874bc3f45 __libc_start_main
> #19 0x0000006837e5 <unknown>
> 
> So apparently/possibly there is something incompatible between how
> content_unittests sets up mojo, and how service_manager::test::ServiceTest
> expects things to be setup. But well, I really have no idea how all the low
> level mojo bits work. Also it's very possible that I should just not be using
> ServiceTest in content_unittests at all (or that the way I'm using it is
wrong).
> But the tests themselves do seem to actually work, so I don't know.

+Ken for the test harness issue

[Ken Rockot(use gerrit already), 2016-12-27 18:25:36]

On 2016/12/27 at 18:23:37, jam wrote:
> On 2016/12/22 23:52:08, Marijn Kruisselbrink wrote:
> > This is my attempt to add some unit tests that test how
LocalStorageContextMojo
> > behaves when it is using the actual file service. The good part, the tests
seem
> > to work (and found some bugs). The bad part, they cause content_unittests to
> > quit on exit, with a stack trace like:
> > 
> > Received signal 11 <unknown> 000000000000
> > #0 0x7fd885d2372e base::debug::StackTrace::StackTrace()
> > #1 0x7fd885d2326f base::debug::(anonymous
namespace)::StackDumpSignalHandler()
> > #2 0x7fd889101330 <unknown>
> > #3 0x7fd87bc43cd6
> >
_ZN4base8internal13FunctorTraitsIMN4mojo3edk14NodeControllerEFvvEvE6InvokeIPS4_JEEEvS6_OT_DpOT0_
> > #4 0x7fd87bc72071
> >
_ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN4mojo3edk15ProcessDelegateEFvvEJPS6_EEEvOT_DpOT0_
> > #5 0x7fd87bc72017
> >
_ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk15ProcessDelegateEFvvEJNS0_17UnretainedWrapperIS5_EEEEEFvvEE7RunImplIRKS7_RKSt5tupleIJS9_EEJLm0EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
> > #6 0x7fd87bc71f5c
> >
_ZN4base8internal7InvokerINS0_9BindStateIMN4mojo3edk15ProcessDelegateEFvvEJNS0_17UnretainedWrapperIS5_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
> > #7 0x7fd87bc3ef6b base::internal::RunMixin<>::Run()
> > #8 0x7fd87bc3185b mojo::edk::NodeController::AttemptShutdownIfRequested()
> > #9 0x7fd87bc3156a mojo::edk::NodeController::RequestShutdown()
> > #10 0x7fd87bc03a75 mojo::edk::Core::RequestShutdown()
> > #11 0x7fd87bc718af mojo::edk::ShutdownIPCSupport()
> > #12 0x00000218953d mojo::edk::test::ScopedIPCSupport::~ScopedIPCSupport()
> > #13 0x000002189719 mojo::edk::test::ScopedIPCSupport::~ScopedIPCSupport()
> > #14 0x0000007059bf std::default_delete<>::operator()()
> > #15 0x0000017664cc std::unique_ptr<>::reset()
> > #16 0x000001766599 std::unique_ptr<>::~unique_ptr()
> > #17 0x0000017663cf main
> > #18 0x7fd874bc3f45 __libc_start_main
> > #19 0x0000006837e5 <unknown>
> > 
> > So apparently/possibly there is something incompatible between how
> > content_unittests sets up mojo, and how service_manager::test::ServiceTest
> > expects things to be setup. But well, I really have no idea how all the low
> > level mojo bits work. Also it's very possible that I should just not be
using
> > ServiceTest in content_unittests at all (or that the way I'm using it is
wrong).
> > But the tests themselves do seem to actually work, so I don't know.
> 
> +Ken for the test harness issue

ServiceTest (via BackgroundServiceManager) always initializes and shutdown the
EDK, but the content_unittests test runner also does this.
BackgroundServiceManager::InitParams provides a mechanism to prevent this
behavior, but it needs to be plumbed through to ServiceTest::SetUp.

[Ken Rockot(use gerrit already), 2016-12-27 18:44:25]

Otherwise ServiceTest usage LG

[Marijn Kruisselbrink, 2016-12-27 18:44:36]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 18:44:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-27 18:48:17]

Description was changed from

==========
WIP: Add tests for how LocalStorageContextMojo interacts with file/leveldb
services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194
==========

to

==========
Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194
==========

[Marijn Kruisselbrink, 2016-12-27 18:49:29]

On 2016/12/27 at 18:25:36, rockot wrote:
> On 2016/12/27 at 18:23:37, jam wrote:

> > +Ken for the test harness issue
> 
> ServiceTest (via BackgroundServiceManager) always initializes and shutdown the
EDK, but the content_unittests test runner also does this.
BackgroundServiceManager::InitParams provides a mechanism to prevent this
behavior, but it needs to be plumbed through to ServiceTest::SetUp.

Ah yes, that did the trick. Plumbed it through by adding a init_edk parameter
(defaulting to true) to the ServiceTest constructor.

[commit-bot: I haz the power, 2016-12-27 19:22:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 19:22:48]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Marijn Kruisselbrink, 2016-12-27 19:33:36]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 19:33:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-27 20:19:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 20:19:39]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Marijn Kruisselbrink, 2016-12-27 22:22:29]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 22:22:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-27 23:01:52]

and when https://codereview.chromium.org/2604843002 and
https://codereview.chromium.org/2600153003 (or similar fixes for those issues)
land, I believe this should be green on all bots

[commit-bot: I haz the power, 2016-12-28 00:09:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 00:09:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2016-12-28 05:47:54]

lgtm

[Marijn Kruisselbrink, 2016-12-28 17:18:25]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2016-12-28 17:18:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-28 17:22:16]

The CQ bit was unchecked by mek@chromium.org

[Marijn Kruisselbrink, 2016-12-28 17:27:31]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 17:27:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 17:29:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 17:29:06]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Marijn Kruisselbrink, 2016-12-28 17:30:27]

Patchset #7 (id:120001) has been deleted

[Marijn Kruisselbrink, 2016-12-28 17:37:03]

Patchset #7 (id:140001) has been deleted

[Marijn Kruisselbrink, 2016-12-28 17:39:30]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 17:39:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-28 17:50:15]

The CQ bit was unchecked by mek@chromium.org

[Marijn Kruisselbrink, 2016-12-28 17:50:19]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-12-28 17:50:20]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2597983003/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-28 17:50:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-28 19:13:16]

The CQ bit was unchecked by mek@chromium.org

[Marijn Kruisselbrink, 2016-12-28 19:15:54]

The CQ bit was checked by mek@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-28 19:16:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Marijn Kruisselbrink, 2016-12-28 20:05:44]

The CQ bit was unchecked by mek@chromium.org

[Marijn Kruisselbrink, 2016-12-28 20:05:48]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-12-28 20:05:48]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2597983003/#ps180001
(title: "reapply change that got lost during rebase")

[commit-bot: I haz the power, 2016-12-28 20:06:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 20:21:36]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1482955548132970,
"parent_rev": "74a98b1b47bac7b856e4713b0fa8456a64d3c91a", "commit_rev":
"eeda0a91b96741217756081575f808a068e3cd49"}

[commit-bot: I haz the power, 2016-12-28 20:22:03]

Description was changed from

==========
Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194
==========

to

==========
Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194

Review-Url: https://codereview.chromium.org/2597983003
==========

[commit-bot: I haz the power, 2016-12-28 20:22:04]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2017-01-02 15:50:05]

Description was changed from

==========
Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194

Review-Url: https://codereview.chromium.org/2597983003
==========

to

==========
Add tests for how LocalStorageContextMojo interacts with file/leveldb services.

Also fixes bugs exposed by those tests:
- when database does not exists it needs to be created.
- when database can't be opened null is passed to LevelDBWrapper.

BUG=586194

Committed: https://crrev.com/38d9efa1374c0cb319450f965d09f4ac83dc4a09
Cr-Commit-Position: refs/heads/master@{#440886}
==========

[commit-bot: I haz the power, 2017-01-02 15:50:06]

Patchset 8 (id:??) landed as
https://crrev.com/38d9efa1374c0cb319450f965d09f4ac83dc4a09
Cr-Commit-Position: refs/heads/master@{#440886}