arc: Prepare ArcAuthEndpoint flag for finch experiment.

arc: Prepare ArcAuthEndpoint flag for finch experiment.

We would like to test silent Arc opt-in flow via finch experiment
and this CL updates switch that acivates silent auth workflow.

TEST=Manually on device using --enable-features=ArcUseAuthEndpoint
     and switch in chrome://flags. Plase note, for test I used
     www-googleapis-staging.sandbox.google.com instead of
     www.googleapis.com that should be availble on Nov 17.
BUG=b/28987931

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

[khmel, 2016-11-16 21:50:13]

khmel@chromium.org changed reviewers:
+ lhchavez@chromium.org, rkaplow@chromium.org

[khmel, 2016-11-16 21:50:14]

PTAL

[khmel, 2016-11-16 21:52:00]

Adding asvitkine@ in case  rkaplow@ is unable to review.

PTAL

[Luis Héctor Chávez, 2016-11-16 22:11:29]

lgtm with nits

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features.cc
File components/arc/arc_features.cc (right):

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features...
components/arc/arc_features.cc:16: const base::Feature
kArcUseAuthEndpointFeature {
Same here, keep alphabetized.

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features.h
File components/arc/arc_features.h (right):

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features...
components/arc/arc_features.h:15: extern const base::Feature
kArcUseAuthEndpointFeature;
Please add "// Please keep alphabetized." (and keep them alphabetized).

[khmel, 2016-11-16 22:22:13]

khmel@chromium.org changed reviewers:
+ asvitkine@chromium.org

[khmel, 2016-11-16 22:22:14]

Thank you Luis!

+asvitkine@ in case  rkaplow@ is unable to review.

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features.cc
File components/arc/arc_features.cc (right):

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features...
components/arc/arc_features.cc:16: const base::Feature
kArcUseAuthEndpointFeature {
On 2016/11/16 22:11:29, Luis Héctor Chávez wrote:
> Same here, keep alphabetized.

Done.

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features.h
File components/arc/arc_features.h (right):

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features...
components/arc/arc_features.h:15: extern const base::Feature
kArcUseAuthEndpointFeature;
On 2016/11/16 22:11:29, Luis Héctor Chávez wrote:
> Please add "// Please keep alphabetized." (and keep them alphabetized).

Done.

[khmel, 2016-11-16 22:22:16]

Thank you Luis!

+asvitkine@ in case  rkaplow@ is unable to review.

[Luis Héctor Chávez, 2016-11-16 22:24:58]

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features.h
File components/arc/arc_features.h (right):

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features...
components/arc/arc_features.h:15: extern const base::Feature
kArcUseAuthEndpointFeature;
On 2016/11/16 22:22:14, khmel wrote:
> On 2016/11/16 22:11:29, Luis Héctor Chávez wrote:
> > Please add "// Please keep alphabetized." (and keep them alphabetized).
> 
> Done.

Can you please also add the comment so any other person that wants to modify
this list knows this should be sorted alphabetically?

[khmel, 2016-11-17 18:45:37]

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features.h
File components/arc/arc_features.h (right):

https://codereview.chromium.org/2508713003/diff/1/components/arc/arc_features...
components/arc/arc_features.h:15: extern const base::Feature
kArcUseAuthEndpointFeature;
On 2016/11/16 22:24:58, Luis Héctor Chávez wrote:
> On 2016/11/16 22:22:14, khmel wrote:
> > On 2016/11/16 22:11:29, Luis Héctor Chávez wrote:
> > > Please add "// Please keep alphabetized." (and keep them alphabetized).
> > 
> > Done.
> 
> Can you please also add the comment so any other person that wants to modify
> this list knows this should be sorted alphabetically?

Sorry, missed you comment

[khmel, 2016-11-17 21:53:00]

khmel@chromium.org changed reviewers:
+ isherman@chromium.org

[khmel, 2016-11-17 21:53:00]

+ isherman@

I have not got response to review histograms.xml
PTAL

[Alexei Svitkine (slow), 2016-11-17 22:56:46]

lgtm % comment below

sorry for the slow reply, rkaplow@ and I had all day meetings yesterday and on a
plane today

https://codereview.chromium.org/2508713003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2508713003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:91590: -  <int value="1458583431"
label="arc-use-auth-endpoint"/>
Please keep this line as it's needed to decode data from old versions.

[khmel, 2016-11-17 23:48:26]

Thank you a lot Alexei for your review! And sorry for this rush. We have a plan
to land this before M57 branch.

https://codereview.chromium.org/2508713003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/2508713003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:91590: -  <int value="1458583431"
label="arc-use-auth-endpoint"/>
On 2016/11/17 22:56:46, Alexei Svitkine (very slow) wrote:
> Please keep this line as it's needed to decode data from old versions.

Good point, thanks!

[khmel, 2016-11-17 23:48:37]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-11-17 23:48:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2508713003/#ps60001
(title: "restore old value in histograms")

[commit-bot: I haz the power, 2016-11-17 23:50:29]

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-11-18 01:55:41]

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

[commit-bot: I haz the power, 2016-11-18 01:55:42]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[khmel, 2016-11-18 01:56:15]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-11-18 01:57:51]

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-11-18 04:34:26]

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

[commit-bot: I haz the power, 2016-11-18 04:34:28]

Failed to apply patch for chrome/browser/chromeos/arc/arc_auth_service.cc:
While running git apply --index -p1;
  error: patch failed: chrome/browser/chromeos/arc/arc_auth_service.cc:401
  error: chrome/browser/chromeos/arc/arc_auth_service.cc: patch does not apply

