Extract ArcSupportMessageHost.

chrome/browser/chromeos/arc/arc_support_host.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
 
 #include <string>
 
 #include "ash/common/system/chromeos/devicetype_utils.h"
 #include "base/i18n/timezone.h"
@@ skipping 60 matching lines @@
 constexpr char kIsMetricsEnabled[] = "isMetricsEnabled";
 constexpr char kIsBackupRestoreEnabled[] = "isBackupRestoreEnabled";
 constexpr char kIsLocationServiceEnabled[] = "isLocationServiceEnabled";
 
 // "onSendFeedbackClicked" is fired when a user clicks "Send Feedback" button.
 constexpr char kEventOnSendFeedbackClicked[] = "onSendFeedbackClicked";
 
 }  // namespace
 
 // static
-const char ArcSupportHost::kHostName[] = "com.google.arc_support";
-
-// static
 const char ArcSupportHost::kHostAppId[] = "cnbgggchhmkkdmeppjobngjoejnihlei";
 
 // static
 const char ArcSupportHost::kStorageId[] = "arc_support";
 
-// static
-const char* const ArcSupportHost::kHostOrigin[] = {
-    "chrome-extension://cnbgggchhmkkdmeppjobngjoejnihlei/"};
-
-// static
-std::unique_ptr<extensions::NativeMessageHost> ArcSupportHost::Create() {
-  return std::unique_ptr<NativeMessageHost>(new ArcSupportHost());
-}
-
 ArcSupportHost::ArcSupportHost() {
+  // TODO(hidehiko): Get rid of dependency to ArcAuthService.
   arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
   DCHECK(arc_auth_service);
 
   if (!arc_auth_service->IsAllowed())
     return;
 
-  arc_auth_service->AddObserver(this);
-  display::Screen::GetScreen()->AddObserver(this);
-
-  pref_local_change_registrar_.Init(g_browser_process->local_state());
-  pref_local_change_registrar_.Add(
-      metrics::prefs::kMetricsReportingEnabled,
-      base::Bind(&ArcSupportHost::OnMetricsPreferenceChanged,
-                 base::Unretained(this)));
+  if (g_browser_process->local_state()) {

[hidehiko, 2016/10/20 18:36:17]

Note: No service is running on unittest, if stmt is workaround.
This will be cleaned, when the logic is extracted.

[Luis Héctor Chávez, 2016/10/21 02:24:08]

Can you add a TODO?

[hidehiko, 2016/10/21 07:40:27]

Done.

+    pref_local_change_registrar_.Init(g_browser_process->local_state());
+    pref_local_change_registrar_.Add(
+        metrics::prefs::kMetricsReportingEnabled,
+        base::Bind(&ArcSupportHost::OnMetricsPreferenceChanged,
+                   base::Unretained(this)));
+  }
 
   pref_change_registrar_.Init(arc_auth_service->profile()->GetPrefs());
   pref_change_registrar_.Add(
       prefs::kArcBackupRestoreEnabled,
       base::Bind(&ArcSupportHost::OnBackupAndRestorePreferenceChanged,
                  base::Unretained(this)));
   pref_change_registrar_.Add(
       prefs::kArcLocationServiceEnabled,
       base::Bind(&ArcSupportHost::OnLocationServicePreferenceChanged,
                  base::Unretained(this)));
 }
 
 ArcSupportHost::~ArcSupportHost() {
-  display::Screen::GetScreen()->RemoveObserver(this);
-  arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
-  if (arc_auth_service)
-    arc_auth_service->RemoveObserver(this);
+  if (message_host_)
+    DisconnectMessageHost();
 }
 
 void ArcSupportHost::Close() {
-  if (!client_)
+  if (!message_host_)
     return;
 
-  close_requested_ = true;
-  base::DictionaryValue response;
-  response.SetString(kAction, kActionCloseWindow);
-  std::string response_string;
-  base::JSONWriter::Write(response, &response_string);
-  client_->PostMessageFromNativeHost(response_string);
+  base::DictionaryValue message;
+  message.SetString(kAction, kActionCloseWindow);
+  message_host_->SendMessage(message);
+
+  // Disconnect immediately, so that onWindowClosed event will not be
+  // delivered to here.

[Luis Héctor Chávez, 2016/10/21 02:24:08]

nit: s/delivered to here/delivered here/

+  DisconnectMessageHost();
 }
 
