[CellularSetupUi] Embed CellularSetup in OsSettingsCellularSetupDialog.

* Move code that was in CellularSetupDialogUI to CellularSetupImpl so
  that both OsSettingsUI and CellularSetupDialogUI can use it.
* Bind OsSettingsUI to mojo interface.
* Add cellular setup localized strings to internet section so they
  will be displayed in the shared UI.

Bug: 1093185
Change-Id: I044e8af8f1cee07a1dda1d7ee0ada38e2c3fe6d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2349389Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Commit-Queue: Regan Hsu <hsuregan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800427}

chrome/browser/chrome_browser_interface_binders.cc

@@ -562,7 +562,8 @@ void PopulateChromeWebUIFrameBinders(
 
   RegisterWebUIControllerInterfaceBinder<
       chromeos::cellular_setup::mojom::CellularSetup,
-      chromeos::cellular_setup::CellularSetupDialogUI>(map);
+      chromeos::cellular_setup::CellularSetupDialogUI,
+      chromeos::settings::OSSettingsUI>(map);
 
   RegisterWebUIControllerInterfaceBinder<
       chromeos::crostini_installer::mojom::PageHandlerFactory,

chrome/browser/resources/chromeos/internet_config_dialog/BUILD.gn

@@ -29,9 +29,12 @@ js_library("internet_config_dialog") {
     "//ui/webui/resources/js:assert",
     "//ui/webui/resources/js:i18n_behavior",
   ]
-  externs_list = chrome_extension_public_externs + [
-                   "$externs_path/chrome_send.js",
-                   "$externs_path/networking_private.js",
-                 ]
+
+  externs_list = [
+    # TODO(crbug/1081815): Use autogenerated files instead of chrome_extensions.
+    "$externs_path/chrome_extensions.js",
+    "$externs_path/chrome_send.js",
+    "$externs_path/networking_private.js",
+  ]
   extra_sources = [ "$interfaces_path/networking_private_interface.js" ]
 }

chrome/browser/resources/settings/chromeos/internet_page/BUILD.gn

@@ -169,7 +169,10 @@ js_library("tether_connection_dialog") {
 }
 
 js_library("cellular_setup_dialog") {
-  deps = [ "//ui/webui/resources/cr_elements/cr_dialog:cr_dialog" ]
+  deps = [
+    "//ui/webui/resources/cr_components/chromeos/cellular_setup",
+    "//ui/webui/resources/cr_elements/cr_dialog:cr_dialog",
+  ]
 }
 
 # TODO: Uncomment as the Polymer3 migration makes progress.

chrome/browser/resources/settings/chromeos/internet_page/cellular_setup_dialog.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
+<link rel="import" href="chrome://resources/cr_components/chromeos/cellular_setup/cellular_setup.html">
 <link rel="import" href="chrome://resources/cr_elements/cr_dialog/cr_dialog.html">
 <link rel="import" href="chrome://resources/cr_elements/shared_vars_css.html">
 
@@ -11,9 +12,7 @@
     <cr-dialog id="dialog">
       <div slot="title">$i18n{cellularSetupDialogTitle}</div>
       <div slot="body">
-        <!-- TODO(crbug/1093185): Fix - embedding CellularSetup currently causes
-            settings to open with Error code: RESULT_CODE_KILLED_BAD_MESSAGE-->
-        <!-- <cellular-setup></cellular-setup> -->
+        <cellular-setup></cellular-setup>
       </div>
     </cr-dialog>
   </template>

chrome/browser/ui/webui/chromeos/cellular_setup/cellular_setup_dialog.cc

@@ -32,37 +32,6 @@ constexpr int kDialogWidthPx = 650;
 
 CellularSetupDialog* dialog_instance = nullptr;
 
