Implement PermissionService::GetNextPermissionChange.

Implement PermissionService::GetNextPermissionChange.

This is adding two methods to start and stop observing permission
changes from content::PermissionManager.

BUG=430238
TBR=gunsch@chromium.org (for chromecast/ boilerplate)

Committed: https://crrev.com/23957a2d3cae89f9107fe028e24c02c80d5b7bfb
Cr-Commit-Position: refs/heads/master@{#323210}

[mlamouri (slow - plz ping), 2015-03-11 11:37:08]

mlamouri@chromium.org changed reviewers:
+ bauerb@chromium.org, jochen@chromium.org, miguelg@chromium.org,
tsepez@chromium.org

[mlamouri (slow - plz ping), 2015-03-11 11:37:08]

mlamouri@chromium.org changed required reviewers:
+ jochen@chromium.org, tsepez@chromium.org

[mlamouri (slow - plz ping), 2015-03-11 11:37:09]

bauerb@, please review changes in:
  chrome/browser/content_settings/permission_context.cc
  chrome/browser/content_settings/permission_context.h
  chrome/browser/content_settings/permission_observer.cc
  chrome/browser/content_settings/permission_observer.h
  chrome/browser/content_settings/permission_observer_factory.cc
  chrome/browser/content_settings/permission_observer_factory.h
  chrome/browser/content_settings/permission_observer_unittest.cc

jochen, please review changes in:
  chrome/browser/content_settings/permission_context.cc
  chrome/browser/content_settings/permission_context.h
  chrome/browser/content_settings/permission_observer.cc
  chrome/browser/content_settings/permission_observer.h
  chrome/browser/content_settings/permission_observer_factory.cc
  chrome/browser/content_settings/permission_observer_factory.h
  chrome/browser/content_settings/permission_observer_unittest.cc
  content/public/browser/content_browser_client.cc
  content/public/browser/content_browser_client.h

tsepez, please review changes in:
  content/common/permission_service.mojom

miguelg, FYI.

Thanks all :)

