Add offline scenario to Crostini configuration dialog

Adds offline scenario to Crostini configuration dialog. If Crostini
configuration failed while internet connection was missing dialog
informs user that Chromebook is offline and user can retry configuration
by hitting retry button.

Bug: 1029809
Change-Id: I6a5e006b5acb7300568eb6a40d9636725c229a12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947655Reviewed-by: Nic Hollingum <hollingum@google.com>
Commit-Queue: Olya Kalitova <okalitova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721487}

chrome/app/chromeos_strings.grdp

@@ -3625,16 +3625,25 @@
     Change location
   </message>
   <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_LABEL" translateable="false" desc="Label for dialog warning user of unavailable Linux container until software configurations are applied.">
-    Linux is being configured by your administrator
+    Configuring Linux
   </message>
   <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_SUBTEXT" translateable="false" desc="Description for dialog warning user of unavailable Linux container until software configurations are applied.">
-    Your app will open when configuration is finished. Configuration can take a few minutes.
+    Linux is being configured by your administrator. Configuration will take a few minutes.
   </message>
   <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_LABEL" translateable="false" desc="Label for Crostini software config dialog when there is an error in Ansible playbook application or installation.">
     Error configuring Linux
   </message>
   <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_SUBTEXT" translateable="false" desc="Text shown for Crostini software config dialog when there is an error in Ansible playbook application or installation.">
-    There was an error configuring Linux. Please contact your administrator.
+    There was an error while configuring Linux. Please contact your administrator.
+  </message>
+  <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_OFFLINE_LABEL" translateable="false" desc="Label for dialog warning user of missing internet connection required for container configuration.">
+    <ph name="DEVICE_TYPE">$1<ex>Chromebook</ex></ph> is offline
+  </message>
+  <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_OFFLINE_SUBTEXT" translateable="false" desc="Text shown for Crostini software config dialog when there is an error in Ansible playbook application or installation due to missing internet connection.">
+    Could not configure Linux. Connect to the internet and try again.
+  </message>
+  <message name="IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_RETRY_BUTTON" translateable="false" desc="Retry button of the Crostini software config dialog that starts the configuration steps again.">
+    Retry
   </message>
   <message name="IDS_CROSTINI_SHUT_DOWN_LINUX_MENU_ITEM" desc="Text shown in the context menu for the Linux terminal app, allowing users to shut down the Linux virtual machine.">
     Shut down Linux (Beta)

chrome/browser/ui/views/crostini/crostini_ansible_software_config_view.cc

@@ -9,9 +9,10 @@
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/public/browser/network_service_instance.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/views/controls/label.h"
-#include "ui/views/controls/progress_bar.h"
+#include "ui/chromeos/devicetype_utils.h"
+#include "ui/strings/grit/ui_strings.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/layout_provider.h"
 #include "ui/views/view_class_properties.h"
