[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: I94eb7d8c1ff60f0f5b4a5843f6e3b2885c63b776
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083018
Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748240}

chrome/app/chromeos_strings.grdp

@@ -959,6 +959,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,8 +298,9 @@ void AppLaunchController::OnNetworkConfigRequested() {
 void AppLaunchController::OnNetworkConfigFinished() {
   DCHECK(network_config_requested_);
   network_config_requested_ = false;
+  showing_network_dialog_ = false;
   app_launch_splash_screen_view_->UpdateAppLaunchState(
-      AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_NETWORK);
+      AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_PROFILE);
   startup_app_launcher_->RestartLauncher();
 }
 
@@ -307,10 +308,12 @@ 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() {
@@ -467,10 +470,18 @@ 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 network configure ui was scheduled to be shown, we should display it
+  // before starting to install the app.
+  if (app_launch_splash_screen_view_->IsNetworkReady() &&
+      !show_network_config_ui_after_profile_load_) {
+    OnNetworkStateChanged(/*online*/ true);
+  }
+}
 bool AppLaunchController::IsNetworkReady() {
   return app_launch_splash_screen_view_ &&
          app_launch_splash_screen_view_->IsNetworkReady();

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;
+  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);
 };