[mlamouri (slow - plz ping), 2015-03-11 11:38:17]

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context.h (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context.h:17: PermissionContextBase*
Get(Profile* profile,
This could be renamed PermissionContextFactory::Create() to match what it is
actually abstracting.
Also, I would gladly move that one directory up if it's a problem to have
chrome/browser/content_settings/ to be aware of the different
PermissionContextBase factories.

[Bernhard Bauer, 2015-03-11 14:06:18]

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context.h (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context.h:13: namespace
PermissionContext {
Namespaces should be lowercase_with_underscores. Or you could make this a class
with only static methods...

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context.h:17: PermissionContextBase*
Get(Profile* profile,
On 2015/03/11 11:38:17, Mounir Lamouri wrote:
> This could be renamed PermissionContextFactory::Create() to match what it is
> actually abstracting.

I prefer Get() over Create(), as the method doesn't unconditionally create a
permission context. And neither PermissionContext nor PermissionContextFactory
are actually classses, so both would be in the wrong naming style.

> Also, I would gladly move that one directory up if it's a problem to have
> chrome/browser/content_settings/ to be aware of the different
> PermissionContextBase factories.

Meh, as long as there is no conceptual layering violation, I'm fine to keep both
in the same directory until content settings and permissions get separated out
more explicitly.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer.h (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer.h:35: void
OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,
Add // content_settings::Observer implementation.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer_factory.cc (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_factory.cc:27:
BrowserContextDependencyManager::GetInstance()) {
You need to declare your dependencies on other keyed services here.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer_unittest.cc (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:13: class
PermissionObserverTests : public ChromeRenderViewHostTestHarness {
The usual style is to use singular for the test fixture name.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:37: return url_;
If this URL is constant, you could just directly use a constant at call sites.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
File content/browser/permissions/permission_service_impl.cc (right):

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:153: int id =
pending_subscriptions_.Add(subscription);
I don't think you need this map. You could create a scoped_ptr<int> and bind it
into OnPermissionStatusChanged, then set the value to the ID returned by
SubscribePermissionStatusChange. In OnPermissionStatusChanged() you would then
dereference the pointer to get the actual ID.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:155: // If the
embedding_origin is empty, we,ll use the |origin| instead.
Nit: "we'll". Also, move the code here that falls back to the origin if the
embedding origin is empty, so that it's clear what the comment refers to?

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:169: PermissionStatus
PermissionServiceImpl::GetPermissionStatus(
Overloading methods is discouraged by the style guide.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:194: const
mojo::Callback<void(PermissionStatus)>& callback,
Are you going to call the callback?

[jochen (gone - plz use gerrit), 2015-03-11 16:11:24]

https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
content/public/browser/content_browser_client.h:458: virtual int
SubscribePermissionStatusChange(
why is this not on browser_context?

[Tom Sepez, 2015-03-11 17:18:42]

https://codereview.chromium.org/990303002/diff/20001/content/common/permissio...
File content/common/permission_service.mojom (right):

https://codereview.chromium.org/990303002/diff/20001/content/common/permissio...
content/common/permission_service.mojom:31: // conditions, the caller must pass
the last known value.
Doesn't this imply that the service provider might have to buffer a potentially
infinite set of changes since a request for the next one might arrive at any
point in the future?

[mlamouri (slow - plz ping), 2015-03-18 16:24:02]

PTAL

[mlamouri (slow - plz ping), 2015-03-18 16:24:40]

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context.h (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context.h:13: namespace
PermissionContext {
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> Namespaces should be lowercase_with_underscores. Or you could make this a
class with only static methods...

Done.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context.h:17: PermissionContextBase*
Get(Profile* profile,
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> On 2015/03/11 11:38:17, Mounir Lamouri wrote:
> > This could be renamed PermissionContextFactory::Create() to match what it is
> > actually abstracting.
> 
> I prefer Get() over Create(), as the method doesn't unconditionally create a
permission context. And neither PermissionContext nor PermissionContextFactory
are actually classses, so both would be in the wrong naming style.
> 
> > Also, I would gladly move that one directory up if it's a problem to have
> > chrome/browser/content_settings/ to be aware of the different
> > PermissionContextBase factories.
> 
> Meh, as long as there is no conceptual layering violation, I'm fine to keep
both in the same directory until content settings and permissions get separated
out more explicitly.

Sounds good.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer.h (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer.h:35: void
OnContentSettingChanged(const ContentSettingsPattern& primary_pattern,
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> Add // content_settings::Observer implementation.

Done.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer_factory.cc (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_factory.cc:27:
BrowserContextDependencyManager::GetInstance()) {
On 2015/03/11 at 14:06:18, Bernhard (OOO until Mar 23) wrote:
> You need to declare your dependencies on other keyed services here.

I exposed a way to get the PermissionContext related factories in order to setup
the dependencies. Hopefully, it's not too ugly.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer_unittest.cc (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:13: class
PermissionObserverTests : public ChromeRenderViewHostTestHarness {
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> The usual style is to use singular for the test fixture name.

I will not point that permission_context_base_unittest.cc doesn't follow the
style then :)

Fixed.

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:37: return url_;
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> If this URL is constant, you could just directly use a constant at call sites.

I used to do that and I stopped because it was adding more pain when adding a
new test. I've even moved "https://foo.com" as "other_url_". I think reducing
the cost of adding a new unit test is probably a good thing.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
File content/browser/permissions/permission_service_impl.cc (right):

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:153: int id =
pending_subscriptions_.Add(subscription);
On 2015/03/11 at 14:06:18, Bernhard (OOO until Mar 23) wrote:
> I don't think you need this map. You could create a scoped_ptr<int> and bind
it into OnPermissionStatusChanged, then set the value to the ID returned by
SubscribePermissionStatusChange. In OnPermissionStatusChanged() you would then
dereference the pointer to get the actual ID.

Didn't use a scoped_ptr but a raw pointer that is then base::Owned by the
callback.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:155: // If the
embedding_origin is empty, we,ll use the |origin| instead.
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> Nit: "we'll". Also, move the code here that falls back to the origin if the
embedding origin is empty, so that it's clear what the comment refers to?

Done.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:169: PermissionStatus
PermissionServiceImpl::GetPermissionStatus(
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> Overloading methods is discouraged by the style guide.

Snif. I fixed that. GetPermissionStatusFromType() and
GetPermissionStatusFromName() doesn't sound like the best names but if that's
what the style guide wants.

https://codereview.chromium.org/990303002/diff/20001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:194: const
mojo::Callback<void(PermissionStatus)>& callback,
On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> Are you going to call the callback?

Oups :)

https://codereview.chromium.org/990303002/diff/20001/content/common/permissio...
File content/common/permission_service.mojom (right):

https://codereview.chromium.org/990303002/diff/20001/content/common/permissio...
content/common/permission_service.mojom:31: // conditions, the caller must pass
the last known value.
On 2015/03/11 at 17:18:42, Tom Sepez wrote:
> Doesn't this imply that the service provider might have to buffer a
potentially infinite set of changes since a request for the next one might
arrive at any point in the future?

No. It means that the service provider will do a check when it receives the call
and if the value it gets is different from the one being passed, it will
directly run the callback with the new value instead of observing. It will only
observe for changes if the current value is different from the last known value
by the caller.

https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
content/public/browser/content_browser_client.h:458: virtual int
SubscribePermissionStatusChange(
On 2015/03/11 at 16:11:24, jochen (slow) wrote:
> why is this not on browser_context?

All the permission related method live here but I think it would make great
sense to make sense to have a PermissionManager hanging from the BrowserContext
(or move the methods there).

However, it might require some refactoring so it would probably be better in a
follow-up.

[Bernhard Bauer, 2015-03-23 10:20:44]

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer_unittest.cc (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:13: class
PermissionObserverTests : public ChromeRenderViewHostTestHarness {
On 2015/03/18 16:24:40, Mounir Lamouri wrote:
> On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> > The usual style is to use singular for the test fixture name.
> 
> I will not point that permission_context_base_unittest.cc doesn't follow the
> style then :)
> 
> Fixed.

Feel free to complain about that to the person who originally wrote the fixture
class :D

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:37: return url_;
On 2015/03/18 16:24:40, Mounir Lamouri wrote:
> On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> > If this URL is constant, you could just directly use a constant at call
sites.
> 
> I used to do that and I stopped because it was adding more pain when adding a
> new test. I've even moved "https://foo.com" as "other_url_". I think reducing
> the cost of adding a new unit test is probably a good thing.

What additional cost is that?

https://codereview.chromium.org/990303002/diff/40001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_context.h (right):

https://codereview.chromium.org/990303002/diff/40001/chrome/browser/content_s...
chrome/browser/content_settings/permission_context.h:28: };
Add DISALLOW_IMPLICIT_CONSTRUCTORS to prevent this class from being
instantiated.

https://codereview.chromium.org/990303002/diff/40001/content/browser/permissi...
File content/browser/permissions/permission_service_impl.cc (right):

https://codereview.chromium.org/990303002/diff/40001/content/browser/permissi...
content/browser/permissions/permission_service_impl.cc:195: int*
subscription_id,
Could you make this const int*?

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
File content/child/permissions/permission_manager.cc (right):

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.cc:26: // The default statement is
only there to prevent compilation failures if
Uh, if WebPermissionType does in fact get extended without this switch being
updated, I think a compilation failure would be pretty good to draw attention to
that.

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.cc:101: // callback will not leak.
In the case of |this| gets deleted, the
s/In the case of/In case/ (or "In the case of |this| getting deleted").

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.cc:129: // gracefully fail,
destroying the callback too.
I don't understand this comment. The return value of PostTask is not checked.
And which callback is this about? The |callback| variable or the closure bound
below?

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.cc:133: base::Unretained(callback),
Could you pass this base::Owned or in a scoped_ptr?

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
File content/child/permissions/permission_manager.h (right):

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.h:28:
PermissionManager(ServiceRegistry& service_registry);
explicit

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.h:41: protected:
Why protected? Is this class meant to be subclassed?

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.h:51: static void
RunCallbackOnWorkerThread(
You could move this to an anonymous namespace in the .cc file.

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager.h:57: class CallbackInformation {
Can you only forward-declare this class here?

Also, types should go first in each section.

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
File content/child/permissions/permission_manager_thread_proxy.h (right):

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager_thread_proxy.h:23: class
PermissionManagerThreadProxy :
Can you add a comment that explains what this class does?

https://codereview.chromium.org/990303002/diff/40001/content/child/permission...
content/child/permissions/permission_manager_thread_proxy.h:24: public
blink::WebPermissionClient,
Does this not fit on the previous line anymore?

[mlamouri (slow - plz ping), 2015-03-23 18:06:50]

PTAL.

Revision 3 was included a WIP version of a CL that has landed since. That branch
wasn't fully rebased.

[Bernhard Bauer, 2015-03-23 18:26:18]

LGTM

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
File chrome/browser/content_settings/permission_observer_unittest.cc (right):

https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
chrome/browser/content_settings/permission_observer_unittest.cc:37: return url_;
On 2015/03/23 10:20:43, Bernhard Bauer wrote:
> On 2015/03/18 16:24:40, Mounir Lamouri wrote:
> > On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> > > If this URL is constant, you could just directly use a constant at call
> sites.
> > 
> > I used to do that and I stopped because it was adding more pain when adding
a
> > new test. I've even moved "https://foo.com" as "other_url_". I think
reducing
> > the cost of adding a new unit test is probably a good thing.
> 
> What additional cost is that?

Ping?

[mlamouri (slow - plz ping), 2015-03-23 18:39:36]

On 2015/03/23 at 18:26:18, bauerb wrote:
> LGTM
> 
>
https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
> File chrome/browser/content_settings/permission_observer_unittest.cc (right):
> 
>
https://codereview.chromium.org/990303002/diff/20001/chrome/browser/content_s...
> chrome/browser/content_settings/permission_observer_unittest.cc:37: return
url_;
> On 2015/03/23 10:20:43, Bernhard Bauer wrote:
> > On 2015/03/18 16:24:40, Mounir Lamouri wrote:
> > > On 2015/03/11 at 14:06:18, Bernhard Bauer wrote:
> > > > If this URL is constant, you could just directly use a constant at call
> > sites.
> > > 
> > > I used to do that and I stopped because it was adding more pain when
adding a
> > > new test. I've even moved "https://foo.com" as "other_url_". I think
reducing
> > > the cost of adding a new unit test is probably a good thing.
> > 
> > What additional cost is that?
> 
> Ping?

Sorry for missing that. The cost is about creating a GURL and if there is an
"other url", making sure it's not the same. It's mostly about reducing the
number of lines you have to write in order to have a test done.
Also, having url() and other_url() will make it easier to refactor some bits if
needed.

[jochen (gone - plz use gerrit), 2015-03-24 09:16:35]

On 2015/03/18 at 16:24:40, mlamouri wrote:

>
https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
> File content/public/browser/content_browser_client.h (right):
> 
>
https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
> content/public/browser/content_browser_client.h:458: virtual int
SubscribePermissionStatusChange(
> On 2015/03/11 at 16:11:24, jochen (slow) wrote:
> > why is this not on browser_context?
> 
> All the permission related method live here but I think it would make great
sense to make sense to have a PermissionManager hanging from the BrowserContext
(or move the methods there).
> 
> However, it might require some refactoring so it would probably be better in a
follow-up.

The Content.*Client classes are meant as a last resort fallback. I'd rather not
add more stuff to them. If you expect the permission manager to live on the UI
thread, it should hang of browser context, and if it's supposed to live on the
IO thread, it should hang of resource context.

[mlamouri (slow - plz ping), 2015-03-25 19:45:29]

On 2015/03/24 at 09:16:35, jochen wrote:
> On 2015/03/18 at 16:24:40, mlamouri wrote:
> 
> >
https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
> > File content/public/browser/content_browser_client.h (right):
> > 
> >
https://codereview.chromium.org/990303002/diff/20001/content/public/browser/c...
> > content/public/browser/content_browser_client.h:458: virtual int
SubscribePermissionStatusChange(
> > On 2015/03/11 at 16:11:24, jochen (slow) wrote:
> > > why is this not on browser_context?
> > 
> > All the permission related method live here but I think it would make great
sense to make sense to have a PermissionManager hanging from the BrowserContext
(or move the methods there).
> > 
> > However, it might require some refactoring so it would probably be better in
a follow-up.
> 
> The Content.*Client classes are meant as a last resort fallback. I'd rather
not add more stuff to them. If you expect the permission manager to live on the
UI thread, it should hang of browser context, and if it's supposed to live on
the IO thread, it should hang of resource context.

I wrote that:
https://codereview.chromium.org/1011953003

Will get back to this CL later.

[mlamouri (slow - plz ping), 2015-03-29 21:58:56]

Updated to extend content::PermissionManager interface.

PTAL.

[jochen (gone - plz use gerrit), 2015-03-31 10:22:16]

lgtm

[Tom Sepez, 2015-03-31 16:24:15]

lgtm

[mlamouri (slow - plz ping), 2015-03-31 21:01:19]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-03-31 21:01:19]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
jochen@chromium.org, tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/990303002/#ps140001
(title: "cleanup")

[commit-bot: I haz the power, 2015-03-31 21:02:02]

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

[mlamouri (slow - plz ping), 2015-03-31 21:24:34]

Patchset #8 (id:140001) has been deleted

[mlamouri (slow - plz ping), 2015-03-31 21:28:54]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-03-31 21:28:54]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
jochen@chromium.org, tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/990303002/#ps160001
(title: "nits")

[commit-bot: I haz the power, 2015-03-31 21:30:09]

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

[commit-bot: I haz the power, 2015-03-31 21:42:02]

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

[commit-bot: I haz the power, 2015-03-31 21:42:03]

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

[gunsch, 2015-03-31 23:08:58]

On 2015/03/31 21:42:03, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

FYI, before you submit, I just noticed the new chromecast trybot (not yet
blocking) is about to fail if you submit this:

http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_andr...

could you fix in src/chromecast/ as well?

[mlamouri (slow - plz ping), 2015-04-01 09:40:34]

mlamouri@chromium.org changed reviewers:
+ torne@chromium.org

[mlamouri (slow - plz ping), 2015-04-01 09:40:35]

chromecast/ updated.

torne@, could you PTAL at android_webview/ changes?

[Torne, 2015-04-01 10:14:31]

android_webview LGTM

[mlamouri (slow - plz ping), 2015-04-01 10:30:34]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-04-01 10:30:35]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
jochen@chromium.org, tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/990303002/#ps180001
(title: "fix chromecast/")

[commit-bot: I haz the power, 2015-04-01 10:31:02]

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

[commit-bot: I haz the power, 2015-04-01 10:38:14]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2015-04-01 10:38:43]

Patchset 9 (id:??) landed as
https://crrev.com/23957a2d3cae89f9107fe028e24c02c80d5b7bfb
Cr-Commit-Position: refs/heads/master@{#323210}