For the captive portal network, if the password is not known to the user. It will block logging-in to the device, and a reboot is required. 

Please upload the logs as well.

The issue is seen on R69 as well (Nami, image 10895.11.0).
Will upload the feedback report shortly.

Can you try latest M69 to check if the shill sandboxing revert in 10895.13.0 fixes this?

So this means a user has to log in to an account on the device to be able to log into the captive portal?

The guest user is not able to log in?

I am not sure the screen shots explain it, the screen shot for 70 looks like the captive portal loaded, whereas normally we don't load the captive portal?

For a user already added to the device, the CP overlay should pop up only after logging into the same user again. 

Also when the CP overlay loads in R70, I am not able to add new user/login as a guest. The overlay wont close either.


@Feedback report,
https://listnr.corp.google.com/report/85588089640

The shill sandboxing revert seems to have fixed this issue. I do not see this on 10895.13.0.

Should this be considered fixed then?

I am going to assign this to mortonm@ and mark it as fixed given that the revert of shill sandboxing seems to have fixed the issue.

jmuppala@ - please test/verify this on the next m70 dev and m69 beta qual builds. We should also test/verify this after the shill sandboxing feature relands in M70.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.

I'm confused: this CL landed in 10895.14.0, not .13.0:

https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1161128

Or is crosland wrong?

10895.13.0 started building on 08/02 and https://chromium-review.googlesource.com/c/aosp/platform/system/connectivity/shill/+/1161128 didn't land until the next morning. So yes, the CL landed in .14.0 not .13.0

@#15, indeed...

jmuppala@ - did you specifically test this on 10895.13.0 or a different newer build after .13?

Yes. works fine on 10895.13.0 and 10895.16.0.

Seeing the issue consistently on 10895.11.0

Then its not related to shill sandboxing and nothing else stands out to me from the 10895.11.0 to 10895.13.0 diff... https://crosland.corp.google.com/log/10895.11.0..10895.13.0


Chrome version did change with these 2 builds, perhaps something there fixed it? Diff here 
https://chromium.googlesource.com/chromium/src/+log/69.0.3497.24..69.0.3497.26?n=10000

+ Jacob and Zach, who probably know lock screen better than most on this bug

s/lock/login/

seeing this issue in Nocturne on latest M70, 10975.0.0, 70.0.3542.2.

Is this still seen on m69?

^ To add more context, we disabled views-based login on m69. I expect this is a views-login regression.

yes. seeing it now in M69 (10895.20.0) as well.

10895.20.0, 69.0.3497.33

I don't see any login changes that could have affected this, but there are some changes in captive_portal specific code (ie, [1], [2]).

1: https://chromium.googlesource.com/chromium/src/+/1ba11572999c90d234c5e53fe7decb4bf7db1cf9
2: https://chromium.googlesource.com/chromium/src/+/fbc6125c2e15fe03bb518649cbdd3768b899d632%5E%21/#F0

+agawronska@, qnnguyen@ to CC if they recall any changes here.

Neither CL seems relevant - the crash fix would only matter if we were crashing there before, and the SimpleURLLoader just switches API to load a resource.  Seems like this needs someone who knows the ChromeOS captive portal flow to investigate.

Its currently marked at RBB and RBD. Changing priority to reflect that.

+snanda@ and rkc@ to help find an owner.

I'm on vacation, stumbled across this by accident and don't have easy access to my corp account at the moment. So posting from my personal account.

The original description here doesn't seem to justify release block for beta or dev. What am I missing?

I wrote the CL that added captive portal detection for Views login. This is likely just a case of me not knowing the spec so well and having the overlay pop up immediately upon detection rather than only when "Add user" is clicked. I can take ownership of this since I added the original code.

Hi, I'm planning to do a M70 Dev release tomorrow. Will we be able to get this unblocked in time? Thanks.

re: #33 Albert, seems it was made RBB/RBD because of #2 ("For the captive portal network, if the password is not known to the user. It will block logging-in to the device, and a reboot is required.").

This was marked RBD on August 3rd - have not have a dev release since then?

The CP overlay should not pop up for an existing user. It should be displayed when I click on "Add Person" from login screen. And I should be able to close/dismiss the CP overlay dialog if needed. 

But, here the CP overlay pops up and thereafter I am unable to dismiss the dialog.

I have a device configured with this, if you need to take a look, please drop by my desk.

Removing the Dev block for M70. This also existed in M69 and is not a regression for M70. Captive portal users in Dev channel would be a small percentage of overall uses. Keeping this a a RBB.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa5a44d488005cad8de6e450948e79efa1deaea6

commit aa5a44d488005cad8de6e450948e79efa1deaea6
Author: Quan Nguyen <qnnguyen@chromium.org>
Date: Tue Aug 21 21:30:47 2018

cros: Only show captive portal signin when OOBE dialog is visible in views login.

Bug:  870864 
Change-Id: Iae47b89761e9fff42a3fe4a8cf52bef391900988
Reviewed-on: https://chromium-review.googlesource.com/1183964
Commit-Queue: Quan Nguyen <qnnguyen@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584899}
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/screens/error_screen.cc
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/fake_login_display_host.cc
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/fake_login_display_host.h
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/login_display_host.h
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/login_display_host_mojo.cc
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/login_display_host_mojo.h
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/login_display_host_webui.cc
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/login_display_host_webui.h
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/mock_login_display_host.h
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.cc
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.h
[modify] https://crrev.com/aa5a44d488005cad8de6e450948e79efa1deaea6/chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

Removing the RBB on M-69, since I just confirmed with jmuppala@ that the feature is working correctly on M-69 (69.0.3497.51).

This is a regression in M-70 because the bug is only present in views login (and M-69 is shipping with webui login).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e23a87c67945a3b5052331993e2362cbc140258

commit 7e23a87c67945a3b5052331993e2362cbc140258
Author: Quan Nguyen <qnnguyen@chromium.org>
Date: Mon Aug 27 21:53:05 2018

cros: Make the captive portal dialog closeable.

This CL moves the dialog host to its own class and widget which fills
the entire screen. This allows the captive portal dialog to be
interactible in any location it could be rendered in. We also add an
observer to CaptivePortalWindowProxy to allow OobeUIDialogDelegate to
show/hide the host widget when the captive portal dialog is about to
be shown or has just been hidden.

Bug:  870864 
Change-Id: I898342795a3ecc950c6e4c273582962a2c70122b
Reviewed-on: https://chromium-review.googlesource.com/1187512
Commit-Queue: Quan Nguyen <qnnguyen@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586428}
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/screens/error_screen.cc
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/screens/error_screen.h
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/ui/captive_portal_window_proxy.cc
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/ui/captive_portal_window_proxy.h
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/ui/login_display_host_mojo.cc
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/ui/login_display_host_mojo.h
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.cc
[modify] https://crrev.com/7e23a87c67945a3b5052331993e2362cbc140258/chrome/browser/chromeos/login/ui/oobe_ui_dialog_delegate.h

With the latest CL, we should have the same captive portal overlay behavior in views-login (M70) as in webui-login (M69). Marking fixed.

Not able to reproduce on M70-11021.12.0, 70.0.3538.15 Cyan, hence closing the bug.