Add handle waiting to StashBackend.

Add handle waiting to StashBackend.

This change adds support for monitoring stashed handles for being
readable. This will be used to relaunch suspended extension processes
when a persistent service sends a message or data to its client in the
suspended extension process.

BUG=389016
Committed: https://crrev.com/cf7d082ec44c27d7842da839aaf629a9648e0462
Cr-Commit-Position: refs/heads/master@{#314989}

[Sam McNally, 2014-10-29 02:34:13]

sammc@chromium.org changed reviewers:
+ raymes@chromium.org

[Sam McNally, 2015-01-29 03:51:30]

Patchset #2 (id:20001) has been deleted

[Sam McNally, 2015-01-29 03:56:07]

sammc@chromium.org changed reviewers:
+ amistry@chromium.org
- raymes@chromium.org

[Sam McNally, 2015-01-29 03:56:07]

+amistry

[Anand Mistry (off Chromium), 2015-02-05 11:10:33]

lgtm

As-is, I can't see any issues with the implementation (modulo a few minor nits).
However, without seeing this in use, I can't tell if it's exactly what you want.
It would be nice to get a second opinion. Maybe Ken, since he's also doing
extension stuff.

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
File extensions/browser/mojo/stash_backend.cc (right):

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:60: // Construct an StashEntry for
|stashed_object|. If |on_handle_readable| is
s/an/a

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
File extensions/browser/mojo/stash_backend_unittest.cc (right):

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend_unittest.cc:14: // Create a data pipe,
write some data to the proucer handle and return the
proucer

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend_unittest.cc:187:
mojo::InterfacePtr<StashService> stash_service;
stash_service and stash_service_ is confusing. I has to re-read this test a few
times before I understood what was going on.

[Sam McNally, 2015-02-05 23:31:30]

New patchsets have been uploaded after l-g-t-m from amistry@chromium.org

[Sam McNally, 2015-02-05 23:35:30]

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

[Sam McNally, 2015-02-05 23:35:31]

+rockot

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
File extensions/browser/mojo/stash_backend.cc (right):

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:60: // Construct an StashEntry for
|stashed_object|. If |on_handle_readable| is
On 2015/02/05 11:10:33, Anand Mistry wrote:
> s/an/a

Done.

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
File extensions/browser/mojo/stash_backend_unittest.cc (right):

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend_unittest.cc:14: // Create a data pipe,
write some data to the proucer handle and return the
On 2015/02/05 11:10:33, Anand Mistry wrote:
> proucer

Done.

https://codereview.chromium.org/648853007/diff/40001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend_unittest.cc:187:
mojo::InterfacePtr<StashService> stash_service;
On 2015/02/05 11:10:33, Anand Mistry wrote:
> stash_service and stash_service_ is confusing. I has to re-read this test a
few
> times before I understood what was going on.

Done.

[Ken Rockot(use gerrit already), 2015-02-06 04:21:27]

neat. just a few comments but lgtm

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
File extensions/browser/mojo/stash_backend.cc (right):

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:14: class StashServiceImpl : public
mojo::InterfaceImpl<StashService> {
Maybe nix the InterfaceImpl base and just implement StashService?

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:78:
std::vector<linked_ptr<mojo::AsyncWaiter>> waiters_;
Maybe use a ScopedVector for the waiters instead?

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:98:
stashed_objects_.push_back(linked_ptr<StashEntry>(new StashEntry(
Seems like stashed_objects_ could also be a ScopedVector instead, yeah?

I'm not too picky about it, but these seem like easy cases since you're only
pushing or clearing.

[Sam McNally, 2015-02-06 05:40:25]

New patchsets have been uploaded after l-g-t-m from rockot@chromium.org

[Sam McNally, 2015-02-06 05:48:23]

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
File extensions/browser/mojo/stash_backend.cc (right):

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:14: class StashServiceImpl : public
mojo::InterfaceImpl<StashService> {
On 2015/02/06 04:21:27, Ken Rockot wrote:
> Maybe nix the InterfaceImpl base and just implement StashService?

Done.

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:78:
std::vector<linked_ptr<mojo::AsyncWaiter>> waiters_;
On 2015/02/06 04:21:27, Ken Rockot wrote:
> Maybe use a ScopedVector for the waiters instead?

Done.

https://codereview.chromium.org/648853007/diff/60001/extensions/browser/mojo/...
extensions/browser/mojo/stash_backend.cc:98:
stashed_objects_.push_back(linked_ptr<StashEntry>(new StashEntry(
On 2015/02/06 04:21:27, Ken Rockot wrote:
> Seems like stashed_objects_ could also be a ScopedVector instead, yeah?
> 
> I'm not too picky about it, but these seem like easy cases since you're only
> pushing or clearing.

Done.

[Sam McNally, 2015-02-06 05:48:31]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2015-02-06 05:49:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/648853007/80001

[commit-bot: I haz the power, 2015-02-06 06:46:57]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-02-06 06:47:47]

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