Patch:       chrome/browser/chromeos/arc/arc_auth_service.cc
Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc
b/chrome/browser/chromeos/arc/arc_auth_service.cc
index
37ffd78319836671ddfdb8ccfae8c5033f296c97..7d756ed728070a8f1ca271b4540c68150e7d9e65
100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -39,6 +39,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager_client.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_features.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync_preferences/pref_service_syncable.h"
@@ -401,21 +402,13 @@ void ArcAuthService::RequestAccountInfoInternal(
     return;
   }
 
-  // If endpoint is passed via command line flag, use automatic auth code
-  // fetching.
-  const base::CommandLine* command_line =
-      base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(chromeos::switches::kArcUseAuthEndpoint)) {
-    std::string auth_endpoint = command_line->GetSwitchValueASCII(
-        chromeos::switches::kArcUseAuthEndpoint);
-    if (!auth_endpoint.empty()) {
-      DCHECK(!auth_code_fetcher_);
-      auth_code_fetcher_ = base::MakeUnique<ArcAuthCodeFetcher>(
-          profile_, context_.get(), auth_endpoint);
-      auth_code_fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
-                                           weak_ptr_factory_.GetWeakPtr()));
-      return;
-    }
+  if (base::FeatureList::IsEnabled(arc::kArcUseAuthEndpointFeature)) {
+    DCHECK(!auth_code_fetcher_);
+    auth_code_fetcher_ =
+        base::MakeUnique<ArcAuthCodeFetcher>(profile_, context_.get());
+    auth_code_fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    return;
   }
 
   // Otherwise, show LSO page to user, and let them click "Sign in" button.

[khmel, 2016-11-18 04:39:54]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-11-18 04:39:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2508713003/#ps80001
(title: "rebase2")

[commit-bot: I haz the power, 2016-11-18 04:40:21]

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-11-18 06:18:10]

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

[commit-bot: I haz the power, 2016-11-18 06:18:12]

Failed to apply patch for chrome/browser/chromeos/arc/arc_auth_service.cc:
While running git apply --index -p1;
  error: patch failed: chrome/browser/chromeos/arc/arc_auth_service.cc:39
  error: chrome/browser/chromeos/arc/arc_auth_service.cc: patch does not apply

Patch:       chrome/browser/chromeos/arc/arc_auth_service.cc
Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc
b/chrome/browser/chromeos/arc/arc_auth_service.cc
index
a325ce282d0a29038df34535bb169dfd347b1698..71f1ca85665dc23ddc58cdb55f826ba2a2cc79a0
100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -39,6 +39,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/session_manager_client.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/arc/arc_features.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync_preferences/pref_service_syncable.h"
@@ -401,21 +402,13 @@ void ArcAuthService::RequestAccountInfoInternal(
     return;
   }
 
-  // If endpoint is passed via command line flag, use automatic auth code
-  // fetching.
-  const base::CommandLine* command_line =
-      base::CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(chromeos::switches::kArcUseAuthEndpoint)) {
-    std::string auth_endpoint = command_line->GetSwitchValueASCII(
-        chromeos::switches::kArcUseAuthEndpoint);
-    if (!auth_endpoint.empty()) {
-      DCHECK(!auth_code_fetcher_);
-      auth_code_fetcher_ = base::MakeUnique<ArcAuthCodeFetcher>(
-          profile_, context_.get(), auth_endpoint);
-      auth_code_fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
-                                           weak_ptr_factory_.GetWeakPtr()));
-      return;
-    }
+  if (base::FeatureList::IsEnabled(arc::kArcUseAuthEndpointFeature)) {
+    DCHECK(!auth_code_fetcher_);
+    auth_code_fetcher_ =
+        base::MakeUnique<ArcAuthCodeFetcher>(profile_, context_.get());
+    auth_code_fetcher_->Fetch(base::Bind(&ArcAuthService::OnAuthCodeFetched,
+                                         weak_ptr_factory_.GetWeakPtr()));
+    return;
   }
 
   // Otherwise, show LSO page to user after HTTP context preparation, and let

[khmel, 2016-11-18 06:33:59]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-11-18 06:34:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2508713003/#ps100001
(title: "rebase3")

[commit-bot: I haz the power, 2016-11-18 06:34:22]

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-11-18 06:54:20]

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

[commit-bot: I haz the power, 2016-11-18 06:54:22]

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

[khmel, 2016-11-18 07:29:09]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-11-18 07:29:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2508713003/#ps140001
(title: "fix rebase glitch")

[commit-bot: I haz the power, 2016-11-18 07:29:25]

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-11-18 08:48:04]

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

[commit-bot: I haz the power, 2016-11-18 08:48:07]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[khmel, 2016-11-18 08:48:47]

The CQ bit was checked by khmel@chromium.org

[commit-bot: I haz the power, 2016-11-18 08:48:59]

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-11-18 10:08:07]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-11-18 10:09:38]

Description was changed from

==========
arc: Prepare ArcAuthEndpoint flag for finch experiment.

We would like to test silent Arc opt-in flow via finch experiment
and this CL updates switch that acivates silent auth workflow.

TEST=Manually on device using --enable-features=ArcUseAuthEndpoint
     and switch in chrome://flags. Plase note, for test I used
     www-googleapis-staging.sandbox.google.com instead of
     www.googleapis.com that should be availble on Nov 17.
BUG=b/28987931
==========

to

==========
arc: Prepare ArcAuthEndpoint flag for finch experiment.

We would like to test silent Arc opt-in flow via finch experiment
and this CL updates switch that acivates silent auth workflow.

TEST=Manually on device using --enable-features=ArcUseAuthEndpoint
     and switch in chrome://flags. Plase note, for test I used
     www-googleapis-staging.sandbox.google.com instead of
     www.googleapis.com that should be availble on Nov 17.
BUG=b/28987931

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

[commit-bot: I haz the power, 2016-11-18 10:09:39]

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