@@ -47,18 +48,75 @@ void CloseCrostiniAnsibleSoftwareConfigViewForTesting() {
 }  // namespace crostini
 
 int CrostiniAnsibleSoftwareConfigView::GetDialogButtons() const {
-  if (state_ == State::ERROR) {
-    return ui::DIALOG_BUTTON_OK;
+  switch (state_) {
+    case State::CONFIGURING:
+      return ui::DIALOG_BUTTON_NONE;
+    case State::ERROR:
+      return ui::DIALOG_BUTTON_OK;
+    case State::ERROR_OFFLINE:
+      return ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL;
   }
-  return ui::DIALOG_BUTTON_NONE;
+}
+
+base::string16 CrostiniAnsibleSoftwareConfigView::GetDialogButtonLabel(
+    ui::DialogButton button) const {
+  switch (state_) {
+    case State::ERROR:
+      DCHECK_EQ(button, ui::DIALOG_BUTTON_OK);
+      return l10n_util::GetStringUTF16(IDS_APP_OK);
+    case State::ERROR_OFFLINE:
+      if (button == ui::DIALOG_BUTTON_OK) {
+        return l10n_util::GetStringUTF16(
+            IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_RETRY_BUTTON);
+      } else {
+        DCHECK_EQ(button, ui::DIALOG_BUTTON_CANCEL);
+        return l10n_util::GetStringUTF16(IDS_APP_CANCEL);
+      }
+    case State::CONFIGURING:
+      NOTREACHED();
+      return base::string16();
+  }
+}
+
+bool CrostiniAnsibleSoftwareConfigView::Accept() {
+  if (state_ == State::ERROR_OFFLINE) {
+    state_ = State::CONFIGURING;
+    OnStateChanged();
+
+    ansible_management_service_->ConfigureDefaultContainer(base::DoNothing());
+    return false;
+  }
+  DCHECK_EQ(state_, State::ERROR);
+  return true;
 }
 
 base::string16 CrostiniAnsibleSoftwareConfigView::GetWindowTitle() const {
-  if (state_ == State::ERROR) {
-    return l10n_util::GetStringUTF16(
-        IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_LABEL);
+  switch (state_) {
+    case State::CONFIGURING:
+      return l10n_util::GetStringUTF16(
+          IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_LABEL);
+    case State::ERROR:
+      return l10n_util::GetStringUTF16(
+          IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_LABEL);
+    case State::ERROR_OFFLINE:
+      return l10n_util::GetStringFUTF16(
+          IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_OFFLINE_LABEL,
+          ui::GetChromeOSDeviceName());
+  }
+}
+
+base::string16 CrostiniAnsibleSoftwareConfigView::GetSubtextLabel() const {
+  switch (state_) {
+    case State::CONFIGURING:
+      return l10n_util::GetStringUTF16(
+          IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_SUBTEXT);
+    case State::ERROR:
+      return l10n_util::GetStringUTF16(
+          IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_SUBTEXT);
+    case State::ERROR_OFFLINE:
+      return l10n_util::GetStringUTF16(
+          IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_OFFLINE_SUBTEXT);
   }
-  return l10n_util::GetStringUTF16(IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_LABEL);
 }
 
 gfx::Size CrostiniAnsibleSoftwareConfigView::CalculatePreferredSize() const {
@@ -69,24 +127,19 @@ gfx::Size CrostiniAnsibleSoftwareConfigView::CalculatePreferredSize() const {
 }
 
 void CrostiniAnsibleSoftwareConfigView::
-    OnAnsibleSoftwareConfigurationStarted() {
-  NOTIMPLEMENTED();
-}
+    OnAnsibleSoftwareConfigurationStarted() {}
 
 void CrostiniAnsibleSoftwareConfigView::OnAnsibleSoftwareConfigurationFinished(
     bool success) {
   DCHECK_EQ(state_, State::CONFIGURING);
 
   if (!success) {
-    state_ = State::ERROR;
-
-    GetWidget()->UpdateWindowTitle();
-    subtext_label_->SetText(l10n_util::GetStringUTF16(
-        IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_SUBTEXT));
+    if (content::GetNetworkConnectionTracker()->IsOffline())
+      state_ = State::ERROR_OFFLINE;
+    else
+      state_ = State::ERROR;
 
-    progress_bar_->SetVisible(false);
-
-    DialogModelChanged();
+    OnStateChanged();
     return;
   }
   // TODO(crbug.com/1005774): We should preferably add another ClosedReason
@@ -142,3 +195,11 @@ CrostiniAnsibleSoftwareConfigView::~CrostiniAnsibleSoftwareConfigView() {
   ansible_management_service_->RemoveObserver(this);
   g_crostini_ansible_software_configuration_view = nullptr;
 }
+
+void CrostiniAnsibleSoftwareConfigView::OnStateChanged() {
+  progress_bar_->SetVisible(state_ == State::CONFIGURING);
+  subtext_label_->SetText(GetSubtextLabel());
+  DialogModelChanged();
+  GetWidget()->UpdateWindowTitle();
+  GetWidget()->SetSize(GetWidget()->non_client_view()->GetPreferredSize());
+}

chrome/browser/ui/views/crostini/crostini_ansible_software_config_view.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_UI_VIEWS_CROSTINI_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_VIEW_H_
 
 #include "chrome/browser/chromeos/crostini/ansible/ansible_management_service.h"
+#include "ui/base/ui_base_types.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/progress_bar.h"
@@ -21,6 +22,8 @@ class CrostiniAnsibleSoftwareConfigView
  public:
   // views::DialogDelegateView:
   int GetDialogButtons() const override;
+  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
+  bool Accept() override;
   base::string16 GetWindowTitle() const override;
   gfx::Size CalculatePreferredSize() const override;
 
@@ -38,8 +41,12 @@ class CrostiniAnsibleSoftwareConfigView
   enum class State {
     CONFIGURING,
     ERROR,
+    ERROR_OFFLINE,
   };
 
+  void OnStateChanged();
+  base::string16 GetSubtextLabel() const;
+
   State state_ = State::CONFIGURING;
   crostini::AnsibleManagementService* ansible_management_service_ = nullptr;
 

chrome/browser/ui/views/crostini/crostini_ansible_software_config_view_browsertest.cc

@@ -11,7 +11,10 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/views/crostini/crostini_browser_test_util.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/public/browser/network_service_instance.h"
+#include "services/network/test/test_network_connection_tracker.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/chromeos/devicetype_utils.h"
 
 class CrostiniAnsibleSoftwareConfigViewBrowserTest
     : public CrostiniDialogBrowserTest {
@@ -19,7 +22,9 @@ class CrostiniAnsibleSoftwareConfigViewBrowserTest
   CrostiniAnsibleSoftwareConfigViewBrowserTest()
       : CrostiniDialogBrowserTest(true /*register_termina*/),
         container_id_(crostini::kCrostiniDefaultVmName,
-                      crostini::kCrostiniDefaultContainerName) {}
+                      crostini::kCrostiniDefaultContainerName),
+        network_connection_tracker_(
+            network::TestNetworkConnectionTracker::CreateInstance()) {}
 
   // CrostiniDialogBrowserTest:
   void ShowUi(const std::string& name) override {
@@ -32,11 +37,20 @@ class CrostiniAnsibleSoftwareConfigViewBrowserTest
 
  protected:
   void SetUpOnMainThread() override {
+    // NetworkConnectionTracker should be reset first.
+    content::SetNetworkConnectionTrackerForTesting(nullptr);
+    content::SetNetworkConnectionTrackerForTesting(
+        network_connection_tracker_.get());
+
     test_helper_ = std::make_unique<crostini::AnsibleManagementTestHelper>(
         browser()->profile());
     test_helper_->SetUpAnsiblePlaybookPreference();
   }
 
+  void SetConnectionType(network::mojom::ConnectionType type) {
+    network_connection_tracker_->SetConnectionType(type);
+  }
+
   // A new Widget was created in ShowUi() or since the last VerifyUi().
   bool HasView() { return VerifyUi() && ActiveView() != nullptr; }
 
@@ -46,9 +60,17 @@ class CrostiniAnsibleSoftwareConfigViewBrowserTest
     return !VerifyUi() && ActiveView() == nullptr;
   }
 
-  bool IsDefaultDialog() { return !HasAcceptButton() && HasDefaultStrings(); }
+  bool IsDefaultDialog() {
+    return !HasAcceptButton() && !HasCancelButton() && HasDefaultStrings();
+  }
 
-  bool IsErrorDialog() { return HasAcceptButton() && HasErrorStrings(); }
+  bool IsErrorDialog() {
+    return HasAcceptButton() && !HasCancelButton() && HasErrorStrings();
+  }
+
+  bool IsErrorOfflineDialog() {
+    return HasAcceptButton() && HasCancelButton() && HasErrorOfflineStrings();
+  }
 
   crostini::AnsibleManagementService* ansible_management_service() {
     return crostini::AnsibleManagementService::GetForProfile(
@@ -59,23 +81,38 @@ class CrostiniAnsibleSoftwareConfigViewBrowserTest
 
  private:
   bool HasAcceptButton() { return ActiveView()->GetOkButton() != nullptr; }
+  bool HasCancelButton() { return ActiveView()->GetCancelButton() != nullptr; }
 
   bool HasDefaultStrings() {
-    return (ActiveView()->GetWindowTitle().compare(l10n_util::GetStringUTF16(
-                IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_LABEL)) == 0) &&
-           (ActiveView()->GetSubtextLabelStringForTesting().compare(
-                l10n_util::GetStringUTF16(
-                    IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_SUBTEXT)) == 0);
+    return ActiveView()->GetWindowTitle() ==
+               l10n_util::GetStringUTF16(
+                   IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_LABEL) &&
+           ActiveView()->GetSubtextLabelStringForTesting() ==
+               l10n_util::GetStringUTF16(
+                   IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_SUBTEXT);
   }
 
   bool HasErrorStrings() {
-    return (ActiveView()->GetWindowTitle().compare(l10n_util::GetStringUTF16(
-                IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_LABEL)) == 0) &&
-           (ActiveView()->GetSubtextLabelStringForTesting().compare(
-                l10n_util::GetStringUTF16(
-                    IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_SUBTEXT)) == 0);
+    return ActiveView()->GetWindowTitle() ==
+               l10n_util::GetStringUTF16(
+                   IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_LABEL) &&
+           ActiveView()->GetSubtextLabelStringForTesting() ==
+               l10n_util::GetStringUTF16(
+                   IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_SUBTEXT);
   }
 
+  bool HasErrorOfflineStrings() {
+    return ActiveView()->GetWindowTitle() ==
+               l10n_util::GetStringFUTF16(
+                   IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_OFFLINE_LABEL,
+                   ui::GetChromeOSDeviceName()) &&
+           ActiveView()->GetSubtextLabelStringForTesting() ==
+               l10n_util::GetStringUTF16(
+                   IDS_CROSTINI_ANSIBLE_SOFTWARE_CONFIG_ERROR_OFFLINE_SUBTEXT);
+  }
+
+  std::unique_ptr<network::TestNetworkConnectionTracker>
+      network_connection_tracker_;
   std::unique_ptr<crostini::AnsibleManagementTestHelper> test_helper_;
 };
 
@@ -109,6 +146,62 @@ IN_PROC_BROWSER_TEST_F(CrostiniAnsibleSoftwareConfigViewBrowserTest,
   EXPECT_TRUE(IsErrorDialog());
 }
 
+IN_PROC_BROWSER_TEST_F(CrostiniAnsibleSoftwareConfigViewBrowserTest,
+                       UnsuccessfulFlow_Offline) {
+  SetConnectionType(network::mojom::ConnectionType::CONNECTION_NONE);
+
+  ShowUi("default");
+
+  EXPECT_TRUE(HasView());
+  EXPECT_TRUE(IsDefaultDialog());
+
+  ActiveView()->OnAnsibleSoftwareConfigurationFinished(false);
+
+  EXPECT_NE(nullptr, ActiveView());
+  EXPECT_TRUE(IsErrorOfflineDialog());
+}
+
+IN_PROC_BROWSER_TEST_F(CrostiniAnsibleSoftwareConfigViewBrowserTest,
+                       UnsuccessfulFlow_Offline_CanRetry) {
+  SetConnectionType(network::mojom::ConnectionType::CONNECTION_NONE);
+
+  ShowUi("default");
+
+  EXPECT_TRUE(HasView());
+  EXPECT_TRUE(IsDefaultDialog());
+
+  ActiveView()->OnAnsibleSoftwareConfigurationFinished(false);
+
+  EXPECT_NE(nullptr, ActiveView());
+  EXPECT_TRUE(IsErrorOfflineDialog());
+
+  // Retry button clicked.
+  ActiveView()->AcceptDialog();
+
+  EXPECT_NE(nullptr, ActiveView());
+  EXPECT_TRUE(IsDefaultDialog());
+}
+
+IN_PROC_BROWSER_TEST_F(CrostiniAnsibleSoftwareConfigViewBrowserTest,
+                       UnsuccessfulFlow_Offline_Cancel) {
+  SetConnectionType(network::mojom::ConnectionType::CONNECTION_NONE);
+
+  ShowUi("default");
+
+  EXPECT_TRUE(HasView());
+  EXPECT_TRUE(IsDefaultDialog());
+
+  ActiveView()->OnAnsibleSoftwareConfigurationFinished(false);
+
+  EXPECT_NE(nullptr, ActiveView());
+  EXPECT_TRUE(IsErrorOfflineDialog());
+
+  // Cancel button clicked.
+  ActiveView()->CancelDialog();
+
+  EXPECT_TRUE(HasNoView());
+}
+
 IN_PROC_BROWSER_TEST_F(CrostiniAnsibleSoftwareConfigViewBrowserTest,
                        AnsibleConfigFlow_Successful) {
   ansible_management_service()->ConfigureDefaultContainer(base::DoNothing());