diff --git a/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.cc b/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.cc index 71fabab61b22f0..0843deaf5c6a51 100644 --- a/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.cc +++ b/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.cc @@ -9,10 +9,15 @@ #include "base/bind.h" #include "base/callback.h" #include "chrome/browser/chromeos/android_sms/android_sms_urls.h" +#include "chrome/browser/extensions/extension_util.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/extensions/app_launch_params.h" +#include "chrome/browser/ui/extensions/application_launch.h" #include "chrome/browser/web_applications/components/pending_app_manager.h" #include "chrome/browser/web_applications/web_app_provider.h" #include "chromeos/components/proximity_auth/logging/logging.h" +#include "extensions/common/constants.h" +#include "ui/base/window_open_disposition.h" namespace chromeos { @@ -22,6 +27,7 @@ AndroidSmsAppHelperDelegateImpl::AndroidSmsAppHelperDelegateImpl( Profile* profile) : pending_app_manager_( &web_app::WebAppProvider::Get(profile)->pending_app_manager()), + profile_(profile), weak_ptr_factory_(this) {} AndroidSmsAppHelperDelegateImpl::AndroidSmsAppHelperDelegateImpl( @@ -42,6 +48,24 @@ void AndroidSmsAppHelperDelegateImpl::InstallAndroidSmsApp() { weak_ptr_factory_.GetWeakPtr())); } +bool AndroidSmsAppHelperDelegateImpl::LaunchAndroidSmsApp() { + const extensions::Extension* android_sms_pwa = + extensions::util::GetInstalledPwaForUrl( + profile_, chromeos::android_sms::GetAndroidMessagesURL()); + if (!android_sms_pwa) { + PA_LOG(ERROR) << "No Messages app found."; + return false; + } + + PA_LOG(INFO) << "Messages app Launching..."; + AppLaunchParams params( + profile_, android_sms_pwa, extensions::LAUNCH_CONTAINER_WINDOW, + WindowOpenDisposition::NEW_WINDOW, extensions::SOURCE_CHROME_INTERNAL); + // OpenApplications() is defined in application_launch.h. + OpenApplication(params); + return true; +} + void AndroidSmsAppHelperDelegateImpl::OnAppInstalled( const GURL& app_url, const base::Optional& app_id) { diff --git a/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h b/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h index dde921fb427aa9..2f47736eca90a5 100644 --- a/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h +++ b/chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h @@ -30,6 +30,10 @@ class AndroidSmsAppHelperDelegateImpl : public AndroidSmsAppHelperDelegate { private: friend class AndroidSmsAppHelperDelegateImplTest; + // Note: This constructor should only be used in testing. Right now, objects + // built using this constructor will segfault on profile_ if + // LaunchAndroidSmsApp is called. We'll need to fix this once tests for that + // function are added. See https://crbug.com/876972. explicit AndroidSmsAppHelperDelegateImpl( web_app::PendingAppManager* pending_app_manager); void OnAppInstalled(const GURL& app_url, @@ -37,9 +41,11 @@ class AndroidSmsAppHelperDelegateImpl : public AndroidSmsAppHelperDelegate { // AndroidSmsAppHelperDelegate: void InstallAndroidSmsApp() override; + bool LaunchAndroidSmsApp() override; static const char kMessagesWebAppUrl[]; web_app::PendingAppManager* pending_app_manager_; + Profile* profile_; base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(AndroidSmsAppHelperDelegateImpl); diff --git a/chrome/browser/resources/settings/multidevice_page/multidevice_browser_proxy.js b/chrome/browser/resources/settings/multidevice_page/multidevice_browser_proxy.js index 39f278bc6dca0e..bdc01201447294 100644 --- a/chrome/browser/resources/settings/multidevice_page/multidevice_browser_proxy.js +++ b/chrome/browser/resources/settings/multidevice_page/multidevice_browser_proxy.js @@ -24,6 +24,12 @@ cr.define('settings', function() { removeHostDevice() {} retryPendingHostSetup() {} + + /** + * Called when the "Set Up" button is clicked to open the Android Messages + * PWA. + */ + setUpAndroidSms() {} } /** @@ -55,6 +61,11 @@ cr.define('settings', function() { retryPendingHostSetup() { chrome.send('retryPendingHostSetup'); } + + /** @override */ + setUpAndroidSms() { + chrome.send('setUpAndroidSms'); + } } cr.addSingletonGetter(MultiDeviceBrowserProxyImpl); diff --git a/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js b/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js index e7cb3e433fa010..a8da00c237d5c0 100644 --- a/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js +++ b/chrome/browser/resources/settings/multidevice_page/multidevice_subpage.js @@ -56,9 +56,17 @@ Polymer({ }, }, + /** @private {?settings.MultiDeviceBrowserProxy} */ + browserProxy_: null, + + /** @override */ + created: function() { + this.browserProxy_ = settings.MultiDeviceBrowserProxyImpl.getInstance(); + }, + /** @private */ handleAndroidMessagesButtonClick_: function() { - this.androidMessagesRequiresSetup_ = false; + this.browserProxy_.setUpAndroidSms(); }, listeners: { diff --git a/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc b/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc index ac6d62acbb7e18..36ef94d7f31f5c 100644 --- a/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc +++ b/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.cc @@ -7,6 +7,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/logging.h" +#include "chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h" #include "chrome/browser/ui/webui/chromeos/multidevice_setup/multidevice_setup_dialog.h" #include "chromeos/components/proximity_auth/logging/logging.h" #include "content/public/browser/web_ui.h" @@ -35,8 +36,11 @@ void OnRetrySetHostNowResult(bool success) { } // namespace MultideviceHandler::MultideviceHandler( - multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client) + multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client, + std::unique_ptr + android_sms_app_helper) : multidevice_setup_client_(multidevice_setup_client), + android_sms_app_helper_(std::move(android_sms_app_helper)), multidevice_setup_observer_(this), callback_weak_ptr_factory_(this) {} @@ -63,6 +67,10 @@ void MultideviceHandler::RegisterMessages() { "retryPendingHostSetup", base::BindRepeating(&MultideviceHandler::HandleRetryPendingHostSetup, base::Unretained(this))); + web_ui()->RegisterMessageCallback( + "setUpAndroidSms", + base::BindRepeating(&MultideviceHandler::HandleSetUpAndroidSms, + base::Unretained(this))); } void MultideviceHandler::OnJavascriptAllowed() { @@ -158,6 +166,12 @@ void MultideviceHandler::HandleRetryPendingHostSetup( base::BindOnce(&OnRetrySetHostNowResult)); } +void MultideviceHandler::HandleSetUpAndroidSms(const base::ListValue* args) { + PA_LOG(WARNING) << "HandlingSetupSms"; + DCHECK(args->empty()); + android_sms_app_helper_->LaunchAndroidSmsApp(); +} + void MultideviceHandler::OnSetFeatureStateEnabledResult( const std::string& js_callback_id, bool success) { diff --git a/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h b/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h index 3efb6584ea0ffa..cc5e9037e95218 100644 --- a/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h +++ b/chrome/browser/ui/webui/settings/chromeos/multidevice_handler.h @@ -15,6 +15,10 @@ namespace chromeos { +namespace multidevice_setup { +class AndroidSmsAppHelperDelegate; +} // namespace multidevice_setup + namespace settings { // Chrome "Multidevice" (a.k.a. "Connected Devices") settings page UI handler. @@ -23,7 +27,9 @@ class MultideviceHandler public multidevice_setup::MultiDeviceSetupClient::Observer { public: explicit MultideviceHandler( - multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client); + multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client, + std::unique_ptr + android_sms_app_helper); ~MultideviceHandler() override; protected: @@ -52,6 +58,7 @@ class MultideviceHandler void HandleSetFeatureEnabledState(const base::ListValue* args); void HandleRemoveHostDevice(const base::ListValue* args); void HandleRetryPendingHostSetup(const base::ListValue* args); + void HandleSetUpAndroidSms(const base::ListValue* args); void OnSetFeatureStateEnabledResult(const std::string& js_callback_id, bool success); @@ -62,6 +69,8 @@ class MultideviceHandler std::unique_ptr GeneratePageContentDataDictionary(); multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client_; + std::unique_ptr + android_sms_app_helper_; ScopedObserver diff --git a/chrome/browser/ui/webui/settings/chromeos/multidevice_handler_unittest.cc b/chrome/browser/ui/webui/settings/chromeos/multidevice_handler_unittest.cc index 61ef44afa3187f..658795d6c18d9d 100644 --- a/chrome/browser/ui/webui/settings/chromeos/multidevice_handler_unittest.cc +++ b/chrome/browser/ui/webui/settings/chromeos/multidevice_handler_unittest.cc @@ -7,6 +7,7 @@ #include #include "base/macros.h" +#include "chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h" #include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h" #include "components/cryptauth/remote_device_test_util.h" #include "content/public/test/test_web_ui.h" @@ -21,8 +22,11 @@ namespace { class TestMultideviceHandler : public MultideviceHandler { public: TestMultideviceHandler( - multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client) - : MultideviceHandler(multidevice_setup_client) {} + multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client, + std::unique_ptr + android_sms_app_helper) + : MultideviceHandler(multidevice_setup_client, + std::move(android_sms_app_helper)) {} ~TestMultideviceHandler() override = default; // Make public for testing. @@ -107,9 +111,14 @@ class MultideviceHandlerTest : public testing::Test { fake_multidevice_setup_client_ = std::make_unique(); + auto fake_android_sms_app_helper_delegate = + std::make_unique(); + fake_android_sms_app_helper_delegate_ = + fake_android_sms_app_helper_delegate.get(); handler_ = std::make_unique( - fake_multidevice_setup_client_.get()); + fake_multidevice_setup_client_.get(), + std::move(fake_android_sms_app_helper_delegate)); handler_->set_web_ui(test_web_ui_.get()); handler_->RegisterMessages(); handler_->AllowJavascript(); @@ -184,6 +193,11 @@ class MultideviceHandlerTest : public testing::Test { success); } + void CallSetUpAndroidSms() { + base::ListValue empty_args; + test_web_ui()->HandleReceivedMessage("setUpAndroidSms", &empty_args); + } + void CallSetFeatureEnabledState(multidevice_setup::mojom::Feature feature, bool enabled, const base::Optional& auth_token, @@ -225,6 +239,11 @@ class MultideviceHandlerTest : public testing::Test { return fake_multidevice_setup_client_.get(); } + multidevice_setup::FakeAndroidSmsAppHelperDelegate* + fake_android_sms_app_helper_delegate() { + return fake_android_sms_app_helper_delegate_; + } + const cryptauth::RemoteDeviceRef test_device_; private: @@ -244,6 +263,8 @@ class MultideviceHandlerTest : public testing::Test { host_status_with_device_; multidevice_setup::MultiDeviceSetupClient::FeatureStatesMap feature_states_map_; + multidevice_setup::FakeAndroidSmsAppHelperDelegate* + fake_android_sms_app_helper_delegate_; DISALLOW_COPY_AND_ASSIGN(MultideviceHandlerTest); }; @@ -280,6 +301,12 @@ TEST_F(MultideviceHandlerTest, RetryPendingHostSetup) { CallRetryPendingHostSetup(false /* success */); } +TEST_F(MultideviceHandlerTest, SetUpAndroidSms) { + EXPECT_FALSE(fake_android_sms_app_helper_delegate()->HasLaunchedApp()); + CallSetUpAndroidSms(); + EXPECT_TRUE(fake_android_sms_app_helper_delegate()->HasLaunchedApp()); +} + TEST_F(MultideviceHandlerTest, SetFeatureEnabledState) { CallSetFeatureEnabledState( multidevice_setup::mojom::Feature::kBetterTogetherSuite, diff --git a/chrome/browser/ui/webui/settings/md_settings_ui.cc b/chrome/browser/ui/webui/settings/md_settings_ui.cc index 368ba572922402..fa6dd8d8b36b98 100644 --- a/chrome/browser/ui/webui/settings/md_settings_ui.cc +++ b/chrome/browser/ui/webui/settings/md_settings_ui.cc @@ -77,6 +77,7 @@ #include "chrome/browser/chromeos/crostini/crostini_util.h" #include "chrome/browser/chromeos/login/demo_mode/demo_session.h" #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_utils.h" +#include "chrome/browser/chromeos/multidevice_setup/android_sms_app_helper_delegate_impl.h" #include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h" #include "chrome/browser/signin/account_tracker_service_factory.h" #include "chrome/browser/ui/webui/settings/chromeos/accessibility_handler.h" @@ -226,7 +227,10 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui) AddSettingsPageUIHandler( std::make_unique( chromeos::multidevice_setup::MultiDeviceSetupClientFactory:: - GetForProfile(profile))); + GetForProfile(profile), + std::make_unique< + chromeos::multidevice_setup::AndroidSmsAppHelperDelegateImpl>( + profile))); } AddSettingsPageUIHandler( std::make_unique()); diff --git a/chrome/test/data/webui/settings/cr_settings_browsertest.js b/chrome/test/data/webui/settings/cr_settings_browsertest.js index 4ea353348a5c3c..41c35b50342f60 100644 --- a/chrome/test/data/webui/settings/cr_settings_browsertest.js +++ b/chrome/test/data/webui/settings/cr_settings_browsertest.js @@ -1992,6 +1992,7 @@ CrSettingsMultideviceSubpageTest.prototype = { /** @override */ extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([ + '../test_browser_proxy.js', 'multidevice_subpage_tests.js', ]), }; diff --git a/chrome/test/data/webui/settings/multidevice_subpage_tests.js b/chrome/test/data/webui/settings/multidevice_subpage_tests.js index 76d50474a00309..64733642c4fbcb 100644 --- a/chrome/test/data/webui/settings/multidevice_subpage_tests.js +++ b/chrome/test/data/webui/settings/multidevice_subpage_tests.js @@ -2,8 +2,25 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +/** + * @implements {settings.MultideviceBrowserProxy} + */ +class TestMultideviceBrowserProxy extends TestBrowserProxy { + constructor() { + super([ + 'setUpAndroidSms', + ]); + } + + /** @override */ + setUpAndroidSms() { + this.methodCalled('setUpAndroidSms'); + } +} + suite('Multidevice', function() { let multideviceSubpage = null; + let browserProxy = null; // Although HOST_SET_MODES is effectively a constant, it cannot reference the // enum settings.MultiDeviceSettingsMode from here so its initialization is // deferred to the suiteSetup function. @@ -60,6 +77,10 @@ suite('Multidevice', function() { }); setup(function() { + browserProxy = new TestMultideviceBrowserProxy(); + settings.MultiDeviceBrowserProxyImpl.instance_ = browserProxy; + + PolymerTest.clearBody(); multideviceSubpage = document.createElement('settings-multidevice-subpage'); multideviceSubpage.pageContentData = {hostDeviceName: 'Pixel XL'}; setMode(settings.MultiDeviceSettingsMode.HOST_SET_VERIFIED); @@ -133,4 +154,20 @@ suite('Multidevice', function() { assertFalse(!!multideviceSubpage.$$(controllerSelector)); }); + + test( + 'AndroidMessages set up button calls browser proxy function', function() { + const messagesItem = multideviceSubpage.$$('#messagesItem'); + + multideviceSubpage.androidMessagesRequiresSetup_ = true; + Polymer.dom.flush(); + + const setUpButton = + multideviceSubpage.$$('#messagesItem > [slot=feature-controller]'); + assertTrue(!!setUpButton); + + setUpButton.click(); + + return browserProxy.whenCalled('setUpAndroidSms'); + }); }); diff --git a/chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h b/chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h index bbc658b61bd148..96fb7572847583 100644 --- a/chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h +++ b/chromeos/services/multidevice_setup/public/cpp/android_sms_app_helper_delegate.h @@ -17,6 +17,9 @@ class AndroidSmsAppHelperDelegate { // Installs the Messages for Web PWA. Handles retries and errors internally. virtual void InstallAndroidSmsApp() = 0; + // Launches the Messages for Web PWA if it's installed. Returns true if the + // app was launched successfully, false otherwise. + virtual bool LaunchAndroidSmsApp() = 0; protected: AndroidSmsAppHelperDelegate() = default; diff --git a/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.cc b/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.cc index 21cee192f84dcb..df07759535ae3d 100644 --- a/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.cc +++ b/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.cc @@ -23,6 +23,15 @@ bool FakeAndroidSmsAppHelperDelegate::HasInstalledApp() { return has_installed_; } +bool FakeAndroidSmsAppHelperDelegate::LaunchAndroidSmsApp() { + has_launched_ = true; + return true; +} + +bool FakeAndroidSmsAppHelperDelegate::HasLaunchedApp() { + return has_launched_; +} + } // namespace multidevice_setup } // namespace chromeos diff --git a/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h b/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h index a5df4ce9bcf49b..28be5e14137dcf 100644 --- a/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h +++ b/chromeos/services/multidevice_setup/public/cpp/fake_android_sms_app_helper_delegate.h @@ -16,12 +16,15 @@ class FakeAndroidSmsAppHelperDelegate : public AndroidSmsAppHelperDelegate { FakeAndroidSmsAppHelperDelegate(); ~FakeAndroidSmsAppHelperDelegate() override; bool HasInstalledApp(); + bool HasLaunchedApp(); // AndroidSmsAppHelperDelegate: void InstallAndroidSmsApp() override; + bool LaunchAndroidSmsApp() override; private: bool has_installed_ = false; + bool has_launched_ = false; DISALLOW_COPY_AND_ASSIGN(FakeAndroidSmsAppHelperDelegate); };