Make LevelDBObserver an associated interface to LevelDBWrapper.

Make LevelDBObserver an associated interface to LevelDBWrapper.

LocalStorageCachedArea relies on an accurate interleaving of observations and
replies to LevelDBWrapper method calls, so make observers associated interfaces
for LevelDBWrapper.

BUG=676078
BUG=586194
Committed: https://crrev.com/e80c2a1472bf8e7118e454bb8383d6046620007a
Cr-Commit-Position: refs/heads/master@{#440039}

[Marijn Kruisselbrink, 2016-12-20 21:38:00]

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

[commit-bot: I haz the power, 2016-12-20 21:38:35]

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-20 21:57:03]

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

[Marijn Kruisselbrink, 2016-12-20 21:57:04]

Another option would have been to make both LevelDBWrapper and LevelDBOverver be
passed as associated interfaces of StoragePartitionService, but this seemed
cleaner/more sensible.

[commit-bot: I haz the power, 2016-12-20 22:52:39]

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

[commit-bot: I haz the power, 2016-12-20 22:52:40]

Dry run: This issue passed the CQ dry run.

[jam, 2016-12-20 23:23:36]

lgtm

Is it possible to write tests that fail when the pipes aren't associated? This
would prevent regressions, and also allow us to see why this is needed (I trust
you that it is, but it's not obvious to me what the bugs are).

[Marijn Kruisselbrink, 2016-12-21 01:43:40]

On 2016/12/20 at 23:23:36, jam wrote:
> lgtm
> 
> Is it possible to write tests that fail when the pipes aren't associated? This
would prevent regressions, and also allow us to see why this is needed (I trust
you that it is, but it's not obvious to me what the bugs are).

Not sure how exactly I would test that. In
https://codereview.chromium.org/2588273005/ I'm working on adding (very basic)
tests to at least test that the correct observations are made at the correct
time. I suppose that if someone naively changes this back to a non-associated
interface there is at least some chance that those test will start failing (but
of course they couldn't make such a change without also modifying the test
code). Note that leveldb_wrapper.mojom already said "Note that observer methods
are called before the callbacks for the LevelDBWrapper methods are run.", which
of course made no sense when they were on separate pipes.
In general the problem is that LocalStorageCachedArea sets/clears flags when
calling LevelDBWrapper methods to decide which observations to ignore. But with
no ordering guarantees that had the potential for race conditions.

[Marijn Kruisselbrink, 2016-12-21 01:43:44]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2016-12-21 01:44:05]

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-21 01:55:49]

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

[commit-bot: I haz the power, 2016-12-21 01:55:50]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Marijn Kruisselbrink, 2016-12-21 02:01:26]

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

[Marijn Kruisselbrink, 2016-12-21 02:01:26]

+dcheng for mojom owners

[dcheng, 2016-12-21 05:13:42]

mojo lgtm

[Marijn Kruisselbrink, 2016-12-21 06:09:04]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2016-12-21 06:09:19]

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-21 06:13:31]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1482300544369990, "parent_rev":
"7e887c0eb8ecebae641b12afdd744e03e2687020", "commit_rev":
"677f9934f1eb507ab2415f7f19a4fbbe952033aa"}

[commit-bot: I haz the power, 2016-12-21 06:14:00]

Description was changed from

==========
Make LevelDBObserver an associated interface to LevelDBWrapper.

LocalStorageCachedArea relies on an accurate interleaving of observations and
replies to LevelDBWrapper method calls, so make observers associated interfaces
for LevelDBWrapper.

BUG=676078
BUG=586194
==========

to

==========
Make LevelDBObserver an associated interface to LevelDBWrapper.

LocalStorageCachedArea relies on an accurate interleaving of observations and
replies to LevelDBWrapper method calls, so make observers associated interfaces
for LevelDBWrapper.

BUG=676078
BUG=586194

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

[commit-bot: I haz the power, 2016-12-21 06:14:01]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-21 06:16:25]

Description was changed from

==========
Make LevelDBObserver an associated interface to LevelDBWrapper.

LocalStorageCachedArea relies on an accurate interleaving of observations and
replies to LevelDBWrapper method calls, so make observers associated interfaces
for LevelDBWrapper.

BUG=676078
BUG=586194

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

to

==========
Make LevelDBObserver an associated interface to LevelDBWrapper.

LocalStorageCachedArea relies on an accurate interleaving of observations and
replies to LevelDBWrapper method calls, so make observers associated interfaces
for LevelDBWrapper.

BUG=676078
BUG=586194

Committed: https://crrev.com/e80c2a1472bf8e7118e454bb8383d6046620007a
Cr-Commit-Position: refs/heads/master@{#440039}
==========

[commit-bot: I haz the power, 2016-12-21 06:16:28]

Patchset 1 (id:??) landed as
https://crrev.com/e80c2a1472bf8e7118e454bb8383d6046620007a
Cr-Commit-Position: refs/heads/master@{#440039}