-void ArcSupportHost::Start(Client* client) {
-  DCHECK(!client_);
-  client_ = client;
+void ArcSupportHost::ShowPage(arc::ArcAuthService::UIPage page,
+                              const base::string16& status) {
+  if (!message_host_)
+    return;

[khmel, 2016/10/21 03:34:49]

IIUC this should never happen. May be LOG(ERROR)? 

[hidehiko, 2016/10/21 07:40:27]

This happens. Because ArcSupportMessageHost creation is async, when this is
called, it is no guarantee that message_host_ is ready.

[khmel, 2016/10/21 14:11:17]

I have concern about losing data for user (not shown error or other important
information is bad). Normally, IIUC, when this happens that means page is
creating in the background and will be shown soon. But if not, and page could
not be created? In this case in log we would have nothing. Not sure how to
handle this better. May be set flag that indicates we have pending request to
show the page and on closing to analyze that we reset it.

[hidehiko, 2016/10/24 06:03:26]

Currently, no data loss is ensured in the ArcAuthService.
If the page show event is fired in ArcAuthService before the actual window
creation (or, in more precise, before ArcSupportMessageHost is set to
ArcSupportHost), the ArcAuthService's fields remember it, and Initialize() sends
them to the extension.
It is just because the existing code is so, and I think it is not good design.
I'll send another CL to move the responsibility to this class immediately after
this CL ges landed. Then, client won't be necessary to take care about such a
timing issue.

[khmel, 2016/10/24 15:05:36]

Acknowledged.

+
+  base::DictionaryValue message;
+  message.SetString(kAction, kActionShowPage);
+  message.SetInteger(kPage, static_cast<int>(page));
+  message.SetString(kStatus, status);
+  message_host_->SendMessage(message);
+}
+
+void ArcSupportHost::SetMessageHost(arc::ArcSupportMessageHost* message_host) {

[Luis Héctor Chávez, 2016/10/21 02:24:07]

if (message_host_ == message_host)
  return;

for symmetry?

[hidehiko, 2016/10/21 07:40:27]

Done.

+  if (message_host_)
+    DisconnectMessageHost();
+  message_host_ = message_host;
+  message_host_->SetObserver(this);
+  display::Screen::GetScreen()->AddObserver(this);
 
   if (!Initialize()) {
     Close();
     return;
   }
 
   SendMetricsMode();
   SendBackupAndRestoreMode();
   SendLocationServicesMode();
 
   arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
   DCHECK(arc_auth_service);
-  OnOptInUIShowPage(arc_auth_service->ui_page(),
-                    arc_auth_service->ui_page_status());
+  ShowPage(arc_auth_service->ui_page(), arc_auth_service->ui_page_status());
+}
+
+void ArcSupportHost::UnsetMessageHost(
+    arc::ArcSupportMessageHost* message_host) {
+  if (message_host_ != message_host)
+    return;
+  DisconnectMessageHost();
+}
+
+void ArcSupportHost::DisconnectMessageHost() {
+  display::Screen::GetScreen()->RemoveObserver(this);

[Luis Héctor Chávez, 2016/10/21 02:24:07]

DCHECK(message_host_); ?

[hidehiko, 2016/10/21 07:40:27]

Done.

+  message_host_->SetObserver(nullptr);
+  message_host_ = nullptr;
 }
 
 bool ArcSupportHost::Initialize() {
-  DCHECK(client_);
+  DCHECK(message_host_);
   arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
   if (!arc_auth_service->IsAllowed())
     return false;
 
   std::unique_ptr<base::DictionaryValue> loadtime_data(
       new base::DictionaryValue());
   base::string16 device_name = ash::GetChromeOSDeviceName();
   loadtime_data->SetString(
       "greetingHeader",
       l10n_util::GetStringFUTF16(IDS_ARC_OPT_IN_DIALOG_HEADER, device_name));
@@ skipping 71 matching lines @@
 
   webui::SetLoadTimeDataDefaults(app_locale, loadtime_data.get());
   DCHECK(arc_auth_service);
   const std::string device_id = user_manager::known_user::GetDeviceId(
       multi_user_util::GetAccountIdFromProfile(arc_auth_service->profile()));
   DCHECK(!device_id.empty());
   loadtime_data->SetBoolean(
       "isOwnerProfile",
       chromeos::ProfileHelper::IsOwnerProfile(arc_auth_service->profile()));
 
-  base::DictionaryValue request;
-  std::string request_string;
-  request.SetString(kAction, kActionInitialize);
-  request.Set(kData, std::move(loadtime_data));
-  request.SetString(kDeviceId, device_id);
-  base::JSONWriter::Write(request, &request_string);
-  client_->PostMessageFromNativeHost(request_string);
-
+  base::DictionaryValue message;
+  message.SetString(kAction, kActionInitialize);
+  message.Set(kData, std::move(loadtime_data));
+  message.SetString(kDeviceId, device_id);
+  message_host_->SendMessage(message);
   return true;
 }
 
 void ArcSupportHost::OnDisplayAdded(const display::Display& new_display) {}
 
 void ArcSupportHost::OnDisplayRemoved(const display::Display& old_display) {}
 
 void ArcSupportHost::OnDisplayMetricsChanged(const display::Display& display,
                                              uint32_t changed_metrics) {
-  base::DictionaryValue request;
-  std::string request_string;
-  request.SetString(kAction, kActionSetWindowBounds);
-  base::JSONWriter::Write(request, &request_string);
-  client_->PostMessageFromNativeHost(request_string);
+  if (!message_host_)
+    return;
+
+  base::DictionaryValue message;
+  message.SetString(kAction, kActionSetWindowBounds);
+  message_host_->SendMessage(message);
 }
 
 void ArcSupportHost::OnMetricsPreferenceChanged() {
   SendMetricsMode();
 }
 
 void ArcSupportHost::OnBackupAndRestorePreferenceChanged() {
   SendBackupAndRestoreMode();
 }
 
@@ skipping 22 matching lines @@
   DCHECK(arc_auth_service);
   SendPreferenceUpdate(
       action_name,
       arc_auth_service->profile()->GetPrefs()->GetBoolean(pref_name),
       arc_auth_service->profile()->GetPrefs()->IsManagedPreference(pref_name));
 }
 
 void ArcSupportHost::SendPreferenceUpdate(const std::string& action_name,
                                           bool is_enabled,
                                           bool is_managed) {
-  base::DictionaryValue request;
-  std::string request_string;
-  request.SetString(kAction, action_name);
-  request.SetBoolean(kEnabled, is_enabled);
-  request.SetBoolean(kManaged, is_managed);
-  base::JSONWriter::Write(request, &request_string);
-  client_->PostMessageFromNativeHost(request_string);
-}
-
-void ArcSupportHost::OnOptInUIClose() {
-  Close();
-}
-
-void ArcSupportHost::OnOptInUIShowPage(arc::ArcAuthService::UIPage page,
-                                       const base::string16& status) {
-  if (!client_)
+  if (!message_host_)
     return;
 
-  base::DictionaryValue response;
-  response.SetString(kAction, kActionShowPage);
-  response.SetInteger(kPage, static_cast<int>(page));
-  response.SetString(kStatus, status);
-  std::string response_string;
-  base::JSONWriter::Write(response, &response_string);
-  client_->PostMessageFromNativeHost(response_string);
+  base::DictionaryValue message;
+  message.SetString(kAction, action_name);
+  message.SetBoolean(kEnabled, is_enabled);
+  message.SetBoolean(kManaged, is_managed);
+  message_host_->SendMessage(message);
 }
 
 void ArcSupportHost::EnableMetrics(bool is_enabled) {
   ChangeMetricsReportingState(is_enabled);
 }
 
 void ArcSupportHost::EnableBackupRestore(bool is_enabled) {
   arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
   DCHECK(arc_auth_service && arc_auth_service->IsAllowed());
   PrefService* pref_service = arc_auth_service->profile()->GetPrefs();
   pref_service->SetBoolean(prefs::kArcBackupRestoreEnabled, is_enabled);
 }
 
 void ArcSupportHost::EnableLocationService(bool is_enabled) {
   arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
   DCHECK(arc_auth_service && arc_auth_service->IsAllowed());
   PrefService* pref_service = arc_auth_service->profile()->GetPrefs();
   pref_service->SetBoolean(prefs::kArcLocationServiceEnabled, is_enabled);
 }
 
