arc: Rename methods in ArcSessionManager::Observer.

arc: Rename methods in ArcSessionManager::Observer.

Rename OnOptInEnabled to OnArcOptInChanged (to make it
clearer that it's also called on opt-out), and add "Arc" to
other method names as well to make their meaning clearer
when implemented by non-ARC-specific classes.

BUG=633243
Committed: https://crrev.com/75a477e7e2b795f9fbbac132ed610affb07e5300
Cr-Commit-Position: refs/heads/master@{#438920}

[Daniel Erat, 2016-12-15 16:21:07]

derat@chromium.org changed reviewers:
+ hidehiko@chromium.org, lhchavez@chromium.org, xiyuan@chromium.org

[Daniel Erat, 2016-12-15 16:21:08]

please take a look. thanks!

xiyuan: chrome/browser/ui/app_list/
hidehiko or lhchavez: the rest

[Daniel Erat, 2016-12-15 16:21:13]

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

[commit-bot: I haz the power, 2016-12-15 16:21:35]

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-15 16:48:03]

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

[commit-bot: I haz the power, 2016-12-15 16:48:04]

Dry run: This issue passed the CQ dry run.

[xiyuan, 2016-12-15 17:09:22]

c/b/ui/app_list/ lgtm

[Luis Héctor Chávez, 2016-12-15 18:10:39]

lgtm with a nit.

https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.h (right):

https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.h:106: virtual void
OnArcShutdownBridge() {}
nit: the other methods use OnArcNounVerb() pattern. Can you rename this to
OnArcBridgeShutdown()?

[Daniel Erat, 2016-12-15 18:15:33]

https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.h (right):

https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.h:106: virtual void
OnArcShutdownBridge() {}
On 2016/12/15 18:10:39, Luis Héctor Chávez wrote:
> nit: the other methods use OnArcNounVerb() pattern. Can you rename this to
> OnArcBridgeShutdown()?

sure. thoughts about going with "OnArcBridgeShutDown" instead? "shut down" is
the verb form, while "shutdown" is the noun, although chrome incorrectly uses
"shutdown" as a verb in a bunch of places. OnArcInitialStart also seems to be
ending with a noun, though, so i'm okay with keeping "shutdown" here too.

[Luis Héctor Chávez, 2016-12-15 18:21:51]

https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
File chrome/browser/chromeos/arc/arc_session_manager.h (right):

https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
chrome/browser/chromeos/arc/arc_session_manager.h:106: virtual void
OnArcShutdownBridge() {}
On 2016/12/15 18:15:33, Daniel Erat wrote:
> On 2016/12/15 18:10:39, Luis Héctor Chávez wrote:
> > nit: the other methods use OnArcNounVerb() pattern. Can you rename this to
> > OnArcBridgeShutdown()?
> 
> sure. thoughts about going with "OnArcBridgeShutDown" instead? "shut down" is
> the verb form, while "shutdown" is the noun, although chrome incorrectly uses
> "shutdown" as a verb in a bunch of places. OnArcInitialStart also seems to be
> ending with a noun, though, so i'm okay with keeping "shutdown" here too.

Yeah, I was hesitant to add the comment about OnArcInitialStart because there
wasn't a name that was clearly better without sounding too forced :/

I agree with you that ShutDown is more correct, bue given that there are 402
instances of Shutdown in chrome/ and only 2 of ShutDown, this would look out of
place if we went with the latter (kind of like with HTTP's Referer).

[Daniel Erat, 2016-12-15 18:32:32]

On 2016/12/15 18:21:51, Luis Héctor Chávez wrote:
>
https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
> File chrome/browser/chromeos/arc/arc_session_manager.h (right):
> 
>
https://codereview.chromium.org/2577103002/diff/1/chrome/browser/chromeos/arc...
> chrome/browser/chromeos/arc/arc_session_manager.h:106: virtual void
> OnArcShutdownBridge() {}
> On 2016/12/15 18:15:33, Daniel Erat wrote:
> > On 2016/12/15 18:10:39, Luis Héctor Chávez wrote:
> > > nit: the other methods use OnArcNounVerb() pattern. Can you rename this to
> > > OnArcBridgeShutdown()?
> > 
> > sure. thoughts about going with "OnArcBridgeShutDown" instead? "shut down"
is
> > the verb form, while "shutdown" is the noun, although chrome incorrectly
uses
> > "shutdown" as a verb in a bunch of places. OnArcInitialStart also seems to
be
> > ending with a noun, though, so i'm okay with keeping "shutdown" here too.
> 
> Yeah, I was hesitant to add the comment about OnArcInitialStart because there
> wasn't a name that was clearly better without sounding too forced :/
> 
> I agree with you that ShutDown is more correct, bue given that there are 402
> instances of Shutdown in chrome/ and only 2 of ShutDown, this would look out
of
> place if we went with the latter (kind of like with HTTP's Referer).

sure, makes sense. i've gone with OnArcBridgeShutdown.

[Daniel Erat, 2016-12-15 18:32:40]

The CQ bit was checked by derat@chromium.org

[Daniel Erat, 2016-12-15 18:32:41]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
lhchavez@chromium.org
Link to the patchset: https://codereview.chromium.org/2577103002/#ps20001
(title: "OnArcShutdownBridge -> OnArcBridgeShutdown")

[commit-bot: I haz the power, 2016-12-15 18:33:13]

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-15 20:52:41]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481826760665180,
"parent_rev": "2beb1bb493f1510f9af17f737f3a6eee51bd976e", "commit_rev":
"562331a081e9295d3f16ea35d57b767981fa7f9e"}

[commit-bot: I haz the power, 2016-12-15 20:53:06]

Description was changed from

==========
arc: Rename methods in ArcSessionManager::Observer.

Rename OnOptInEnabled to OnArcOptInChanged (to make it
clearer that it's also called on opt-out), and add "Arc" to
other method names as well to make their meaning clearer
when implemented by non-ARC-specific classes.

BUG=633243
==========

to

==========
arc: Rename methods in ArcSessionManager::Observer.

Rename OnOptInEnabled to OnArcOptInChanged (to make it
clearer that it's also called on opt-out), and add "Arc" to
other method names as well to make their meaning clearer
when implemented by non-ARC-specific classes.

BUG=633243

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

[commit-bot: I haz the power, 2016-12-15 20:53:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-15 20:54:33]

Description was changed from

==========
arc: Rename methods in ArcSessionManager::Observer.

Rename OnOptInEnabled to OnArcOptInChanged (to make it
clearer that it's also called on opt-out), and add "Arc" to
other method names as well to make their meaning clearer
when implemented by non-ARC-specific classes.

BUG=633243

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

to

==========
arc: Rename methods in ArcSessionManager::Observer.

Rename OnOptInEnabled to OnArcOptInChanged (to make it
clearer that it's also called on opt-out), and add "Arc" to
other method names as well to make their meaning clearer
when implemented by non-ARC-specific classes.

BUG=633243

Committed: https://crrev.com/75a477e7e2b795f9fbbac132ed610affb07e5300
Cr-Commit-Position: refs/heads/master@{#438920}
==========

[commit-bot: I haz the power, 2016-12-15 20:54:35]

Patchset 2 (id:??) landed as
https://crrev.com/75a477e7e2b795f9fbbac132ed610affb07e5300
Cr-Commit-Position: refs/heads/master@{#438920}