Reland "[Kiosk] Refactor AppLaunchSplashScreenHandler so it works more reliably"

This reverts commit 0e5a5146.

Reason for revert: Fixed errors in AppLaunchController. Time refactor all of this.

Original change's description:
> Revert "[Kiosk] Refactor AppLaunchSplashScreenHandler so it works more reliably"
>
> This reverts commit cdce07c9.
>
> Reason for revert: KioskTest.LaunchAppWithNetworkConfigAccelerator has been failing on msan fairly consistently since this landed:
> https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/17653
>
> Original change's description:
> > [Kiosk] Refactor AppLaunchSplashScreenHandler so it works more reliably
> >
> > This handler was little bit overcomplicated. This led to a bug in web
> > kiosk mode when we update network state, but do not update the handler.
> >
> > Because of this convolution, in web kiosks, the network configure
> > screen did not update itself when connected to network(which is what
> > client will expect from the handler which observes network state).
> > Now it behaves more logically and predictably.
> >
> > Bug: 1015383
> > Change-Id: I78b024b1d51b54b0d01d0a5a5694f6ae3f5ac19e
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2031406
> > Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
> > Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#739128}
>
> TBR=xiyuan@chromium.org,rsorokin@chromium.org,apotapchuk@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 1015383
> Change-Id: Ibc08de19048d9c8af95c3f517b748d09e37c7597
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2045479
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#739735}

TBR=xiyuan@chromium.org,thakis@chromium.org,rsorokin@chromium.org,apotapchuk@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1015383
Change-Id: Ie77ba40fa6bf3bc94a22c97a8d2b0764ef0614d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2047225Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745056}

chrome/app/chromeos_strings.grdp

@@ -929,6 +929,9 @@
   <message name="IDS_DISCOVER_PIN_SETUP_DONE" desc="Label for the done button on the success screen in 'PIN-unlock' setup dialog.">
     Done
   </message>
+  <message name="IDS_APP_START_PREPARING_PROFILE_MESSAGE" desc="Message displayed while preparing profile in which the app will be run.">
+    Preparing app profile...
+  </message>
   <message name="IDS_APP_START_NETWORK_WAIT_MESSAGE" desc="Message displayed while installing and/or launching web application in kiosk mode.">
     Waiting for network connection...
   </message>

chrome/app/chromium_strings_grd/IDS_APP_START_PREPARING_PROFILE_MESSAGE.png.sha1

+12b000c73d729c233b7fd0622515f8ed8f42416d
\ No newline at end of file

chrome/browser/chromeos/login/app_launch_controller.cc

@@ -298,7 +298,7 @@ void AppLaunchController::OnNetworkConfigFinished() {
   DCHECK(network_config_requested_);
   network_config_requested_ = false;
   app_launch_splash_screen_view_->UpdateAppLaunchState(
-      AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_NETWORK);
+      AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_PROFILE);
   startup_app_launcher_->RestartLauncher();
 }
 
