[Webview, Child Accounts] Automatically sign in to Chrome if needed.

[Webview, Child Accounts] Automatically sign in to Chrome if needed.

Webview gets its URL filtering information from Chrome; however
it can't do so until Chrome has signed in, which in a child account
environment it does automatically on first run.

This CL checks for the case where Chrome has never been run,
and attempts to force sign in Chrome. If it fails (as far as
I know this should only happen if GMSCore is out of date) it
generates a message asking the user to start Chrome.

BUG=686732
Review-Url: https://codereview.chromium.org/2695113002
Cr-Commit-Position: refs/heads/master@{#450961}
Committed: https://chromium.googlesource.com/chromium/src/+/5fdb85e384dd972552e22622350d12ff8492c686

[aberent, 2017-02-15 14:10:25]

aberent@chromium.org changed reviewers:
+ bauerb@chromium.org

[aberent, 2017-02-15 14:10:26]

bauerb@ - please review.

Not yet tested end-to-end due to Unicorn sync problems etc.

[Bernhard Bauer, 2017-02-15 15:09:38]

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/childaccounts/ChildAccountService.java
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/childaccounts/ChildAccountService.java:65:
public static void listenForStatusChange(Runnable callback) {
Javadoc for public methods. In particular, when and how often is the callback
going to be called?

You could also have this method take a Callback<Boolean> which gets the child
account status, which would have the additional advantage that you can directly
call run() on the Callback from C++ without having to go through an additional
method here.

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:64:
private static ChromeBrowserInitializer sChromeBrowserInitiliazer;
I'm going to resist the temptation of digging out who misspelled initializer
here... but could you fix the typo please? :)

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:445:
* @param initializer
If you add a param, document it.

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java:63:
synchronized (sContentProviderLock) {
Maybe add a comment that this potentially holds the lock for a long time while
we start the browser, but we only do that from a background thread?

https://codereview.chromium.org/2695113002/diff/1/chrome/browser/supervised_u...
File
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc:34:
base::Bind(&Java_ChildAccountService_childStatusReceived, env,
Don't hold on to the JNIEnv* by binding it into the callback. Instead use a
callback to a static method and get the JNIEnv there by attaching to the current
thread.

https://codereview.chromium.org/2695113002/diff/1/components/supervised_user_...
File components/supervised_user_error_page/supervised_user_error_page.h (right):

https://codereview.chromium.org/2695113002/diff/1/components/supervised_user_...
components/supervised_user_error_page/supervised_user_error_page.h:14: // needs
to match.
We can actually create a Java enum (well, @IntDef) from the C++ one.

[aberent, 2017-02-15 18:34:30]

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

[aberent, 2017-02-15 18:34:46]

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/childaccounts/ChildAccountService.java
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/childaccounts/ChildAccountService.java:65:
public static void listenForStatusChange(Runnable callback) {
On 2017/02/15 15:09:37, Bernhard Bauer wrote:
> Javadoc for public methods. In particular, when and how often is the callback
> going to be called?
> 
> You could also have this method take a Callback<Boolean> which gets the child
> account status, which would have the additional advantage that you can
directly
> call run() on the Callback from C++ without having to go through an additional
> method here.

Done.

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:64:
private static ChromeBrowserInitializer sChromeBrowserInitiliazer;
On 2017/02/15 15:09:37, Bernhard Bauer wrote:
> I'm going to resist the temptation of digging out who misspelled initializer
> here... but could you fix the typo please? :)

Done.

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java:445:
* @param initializer
On 2017/02/15 15:09:37, Bernhard Bauer wrote:
> If you add a param, document it.

Done.

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProvider.java:63:
synchronized (sContentProviderLock) {
On 2017/02/15 15:09:37, Bernhard Bauer wrote:
> Maybe add a comment that this potentially holds the lock for a long time while
> we start the browser, but we only do that from a background thread?

Done.

https://codereview.chromium.org/2695113002/diff/1/chrome/browser/supervised_u...
File
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc
(right):

https://codereview.chromium.org/2695113002/diff/1/chrome/browser/supervised_u...
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc:34:
base::Bind(&Java_ChildAccountService_childStatusReceived, env,
On 2017/02/15 15:09:38, Bernhard Bauer wrote:
> Don't hold on to the JNIEnv* by binding it into the callback. Instead use a
> callback to a static method and get the JNIEnv there by attaching to the
current
> thread.

Actually switched to binding RunCallbackAndroid, which doesn't need either a
JNIEnv or a new static method.

https://codereview.chromium.org/2695113002/diff/1/components/supervised_user_...
File components/supervised_user_error_page/supervised_user_error_page.h (right):

https://codereview.chromium.org/2695113002/diff/1/components/supervised_user_...
components/supervised_user_error_page/supervised_user_error_page.h:14: // needs
to match.
On 2017/02/15 15:09:38, Bernhard Bauer wrote:
> We can actually create a Java enum (well, @IntDef) from the C++ one.

Done.

[commit-bot: I haz the power, 2017-02-15 18:36:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bernhard Bauer, 2017-02-15 18:39:45]

Description was changed from

==========
[Webview, Unicorn] Automatically sign in to Chrome if needed.

Webview gets its URL filtering information from Chrome; however
it can't do so until Chrome has signed in, which in a Unicorn
environment it does automatically on first run.

This CL checks for the case where Chrome has never been run,
and attempts to force sign in Chrome. If it fails (as far as
I know this should only happen if GMSCore is out of date) it
generates a message asking the kid to start Chrome.

BUG=686732
==========

to

==========
[Webview, Child Accounts] Automatically sign in to Chrome if needed.

Webview gets its URL filtering information from Chrome; however
it can't do so until Chrome has signed in, which in a child account
environment it does automatically on first run.

This CL checks for the case where Chrome has never been run,
and attempts to force sign in Chrome. If it fails (as far as
I know this should only happen if GMSCore is out of date) it
generates a message asking the user to start Chrome.

BUG=686732
==========

[Bernhard Bauer, 2017-02-15 18:40:09]

LGTM with a nit:

https://codereview.chromium.org/2695113002/diff/20001/chrome/browser/supervis...
File
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc
(right):

https://codereview.chromium.org/2695113002/diff/20001/chrome/browser/supervis...
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc:38:
// TODO(http://crbug/692591) Should use base::BindOnce, but
Nit: Use HTTPS, and crbug.com :D

[commit-bot: I haz the power, 2017-02-15 18:44:37]

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

[commit-bot: I haz the power, 2017-02-15 18:44:38]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[aberent, 2017-02-15 18:56:25]

https://codereview.chromium.org/2695113002/diff/20001/chrome/browser/supervis...
File
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc
(right):

https://codereview.chromium.org/2695113002/diff/20001/chrome/browser/supervis...
chrome/browser/supervised_user/child_accounts/child_account_service_android.cc:38:
// TODO(http://crbug/692591) Should use base::BindOnce, but
On 2017/02/15 18:40:09, Bernhard Bauer wrote:
> Nit: Use HTTPS, and http://crbug.com :D

Done.

[aberent, 2017-02-15 18:56:33]

The CQ bit was checked by aberent@chromium.org

[aberent, 2017-02-15 18:56:33]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2695113002/#ps40001
(title: "Fix nit and non-Android builds")

[commit-bot: I haz the power, 2017-02-15 18:57:47]

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, 2017-02-15 21:38:49]

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

[commit-bot: I haz the power, 2017-02-15 21:38:50]

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

[aberent, 2017-02-16 11:01:02]

The CQ bit was checked by aberent@chromium.org

[aberent, 2017-02-16 11:01:03]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2695113002/#ps60001
(title: "Fix instrumentation tests")

[commit-bot: I haz the power, 2017-02-16 11:01:53]

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, 2017-02-16 12:16:36]

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

[commit-bot: I haz the power, 2017-02-16 12:16:37]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[aberent, 2017-02-16 12:40:27]

The CQ bit was checked by aberent@chromium.org

[commit-bot: I haz the power, 2017-02-16 12:40:39]

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, 2017-02-16 14:53:04]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487248827582500,
"parent_rev": "2df25dd0beaa6ff0750ad1bae330e3d9ff84a9c0", "commit_rev":
"5fdb85e384dd972552e22622350d12ff8492c686"}

[commit-bot: I haz the power, 2017-02-16 14:53:49]

Description was changed from

==========
[Webview, Child Accounts] Automatically sign in to Chrome if needed.

Webview gets its URL filtering information from Chrome; however
it can't do so until Chrome has signed in, which in a child account
environment it does automatically on first run.

This CL checks for the case where Chrome has never been run,
and attempts to force sign in Chrome. If it fails (as far as
I know this should only happen if GMSCore is out of date) it
generates a message asking the user to start Chrome.

BUG=686732
==========

to

==========
[Webview, Child Accounts] Automatically sign in to Chrome if needed.

Webview gets its URL filtering information from Chrome; however
it can't do so until Chrome has signed in, which in a child account
environment it does automatically on first run.

This CL checks for the case where Chrome has never been run,
and attempts to force sign in Chrome. If it fails (as far as
I know this should only happen if GMSCore is out of date) it
generates a message asking the user to start Chrome.

BUG=686732

Review-Url: https://codereview.chromium.org/2695113002
Cr-Commit-Position: refs/heads/master@{#450961}
Committed:
https://chromium.googlesource.com/chromium/src/+/5fdb85e384dd972552e22622350d...
==========

[commit-bot: I haz the power, 2017-02-16 14:53:51]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/5fdb85e384dd972552e22622350d...