-void ArcSupportHost::OnMessage(const std::string& message_string) {
-  std::unique_ptr<base::Value> message_value =
-      base::JSONReader::Read(message_string);
-  base::DictionaryValue* message;
-  if (!message_value || !message_value->GetAsDictionary(&message)) {
+void ArcSupportHost::OnMessage(const base::DictionaryValue& message) {
+  std::string event;
+  if (!message.GetString(kEvent, &event)) {
     NOTREACHED();
     return;
   }
-
-  std::string event;
-  if (!message->GetString(kEvent, &event)) {
-    NOTREACHED();
-    return;
-  }
 
   // TODO(hidehiko): Replace by Observer.
   arc::ArcAuthService* arc_auth_service = arc::ArcAuthService::Get();
   DCHECK(arc_auth_service);
 
   if (event == kEventOnWindowClosed) {
-    if (!close_requested_)
-      arc_auth_service->CancelAuthCode();
+    arc_auth_service->CancelAuthCode();
   } else if (event == kEventOnAuthSuccedded) {
     std::string code;
-    if (message->GetString(kCode, &code)) {
+    if (message.GetString(kCode, &code)) {
       arc_auth_service->SetAuthCodeAndStartArc(code);
     } else {
       NOTREACHED();
     }
   } else if (event == kEventOnAgreed) {
     bool is_metrics_enabled;
     bool is_backup_restore_enabled;
     bool is_location_service_enabled;
-    if (message->GetBoolean(kIsMetricsEnabled, &is_metrics_enabled) &&
-        message->GetBoolean(kIsBackupRestoreEnabled,
-                            &is_backup_restore_enabled) &&
-        message->GetBoolean(kIsLocationServiceEnabled,
-                            &is_location_service_enabled)) {
+    if (message.GetBoolean(kIsMetricsEnabled, &is_metrics_enabled) &&
+        message.GetBoolean(kIsBackupRestoreEnabled,
+                           &is_backup_restore_enabled) &&
+        message.GetBoolean(kIsLocationServiceEnabled,
+                           &is_location_service_enabled)) {
       EnableMetrics(is_metrics_enabled);
       EnableBackupRestore(is_backup_restore_enabled);
       EnableLocationService(is_location_service_enabled);
       arc_auth_service->StartLso();
     } else {
       NOTREACHED();
     }
   } else if (event == kEventOnSendFeedbackClicked) {
     chrome::OpenFeedbackDialog(nullptr);
   } else {
-    LOG(ERROR) << "Unknown message: " << message_string;
+    LOG(ERROR) << "Unknown message: " << event;
     NOTREACHED();
   }
 }
-
-scoped_refptr<base::SingleThreadTaskRunner> ArcSupportHost::task_runner()
-    const {
-  return base::ThreadTaskRunnerHandle::Get();
-}