@@ -306,10 +306,11 @@ void AppLaunchController::OnNetworkStateChanged(bool online) {
   if (!waiting_for_network_)
     return;
 
-  if (online && !network_config_requested_)
+  // If the network timed out, we should exit network config dialog as soon as we are back online.
+  if (online && (network_wait_timedout_ || !showing_network_dialog_)) {
+    ClearNetworkWaitTimer();
     startup_app_launcher_->ContinueWithNetworkReady();
-  else if (network_wait_timedout_)
-    MaybeShowNetworkConfigureUI();
+  }
 }
 
 void AppLaunchController::OnDeletingSplashScreenView() {
@@ -466,8 +467,13 @@ void AppLaunchController::InitializeNetwork() {
       FROM_HERE, base::TimeDelta::FromSeconds(network_wait_time_in_seconds),
       this, &AppLaunchController::OnNetworkWaitTimedout);
 
+  // Regardless of the network state, we should notify the view that network
+  // connection is required.
   app_launch_splash_screen_view_->UpdateAppLaunchState(
       AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_NETWORK);
+
+  if (app_launch_splash_screen_view_->IsNetworkReady())
+    OnNetworkStateChanged(/*online*/ true);
 }
 
 bool AppLaunchController::IsNetworkReady() {
@@ -486,7 +492,6 @@ void AppLaunchController::OnInstallingApp() {
   app_launch_splash_screen_view_->UpdateAppLaunchState(
       AppLaunchSplashScreenView::APP_LAUNCH_STATE_INSTALLING_APPLICATION);
 
-  ClearNetworkWaitTimer();
   app_launch_splash_screen_view_->ToggleNetworkConfig(false);
 
   // We have connectivity at this point, so we can skip the network
@@ -513,8 +518,6 @@ void AppLaunchController::OnReadyToLaunch() {
   if (splash_wait_timer_.IsRunning())
     return;
 
-  ClearNetworkWaitTimer();
-
   const int64_t time_taken_ms =
       (base::TimeTicks::Now() -
        base::TimeTicks::FromInternalValue(launch_splash_start_time_))

chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.cc

@@ -127,6 +127,13 @@ void AppLaunchSplashScreenHandler::UpdateAppLaunchState(AppLaunchState state) {
     SetLaunchText(
         l10n_util::GetStringUTF8(GetProgressMessageFromState(state_)));
   }
+
+  // When we are asked to initialize network, we should remember that this app
+  // requires network.
+  if (state_ == AppLaunchState::APP_LAUNCH_STATE_PREPARING_NETWORK) {
+    network_required_ = true;
+  }
+
   UpdateState(NetworkError::ERROR_REASON_UPDATE);
 }
 
@@ -135,13 +142,14 @@ void AppLaunchSplashScreenHandler::SetDelegate(Delegate* delegate) {
 }
 
 void AppLaunchSplashScreenHandler::ShowNetworkConfigureUI() {
+  network_config_shown_ = true;
+
   NetworkStateInformer::State state = network_state_informer_->state();
-  if (state == NetworkStateInformer::ONLINE) {
-    online_state_ = true;
-    if (!network_config_requested_) {
-      delegate_->OnNetworkStateChanged(true);
-      return;
-    }
+
+  // We should not block users when the network was not required by the
+  // controller.
+  if (!network_required_) {
+    state = NetworkStateInformer::ONLINE;
   }
 
   const std::string network_path = network_state_informer_->network_path();
@@ -197,16 +205,16 @@ void AppLaunchSplashScreenHandler::OnNetworkReady() {
 
 void AppLaunchSplashScreenHandler::UpdateState(
     NetworkError::ErrorReason reason) {
-  if (!delegate_ || (state_ != APP_LAUNCH_STATE_PREPARING_NETWORK &&
-                     state_ != APP_LAUNCH_STATE_NETWORK_WAIT_TIMEOUT)) {
+  if (!delegate_)
     return;
-  }
-
   bool new_online_state =
       network_state_informer_->state() == NetworkStateInformer::ONLINE;
   delegate_->OnNetworkStateChanged(new_online_state);
 
-  online_state_ = new_online_state;
+  // Redraw network configure UI when the network state changes.
+  if (network_config_shown_) {
+    ShowNetworkConfigureUI();
+  }
 }
 
 void AppLaunchSplashScreenHandler::PopulateAppInfo(
@@ -233,6 +241,8 @@ void AppLaunchSplashScreenHandler::SetLaunchText(const std::string& text) {
 int AppLaunchSplashScreenHandler::GetProgressMessageFromState(
     AppLaunchState state) {
   switch (state) {
+    case APP_LAUNCH_STATE_PREPARING_PROFILE:
+      return IDS_APP_START_PREPARING_PROFILE_MESSAGE;
     case APP_LAUNCH_STATE_PREPARING_NETWORK:
       return IDS_APP_START_NETWORK_WAIT_MESSAGE;
     case APP_LAUNCH_STATE_INSTALLING_APPLICATION:
@@ -264,21 +274,18 @@ void AppLaunchSplashScreenHandler::HandleCancelAppLaunch() {
 }
 
 void AppLaunchSplashScreenHandler::HandleNetworkConfigRequested() {
-  if (!delegate_ || network_config_done_)
+  if (!delegate_)
     return;
-
-  network_config_requested_ = true;
   delegate_->OnNetworkConfigRequested();
 }
 
 void AppLaunchSplashScreenHandler::HandleContinueAppLaunch() {
-  DCHECK(online_state_);
-  if (delegate_ && online_state_) {
-    network_config_requested_ = false;
-    network_config_done_ = true;
-    delegate_->OnNetworkConfigFinished();
-    Show();
-  }
+  if (!delegate_)
+    return;
+
+  network_config_shown_ = false;
+  delegate_->OnNetworkConfigFinished();
+  Show();
 }
 
 }  // namespace chromeos

chrome/browser/ui/webui/chromeos/login/app_launch_splash_screen_handler.h

@@ -45,6 +45,7 @@ class AppLaunchSplashScreenView {
   };
 
   enum AppLaunchState {
+    APP_LAUNCH_STATE_PREPARING_PROFILE,
     APP_LAUNCH_STATE_PREPARING_NETWORK,
     APP_LAUNCH_STATE_INSTALLING_APPLICATION,
     APP_LAUNCH_STATE_WAITING_APP_WINDOW,
@@ -125,19 +126,15 @@ class AppLaunchSplashScreenHandler
 
   Delegate* delegate_ = nullptr;
   bool show_on_init_ = false;
-  AppLaunchState state_ = APP_LAUNCH_STATE_PREPARING_NETWORK;
+  AppLaunchState state_ = APP_LAUNCH_STATE_PREPARING_PROFILE;
 
   scoped_refptr<NetworkStateInformer> network_state_informer_;
   ErrorScreen* error_screen_;
 
-  // True if we are online.
-  bool online_state_ = false;
-
-  // True if we have network config screen was already shown before.
-  bool network_config_done_ = false;
-
-  // True if we have manually requested network config screen.
-  bool network_config_requested_ = false;
+  // Whether network configure UI is being shown.
+  bool network_config_shown_ = false;
+  // Whether the network is required in order to proceed with app launch.
+  bool network_required_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(AppLaunchSplashScreenHandler);
 };