Revert "Reland "[Kiosk] Refactor AppLaunchSplashScreenHandler so it w…

…orks more reliably""

This reverts commit 39a8ab1.

Reason for revert: Speculatively causing consistent failures in
KioskTest.LaunchAppWithNetworkConfigAccelerator, which was mentioned
earlier in this revert/reland thread.

First failed build included this change: https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/3569

Original change's description:
> Reland "[Kiosk] Refactor AppLaunchSplashScreenHandler so it works more reliably"
> 
> This reverts commit 0e5a514.
> 
> 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 cdce07c.
> >
> > 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/+/2047225
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#745056}

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

Change-Id: I8696f63297d359b9aa34dcaa9d8e402ba9afb419
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1015383
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2079138
Reviewed-by: Katie Dektar <katie@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745126}

chrome/app/chromeos_strings.grdp

@@ -929,9 +929,6 @@
   <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/browser/chromeos/login/app_launch_controller.cc

@@ -298,19 +298,18 @@ void AppLaunchController::OnNetworkConfigFinished() {
   DCHECK(network_config_requested_);
   network_config_requested_ = false;
   app_launch_splash_screen_view_->UpdateAppLaunchState(
-      AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_PROFILE);
+      AppLaunchSplashScreenView::APP_LAUNCH_STATE_PREPARING_NETWORK);
   startup_app_launcher_->RestartLauncher();
 }
 
 void AppLaunchController::OnNetworkStateChanged(bool online) {
   if (!waiting_for_network_)
     return;
 
-  // 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();
+  if (online && !network_config_requested_)
     startup_app_launcher_->ContinueWithNetworkReady();
-  }
+  else if (network_wait_timedout_)
+    MaybeShowNetworkConfigureUI();
 }
 
 void AppLaunchController::OnDeletingSplashScreenView() {
@@ -467,13 +466,8 @@ 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() {
@@ -492,6 +486,7 @@ 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
@@ -518,6 +513,8 @@ 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,13 +127,6 @@ 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);
 }
 
@@ -142,14 +135,13 @@ void AppLaunchSplashScreenHandler::SetDelegate(Delegate* delegate) {
 }
 
 void AppLaunchSplashScreenHandler::ShowNetworkConfigureUI() {
-  network_config_shown_ = true;
-
   NetworkStateInformer::State state = network_state_informer_->state();
-
-  // We should not block users when the network was not required by the
-  // controller.
-  if (!network_required_) {
-    state = NetworkStateInformer::ONLINE;
+  if (state == NetworkStateInformer::ONLINE) {
+    online_state_ = true;
+    if (!network_config_requested_) {
+      delegate_->OnNetworkStateChanged(true);
+      return;
+    }
   }
 
   const std::string network_path = network_state_informer_->network_path();
@@ -205,16 +197,16 @@ void AppLaunchSplashScreenHandler::OnNetworkReady() {
 
 void AppLaunchSplashScreenHandler::UpdateState(
     NetworkError::ErrorReason reason) {
-  if (!delegate_)
+  if (!delegate_ || (state_ != APP_LAUNCH_STATE_PREPARING_NETWORK &&
+                     state_ != APP_LAUNCH_STATE_NETWORK_WAIT_TIMEOUT)) {
     return;
+  }
+
   bool new_online_state =
       network_state_informer_->state() == NetworkStateInformer::ONLINE;
   delegate_->OnNetworkStateChanged(new_online_state);
 
-  // Redraw network configure UI when the network state changes.
-  if (network_config_shown_) {
-    ShowNetworkConfigureUI();
-  }
+  online_state_ = new_online_state;
 }
 
 void AppLaunchSplashScreenHandler::PopulateAppInfo(
@@ -241,8 +233,6 @@ 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:
@@ -274,18 +264,21 @@ void AppLaunchSplashScreenHandler::HandleCancelAppLaunch() {
 }
 
 void AppLaunchSplashScreenHandler::HandleNetworkConfigRequested() {
-  if (!delegate_)
+  if (!delegate_ || network_config_done_)
     return;
+
+  network_config_requested_ = true;
   delegate_->OnNetworkConfigRequested();
 }
 
 void AppLaunchSplashScreenHandler::HandleContinueAppLaunch() {
-  if (!delegate_)
-    return;
-
-  network_config_shown_ = false;
-  delegate_->OnNetworkConfigFinished();
-  Show();
+  DCHECK(online_state_);
+  if (delegate_ && online_state_) {
+    network_config_requested_ = false;
+    network_config_done_ = true;
+    delegate_->OnNetworkConfigFinished();
+    Show();
+  }
 }
 
 }  // namespace chromeos

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

@@ -45,7 +45,6 @@ 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,
@@ -126,15 +125,19 @@ class AppLaunchSplashScreenHandler
 
   Delegate* delegate_ = nullptr;
   bool show_on_init_ = false;
-  AppLaunchState state_ = APP_LAUNCH_STATE_PREPARING_PROFILE;
+  AppLaunchState state_ = APP_LAUNCH_STATE_PREPARING_NETWORK;
 
   scoped_refptr<NetworkStateInformer> network_state_informer_;
   ErrorScreen* error_screen_;
 
-  // 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;
+  // 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;
 
   DISALLOW_COPY_AND_ASSIGN(AppLaunchSplashScreenHandler);
 };