Add ArcAuthService to handle IPC request from ARC

Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, dcheng, achuith
BUG=crbug.com/571146

Committed: https://crrev.com/c47a0c11559c83187cd357e6349fa71d8ed80316
Cr-Commit-Position: refs/heads/master@{#368092}

[victorhsieh, 2015-12-29 23:53:07]

Description was changed from

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

BUG=b/26346965
==========

to

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

R=lhchavez, nasko, achuith
BUG=b/26346965
==========

[victorhsieh, 2015-12-29 23:53:08]

victorhsieh@chromium.org changed reviewers:
+ achuith@google.com, lhchavez@lhchavez.com, nasko@chromium.org

[victorhsieh, 2015-12-29 23:54:03]

victorhsieh@chromium.org changed reviewers:
+ victorhsieh@chromium.org

[victorhsieh, 2015-12-29 23:54:04]

PTAL

lhchavez, please do the overall review
nasko, please review the new mojom
achuith, please review chrome/browser/chromeos/

[Luis Héctor Chávez, 2015-12-30 00:02:37]

lhchavez@chromium.org changed reviewers:
+ lhchavez@chromium.org
- lhchavez@lhchavez.com

[Luis Héctor Chávez, 2015-12-30 00:02:37]

https://codereview.chromium.org/1553613002/diff/20001/components/arc.gypi
File components/arc.gypi (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc.gypi#new...
components/arc.gypi:25: 'arc/arc_auth_service.h',
Can these be in arc/auth?

Also, you need to include these files in BUILD.gn as well.

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
File components/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
components/arc/arc_auth_service.cc:18: void
ArcAuthService::StartObservingBridgeServiceChanges() {
Can this be done in the constructor?

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_brid...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_brid...
components/arc/arc_bridge_service.h:155: AuthInstance* auth_instance() { return
auth_ptr_.get(); }
Sort lexicographically please. Same in the rest of the chunks in this file.

https://codereview.chromium.org/1553613002/diff/20001/components/arc/common/a...
File components/arc/common/arc_bridge.mojom (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/common/a...
components/arc/common/arc_bridge.mojom:39: OnAuthInstanceReady@106(AuthInstance
instance_ptr);
[MinVersion=1] (but if https://codereview.chromium.org/1451353002/ or
https://codereview.chromium.org/1495723004/ land first, you'd need to update
accordingly).

Also, sort alphabetically.

[victorhsieh, 2015-12-30 00:32:34]

https://codereview.chromium.org/1553613002/diff/20001/components/arc.gypi
File components/arc.gypi (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc.gypi#new...
components/arc.gypi:25: 'arc/arc_auth_service.h',
On 2015/12/30 00:02:37, Luis Héctor Chávez wrote:
> Can these be in arc/auth?
> 
> Also, you need to include these files in BUILD.gn as well.

Done.

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
File components/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
components/arc/arc_auth_service.cc:18: void
ArcAuthService::StartObservingBridgeServiceChanges() {
On 2015/12/30 00:02:37, Luis Héctor Chávez wrote:
> Can this be done in the constructor?

No, ABS is not available when this is instantiated in
chrome_browser_main_chromeos.cc.

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_brid...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_brid...
components/arc/arc_bridge_service.h:155: AuthInstance* auth_instance() { return
auth_ptr_.get(); }
On 2015/12/30 00:02:37, Luis Héctor Chávez wrote:
> Sort lexicographically please. Same in the rest of the chunks in this file.

Done.

https://codereview.chromium.org/1553613002/diff/20001/components/arc/common/a...
File components/arc/common/arc_bridge.mojom (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/common/a...
components/arc/common/arc_bridge.mojom:39: OnAuthInstanceReady@106(AuthInstance
instance_ptr);
On 2015/12/30 00:02:37, Luis Héctor Chávez wrote:
> [MinVersion=1] (but if https://codereview.chromium.org/1451353002/ or
> https://codereview.chromium.org/1495723004/ land first, you'd need to update
> accordingly).
> 
> Also, sort alphabetically.

Done.

[lhc(google), 2015-12-30 01:06:47]

lhchavez@google.com changed reviewers:
+ lhchavez@google.com

[lhc(google), 2015-12-30 01:06:48]

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
File components/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
components/arc/arc_auth_service.cc:18: void
ArcAuthService::StartObservingBridgeServiceChanges() {
On 2015/12/30 00:32:34, victorhsieh wrote:
> On 2015/12/30 00:02:37, Luis Héctor Chávez wrote:
> > Can this be done in the constructor?
> 
> No, ABS is not available when this is instantiated in
> chrome_browser_main_chromeos.cc.

Right, but does as the code stands there is no need to do so: the whole point of
the ArcServiceManager is to not have to do a chrome/ change every time we add a
new service.

Having said that, if you plan on taking a dependency on content/ or chrome/, you
will have to move the implementation of the ArcAuthService to one of those
directories (see the applist and the settings for examples), so it's better to
do decide that now.

If it can be self-contained, avoid changing chrome_browser_main_chromeos.cc and
have ArcServiceManager instantiate this class and move the logic of registering
to the constructor. If it can't, move this implementation to its final
destination.

[victorhsieh, 2016-01-04 16:36:28]

Description was changed from

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

R=lhchavez, nasko, achuith
BUG=b/26346965
==========

to

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

R=lhchavez, nasko, achuith
BUG=crbug.com/571146
==========

[victorhsieh, 2016-01-04 20:28:43]

Description was changed from

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

R=lhchavez, nasko, achuith
BUG=crbug.com/571146
==========

to

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, nasko, achuith
BUG=crbug.com/571146
==========

[victorhsieh, 2016-01-04 21:20:02]

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
File components/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1553613002/diff/20001/components/arc/arc_auth...
components/arc/arc_auth_service.cc:18: void
ArcAuthService::StartObservingBridgeServiceChanges() {
Since we need to get a profile to store a pref, this has to stay in chrome/. 
The code is now moved.

[victorhsieh, 2016-01-04 21:20:38]

PTAL

Luis, PTAL at the ag/ changes.  Thanks!

[Luis Héctor Chávez, 2016-01-04 21:26:21]

There are some gn changes needed to be synchronized with
chrome/chrome_browser_chromeos.gypi. Mind running

    gn gen //out/Release '--args=target_os="chromeos" use_ozone=true
is_debug=false is_component_build=false use_goma=true symbol_level=1
dcheck_always_on=true' --check

to verify everything is working?

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_auth_service_impl.cc (right):

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:30:
bridge_service->auth_instance()->Init(std::move(host));
#include <utility>

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:116: #include
"components/arc/auth/arc_auth_service.h"
Not needed anymore?

https://codereview.chromium.org/1553613002/diff/60001/chrome/chrome_browser_c...
File chrome/chrome_browser_chromeos.gypi (right):

https://codereview.chromium.org/1553613002/diff/60001/chrome/chrome_browser_c...
chrome/chrome_browser_chromeos.gypi:57:
'browser/chromeos/arc/arc_auth_service_impl.cc',
Lexicographic order?

https://codereview.chromium.org/1553613002/diff/60001/components/arc/auth/arc...
File components/arc/auth/arc_auth_service.h (right):

https://codereview.chromium.org/1553613002/diff/60001/components/arc/auth/arc...
components/arc/auth/arc_auth_service.h:11: public:
virtual ~ArcAuthService()?

[victorhsieh, 2016-01-04 21:31:28]

gn gen now passed.

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_auth_service_impl.cc (right):

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:30:
bridge_service->auth_instance()->Init(std::move(host));
On 2016/01/04 21:26:21, Luis Héctor Chávez wrote:
> #include <utility>

Done.

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

https://codereview.chromium.org/1553613002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:116: #include
"components/arc/auth/arc_auth_service.h"
On 2016/01/04 21:26:21, Luis Héctor Chávez wrote:
> Not needed anymore?

Done.

https://codereview.chromium.org/1553613002/diff/60001/chrome/chrome_browser_c...
File chrome/chrome_browser_chromeos.gypi (right):

https://codereview.chromium.org/1553613002/diff/60001/chrome/chrome_browser_c...
chrome/chrome_browser_chromeos.gypi:57:
'browser/chromeos/arc/arc_auth_service_impl.cc',
On 2016/01/04 21:26:21, Luis Héctor Chávez wrote:
> Lexicographic order?

Done.

https://codereview.chromium.org/1553613002/diff/60001/components/arc/auth/arc...
File components/arc/auth/arc_auth_service.h (right):

https://codereview.chromium.org/1553613002/diff/60001/components/arc/auth/arc...
components/arc/auth/arc_auth_service.h:11: public:
On 2016/01/04 21:26:21, Luis Héctor Chávez wrote:
> virtual ~ArcAuthService()?

Done.

[Luis Héctor Chávez, 2016-01-04 21:47:03]

lgtm

[achuithb, 2016-01-04 23:41:58]

achuith@chromium.org changed reviewers:
+ achuith@chromium.org

[achuithb, 2016-01-04 23:41:59]

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.cc (right):

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:16: DCHECK(bridge_service);
You don't need this DCHECK, the line below crashes if bridge_service is null.

You should just do:
ArcBridgeService::Get()->RemoveObserver(this);

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:23:
bridge_service->AddObserver(this);
ArcBridgeService::Get()->AddObserver(this);

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:32:
bridge_service->auth_instance()->Init(std::move(host));
ArcBridgeService::Get()->auth_instance()->Init(std::move(host));

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:36: // TODO(victorhsieh):
request auth code from LSO.
Please reference a bug here

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.h (right):

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:23: virtual
~ArcAuthServiceImpl();
use override

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:25: // Overrides
ArcBridgeService::Observer
Make everything below private. period at the end of comment please.

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:28: // Overrides AuthHost
period

[victorhsieh, 2016-01-05 00:39:14]

PTAL

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.cc (right):

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:16: DCHECK(bridge_service);
On 2016/01/04 23:41:59, achuithb wrote:
> You don't need this DCHECK, the line below crashes if bridge_service is null.
> 
> You should just do:
> ArcBridgeService::Get()->RemoveObserver(this);

Done.

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:23:
bridge_service->AddObserver(this);
On 2016/01/04 23:41:59, achuithb wrote:
> ArcBridgeService::Get()->AddObserver(this);

Done.

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:32:
bridge_service->auth_instance()->Init(std::move(host));
On 2016/01/04 23:41:59, achuithb wrote:
> ArcBridgeService::Get()->auth_instance()->Init(std::move(host));

Done.

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.cc:36: // TODO(victorhsieh):
request auth code from LSO.
On 2016/01/04 23:41:59, achuithb wrote:
> Please reference a bug here

Done.

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.h (right):

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:23: virtual
~ArcAuthServiceImpl();
On 2016/01/04 23:41:59, achuithb wrote:
> use override

Should I override a destructor?

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:25: // Overrides
ArcBridgeService::Observer
On 2016/01/04 23:41:59, achuithb wrote:
> Make everything below private. period at the end of comment please.

Done.

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:28: // Overrides AuthHost
On 2016/01/04 23:41:59, achuithb wrote:
> period

Done.

[achuithb, 2016-01-05 01:24:54]

lgtm, please get rid of virtual keyword for ~ArcAuthServiceImpl

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.h (right):

https://codereview.chromium.org/1553613002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:23: virtual
~ArcAuthServiceImpl();
On 2016/01/05 00:39:14, victorhsieh wrote:
> On 2016/01/04 23:41:59, achuithb wrote:
> > use override
> 
> Should I override a destructor?

Yes

https://codereview.chromium.org/1553613002/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.h (right):

https://codereview.chromium.org/1553613002/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:23: virtual
~ArcAuthServiceImpl() override;
Drop the virtual. for ex:
https://code.google.com/p/chromium/codesearch#chromium/src/pdf/pdf.h&l=16

[nasko, 2016-01-05 17:26:47]

The change looks good overall, but I'm lacking context on how ARC is designed. I
would suggest poking a reviewer that is already familiar with ARC.

https://codereview.chromium.org/1553613002/diff/120001/components/arc/common/...
File components/arc/common/arc_bridge.mojom (right):

https://codereview.chromium.org/1553613002/diff/120001/components/arc/common/...
components/arc/common/arc_bridge.mojom:24: [MinVersion=1]
OnAuthInstanceReady@106(AuthInstance instance_ptr);
Should methods be defined alphabetically or by the number?

[Luis Héctor Chávez, 2016-01-05 18:13:04]

FWIW dcheng has been doing most of the other reviews for ARC.

https://codereview.chromium.org/1553613002/diff/120001/components/arc/common/...
File components/arc/common/arc_bridge.mojom (right):

https://codereview.chromium.org/1553613002/diff/120001/components/arc/common/...
components/arc/common/arc_bridge.mojom:24: [MinVersion=1]
OnAuthInstanceReady@106(AuthInstance instance_ptr);
On 2016/01/05 17:26:47, nasko wrote:
> Should methods be defined alphabetically or by the number?

Alphabetically, as noted in L16.

[victorhsieh, 2016-01-05 18:49:25]

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

[victorhsieh, 2016-01-05 18:49:26]

dcheng, PTAL at the mojom change.

I also ran `git cl lint/format`.

https://codereview.chromium.org/1553613002/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/arc/arc_auth_service_impl.h (right):

https://codereview.chromium.org/1553613002/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/arc/arc_auth_service_impl.h:23: virtual
~ArcAuthServiceImpl() override;
On 2016/01/05 01:24:53, achuithb wrote:
> Drop the virtual. for ex:
> https://code.google.com/p/chromium/codesearch#chromium/src/pdf/pdf.h&l=16

Done.

[Luis Héctor Chávez, 2016-01-05 20:15:05]

Mind rebasing to ToT and make changes similar to
https://codereview.chromium.org/1548833002/ and
https://codereview.chromium.org/1551763002/ ?

[victorhsieh, 2016-01-05 20:44:37]

Description was changed from

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, nasko, achuith
BUG=crbug.com/571146
==========

to

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, dcheng, achuith
BUG=crbug.com/571146
==========

[victorhsieh, 2016-01-05 20:45:01]

rebased.

[dcheng, 2016-01-06 23:39:13]

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
components/arc/arc_service_manager.cc:30:
arc_auth_service_(std::move(auth_service)),
Why not just instantiate it directly in here? Then you can just make the
constructor take a ArcBridgeService and AddObserver() in the ctor instead of
indirecting through the virtual method.

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
components/arc/arc_service_manager.cc:34: arc_power_bridge_.reset(new
ArcPowerBridge(arc_bridge_service_.get()));
The inconsistency here feels weird: I think there should be a cleanup patch that
just puts these all in the initializer list (and these aren't even set/created
in a consistent way).

[victorhsieh, 2016-01-06 23:52:33]

Thanks!

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
components/arc/arc_service_manager.cc:30:
arc_auth_service_(std::move(auth_service)),
On 2016/01/06 23:39:13, dcheng wrote:
> Why not just instantiate it directly in here? Then you can just make the
> constructor take a ArcBridgeService and AddObserver() in the ctor instead of
> indirecting through the virtual method.

It's just that we will need Profile to do the actual work (token exchange)
later.

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
components/arc/arc_service_manager.cc:34: arc_power_bridge_.reset(new
ArcPowerBridge(arc_bridge_service_.get()));
On 2016/01/06 23:39:13, dcheng wrote:
> The inconsistency here feels weird: I think there should be a cleanup patch
that
> just puts these all in the initializer list (and these aren't even set/created
> in a consistent way).

Done.

[dcheng, 2016-01-07 00:02:33]

lgtm

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
components/arc/arc_service_manager.cc:30:
arc_auth_service_(std::move(auth_service)),
On 2016/01/06 at 23:52:33, victorhsieh wrote:
> On 2016/01/06 23:39:13, dcheng wrote:
> > Why not just instantiate it directly in here? Then you can just make the
> > constructor take a ArcBridgeService and AddObserver() in the ctor instead of
> > indirecting through the virtual method.
> 
> It's just that we will need Profile to do the actual work (token exchange)
later.

I think it would be simpler for ArcAuthService to just inherit
ArcBridgeService::Observer then. The indirection feels funky.

[victorhsieh, 2016-01-07 00:05:53]

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1553613002/diff/160001/components/arc/arc_ser...
components/arc/arc_service_manager.cc:30:
arc_auth_service_(std::move(auth_service)),
On 2016/01/07 00:02:33, dcheng wrote:
> On 2016/01/06 at 23:52:33, victorhsieh wrote:
> > On 2016/01/06 23:39:13, dcheng wrote:
> > > Why not just instantiate it directly in here? Then you can just make the
> > > constructor take a ArcBridgeService and AddObserver() in the ctor instead
of
> > > indirecting through the virtual method.
> > 
> > It's just that we will need Profile to do the actual work (token exchange)
> later.
> 
> I think it would be simpler for ArcAuthService to just inherit
> ArcBridgeService::Observer then. The indirection feels funky.

Yeah, but that would make components depends on chrome, which is not ok :(

[victorhsieh, 2016-01-07 00:06:22]

The CQ bit was checked by victorhsieh@chromium.org

[victorhsieh, 2016-01-07 00:06:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/1553613002/#ps180001
(title: "reorder member variables init")

[commit-bot: I haz the power, 2016-01-07 00:07:15]

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

[commit-bot: I haz the power, 2016-01-07 00:17:22]

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

[commit-bot: I haz the power, 2016-01-07 00:17:23]

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...)

[victorhsieh, 2016-01-07 00:20:40]

victorhsieh@chromium.org changed reviewers:
+ elijahtaylor@chromium.org

[victorhsieh, 2016-01-07 00:20:40]

Elijah, PTAL

[elijahtaylor1, 2016-01-07 02:51:58]

lgtm

[victorhsieh, 2016-01-07 15:35:29]

The CQ bit was checked by victorhsieh@chromium.org

[victorhsieh, 2016-01-07 15:35:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, dcheng@chromium.org, achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/1553613002/#ps200001
(title: " ")

[commit-bot: I haz the power, 2016-01-07 15:36:01]

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

[commit-bot: I haz the power, 2016-01-07 16:57:57]

Description was changed from

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, dcheng, achuith
BUG=crbug.com/571146
==========

to

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, dcheng, achuith
BUG=crbug.com/571146
==========

[commit-bot: I haz the power, 2016-01-07 16:57:58]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-01-07 16:58:51]

Description was changed from

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, dcheng, achuith
BUG=crbug.com/571146
==========

to

==========
Add ArcAuthService to handle IPC request from ARC

This change currently returns a dummy string to ARC.

Corresponding changes: ag/839326, ag/839593

R=lhchavez, dcheng, achuith
BUG=crbug.com/571146

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

[commit-bot: I haz the power, 2016-01-07 16:58:52]

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

[victorhsieh0, 2016-04-20 20:47:31]

Patchset #12 (id:220001) has been deleted