-// Used to attach an instance of the CellularSetup service to a BrowserContext.
-class CellularSetupServiceHolder : public base::SupportsUserData::Data {
- public:
-  CellularSetupServiceHolder() = default;
-  ~CellularSetupServiceHolder() override = default;
-
-  void BindReceiver(mojo::PendingReceiver<mojom::CellularSetup> receiver) {
-    service_.BindReceiver(std::move(receiver));
-  }
-
- private:
-  CellularSetupImpl service_;
-
-  DISALLOW_COPY_AND_ASSIGN(CellularSetupServiceHolder);
-};
-
-const char kCellularSetupServiceHolderKey[] = "cellular_setup_service_holder";
-
-CellularSetupServiceHolder* GetOrCreateServiceHolder(
-    content::BrowserContext* browser_context) {
-  auto* holder = static_cast<CellularSetupServiceHolder*>(
-      browser_context->GetUserData(kCellularSetupServiceHolderKey));
-  if (!holder) {
-    auto new_holder = std::make_unique<CellularSetupServiceHolder>();
-    holder = new_holder.get();
-    browser_context->SetUserData(kCellularSetupServiceHolderKey,
-                                 std::move(new_holder));
-  }
-  return holder;
-}
-
 }  // namespace
 
 // static
@@ -131,8 +100,7 @@ CellularSetupDialogUI::~CellularSetupDialogUI() = default;
 
 void CellularSetupDialogUI::BindInterface(
     mojo::PendingReceiver<mojom::CellularSetup> receiver) {
-  GetOrCreateServiceHolder(web_ui()->GetWebContents()->GetBrowserContext())
-      ->BindReceiver(std::move(receiver));
+  CellularSetupImpl::CreateAndBindToReciever(std::move(receiver));
 }
 
 WEB_UI_CONTROLLER_TYPE_IMPL(CellularSetupDialogUI)

chrome/browser/ui/webui/settings/chromeos/internet_section.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/no_destructor.h"
 #include "base/strings/stringprintf.h"
+#include "chrome/browser/ui/webui/chromeos/cellular_setup/cellular_setup_localized_strings_provider.h"
 #include "chrome/browser/ui/webui/chromeos/network_element_localized_strings_provider.h"
 #include "chrome/browser/ui/webui/settings/chromeos/constants/routes.mojom.h"
 #include "chrome/browser/ui/webui/settings/chromeos/internet_handler.h"
@@ -661,11 +662,15 @@ void InternetSection::AddLoadTimeData(content::WebUIDataSource* html_source) {
   };
   AddLocalizedStringsBulk(html_source, kLocalizedStrings);
 
-  chromeos::network_element::AddLocalizedStrings(html_source);
-  chromeos::network_element::AddOncLocalizedStrings(html_source);
-  chromeos::network_element::AddDetailsLocalizedStrings(html_source);
-  chromeos::network_element::AddConfigLocalizedStrings(html_source);
-  chromeos::network_element::AddErrorLocalizedStrings(html_source);
+  network_element::AddLocalizedStrings(html_source);
+  network_element::AddOncLocalizedStrings(html_source);
+  network_element::AddDetailsLocalizedStrings(html_source);
+  network_element::AddConfigLocalizedStrings(html_source);
+  network_element::AddErrorLocalizedStrings(html_source);
+  if (base::FeatureList::IsEnabled(
+          chromeos::features::kUpdatedCellularActivationUi)) {
+    cellular_setup::AddLocalizedStrings(html_source);
+  }
 
   html_source->AddBoolean("showTechnologyBadge",
                           !ash::features::IsSeparateNetworkIconsEnabled());

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.cc

@@ -24,6 +24,7 @@
 #include "chrome/grit/os_settings_resources.h"
 #include "chrome/grit/os_settings_resources_map.h"
 #include "chromeos/constants/chromeos_features.h"
+#include "chromeos/services/cellular_setup/cellular_setup_impl.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_ui_data_source.h"
@@ -117,6 +118,12 @@ OSSettingsUI::~OSSettingsUI() {
                                 /*buckets=*/50);
 }
 
+void OSSettingsUI::BindInterface(
+    mojo::PendingReceiver<cellular_setup::mojom::CellularSetup> receiver) {
+  cellular_setup::CellularSetupImpl::CreateAndBindToReciever(
+      std::move(receiver));
+}
+
 void OSSettingsUI::BindInterface(
     mojo::PendingReceiver<network_config::mojom::CrosNetworkConfig> receiver) {
   ash::GetNetworkConfigService(std::move(receiver));

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.h

@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/webui/settings/chromeos/app_management/app_management_page_handler_factory.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/user_action_recorder.mojom-forward.h"
 #include "chrome/browser/ui/webui/webui_load_timer.h"
+#include "chromeos/services/cellular_setup/public/mojom/cellular_setup.mojom-forward.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom-forward.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "ui/webui/mojo_web_ui_controller.h"
@@ -37,6 +38,11 @@ class OSSettingsUI : public ui::MojoWebUIController {
   explicit OSSettingsUI(content::WebUI* web_ui);
   ~OSSettingsUI() override;
 
+  // Instantiates implementor of the mojom::CellularSetup mojo interface
+  // passing the pending receiver that will be internally bound.
+  void BindInterface(
+      mojo::PendingReceiver<cellular_setup::mojom::CellularSetup> receiver);
+
   // Instantiates implementor of the mojom::CrosNetworkConfig mojo interface
   // passing the pending receiver that will be internally bound.
   void BindInterface(

chromeos/services/cellular_setup/cellular_setup_impl.cc

@@ -11,11 +11,19 @@
 #include "base/no_destructor.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/services/cellular_setup/ota_activator_impl.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
 namespace chromeos {
 
 namespace cellular_setup {
 
+// static
+void CellularSetupImpl::CreateAndBindToReciever(
+    mojo::PendingReceiver<mojom::CellularSetup> receiver) {
+  mojo::MakeSelfOwnedReceiver(base::WrapUnique(new CellularSetupImpl()),
+                              std::move(receiver));
+}
+
 CellularSetupImpl::CellularSetupImpl() = default;
 
 CellularSetupImpl::~CellularSetupImpl() = default;

chromeos/services/cellular_setup/cellular_setup_impl.h

@@ -24,10 +24,19 @@ class OtaActivator;
 // pointer back to the client.
 class CellularSetupImpl : public CellularSetupBase {
  public:
-  CellularSetupImpl();
+  // Creates an instance with a lifetime that is bound to the connection
+  // that is supplying |receiver|.
+  static void CreateAndBindToReciever(
+      mojo::PendingReceiver<mojom::CellularSetup> receiver);
+
   ~CellularSetupImpl() override;
 
  private:
+  friend class CellularSetupImplTest;
+
+  // For unit tests.
+  CellularSetupImpl();
+
   // mojom::CellularSetup:
   void StartActivation(mojo::PendingRemote<mojom::ActivationDelegate> delegate,
                        StartActivationCallback callback) override;

chromeos/services/cellular_setup/cellular_setup_impl_unittest.cc

@@ -76,11 +76,9 @@ class CellularSetupImplTest : public testing::Test {
         &fake_ota_activator_factory_);
     shill_clients::InitializeFakes();
     NetworkHandler::Initialize();
-    cellular_setup_ = std::make_unique<CellularSetupImpl>();
   }
 
   void TearDown() override {
-    cellular_setup_.reset();
     NetworkHandler::Shutdown();
     shill_clients::Shutdown();
     OtaActivatorImpl::Factory::SetFactoryForTesting(nullptr);
@@ -92,7 +90,7 @@ class CellularSetupImplTest : public testing::Test {
               fake_ota_activator_factory_.created_instances().size());
 
     base::RunLoop run_loop;
-    cellular_setup_->StartActivation(
+    cellular_setup_.StartActivation(
         fake_activation_delegate->GenerateRemote(),
         base::BindOnce(&CellularSetupImplTest::OnCarrierPortalHandlerReceived,
                        base::Unretained(this), run_loop.QuitClosure()));
@@ -117,7 +115,7 @@ class CellularSetupImplTest : public testing::Test {
   base::test::TaskEnvironment task_environment_;
   FakeOtaActivatorFactory fake_ota_activator_factory_;
 
-  std::unique_ptr<CellularSetupBase> cellular_setup_;
+  CellularSetupImpl cellular_setup_;
 
   size_t num_carrier_portal_handlers_received_ = 0u;
 

ui/webui/resources/cr_components/chromeos/network/BUILD.gn

@@ -83,8 +83,11 @@ js_library("network_config") {
     "//ui/webui/resources/js:assert",
     "//ui/webui/resources/js:i18n_behavior",
   ]
-  externs_list = chrome_extension_public_externs +
-                 [ "$externs_path/networking_private.js" ]
+  externs_list = [
+    # TODO(crbug/1081815): Use autogenerated files instead of chrome_extensions
+    "$externs_path/chrome_extensions.js",
+    "$externs_path/networking_private.js",
+  ]
   extra_sources = [ "$interfaces_path/networking_private_interface.js" ]
 }