Bootstrapping: Send meaningful error code/message from Slave to Master

chrome/browser/chromeos/login/helper.cc

 // Copyright (c) 2012 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/login/helper.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_scheduler/post_task.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/ui/webui_login_view.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/network_connection_handler.h"
 #include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_util.h"
 #include "components/guest_view/browser/guest_view_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/image/image_skia.h"
 
 namespace chromeos {
 
 namespace {
 
+const char kInvalidJsonError[] = "Invalid JSON Dictionary";

[achuithb, 2017/05/25 19:25:53]

Does this not need to be translated?

Also, I think we prefer constexpr

[xdai1, 2017/06/08 22:18:40]

Done. I think it should be fine to leave it untranslated. This error info will
only be visible to the admain admin for error diagnosis.

+
 // Gets the WebContents instance of current login display. If there is none,
 // returns nullptr.
 content::WebContents* GetLoginWebContents() {
   LoginDisplayHost* host = LoginDisplayHost::default_host();
   if (!host || !host->GetWebUILoginView())
     return nullptr;
 
   return host->GetWebUILoginView()->GetWebContents();
 }
 
@@ skipping 108 matching lines @@
   copied_onc->Set(onc::network_config::WifiProperty(onc::wifi::kHexSSID),
                   base::MakeUnique<base::Value>(hex_ssid));
   copied_onc->Set(onc::network_config::WifiProperty(onc::wifi::kSecurity),
                   base::MakeUnique<base::Value>(security));
   base::JSONWriter::Write(*copied_onc.get(), out_onc_spec);
 }
 
 void NetworkStateHelper::CreateAndConnectNetworkFromOnc(
     const std::string& onc_spec,
     const base::Closure& success_callback,
-    const base::Closure& error_callback) const {
+    const network_handler::ErrorCallback& error_callback) const {
   std::string error;
   std::unique_ptr<base::Value> root = base::JSONReader::ReadAndReturnError(
       onc_spec, base::JSON_ALLOW_TRAILING_COMMAS, nullptr, &error);
 
   base::DictionaryValue* toplevel_onc = nullptr;
   if (!root || !root->GetAsDictionary(&toplevel_onc)) {
-    LOG(ERROR) << "Invalid JSON Dictionary: " << error;
-    error_callback.Run();
+    std::unique_ptr<base::DictionaryValue> error_data(
+        new base::DictionaryValue);
+    error_data->SetString(network_handler::kErrorName, kInvalidJsonError);
+    error_data->SetString(network_handler::kErrorDetail, error);
+    LOG(ERROR) << kInvalidJsonError << ": " << error;

[achuithb, 2017/05/25 19:25:53]

Move this to right after the if statement?

[xdai1, 2017/06/08 22:18:40]

Done.

+    error_callback.Run(kInvalidJsonError, std::move(error_data));
     return;
   }
 
   NetworkHandler::Get()
       ->managed_network_configuration_handler()
       ->CreateConfiguration(
           "", *toplevel_onc,
           base::Bind(&NetworkStateHelper::OnCreateConfiguration,
                      base::Unretained(this), success_callback, error_callback),
-          base::Bind(&NetworkStateHelper::OnCreateOrConnectNetworkFailed,
-                     base::Unretained(this), error_callback));
+          error_callback);
 }
 
 bool NetworkStateHelper::IsConnected() const {
   chromeos::NetworkStateHandler* nsh =
       chromeos::NetworkHandler::Get()->network_state_handler();
   return nsh->ConnectedNetworkByType(chromeos::NetworkTypePattern::Default()) !=
          nullptr;
 }
 
 bool NetworkStateHelper::IsConnecting() const {
   chromeos::NetworkStateHandler* nsh =
       chromeos::NetworkHandler::Get()->network_state_handler();
   return nsh->ConnectingNetworkByType(
       chromeos::NetworkTypePattern::Default()) != nullptr;
 }
 
 void NetworkStateHelper::OnCreateConfiguration(
     const base::Closure& success_callback,
-    const base::Closure& error_callback,
+    const network_handler::ErrorCallback& error_callback,
     const std::string& service_path,
     const std::string& guid) const {
   // Connect to the network.
   NetworkHandler::Get()->network_connection_handler()->ConnectToNetwork(
-      service_path, success_callback,
-      base::Bind(&NetworkStateHelper::OnCreateOrConnectNetworkFailed,
-                 base::Unretained(this), error_callback),
-      false);
-}
-
-void NetworkStateHelper::OnCreateOrConnectNetworkFailed(
-    const base::Closure& error_callback,
-    const std::string& error_name,
-    std::unique_ptr<base::DictionaryValue> error_data) const {
-  LOG(ERROR) << "Failed to create or connect to network: " << error_name;
-  error_callback.Run();
+      service_path, success_callback, error_callback, false);
 }
 
 content::StoragePartition* GetSigninPartition() {
   content::WebContents* embedder = GetLoginWebContents();
   if (!embedder)
     return nullptr;
 
   // Note the partition name must match the sign-in webview used. For now,
   // this is the default unnamed, shared, in-memory partition.
   return GetPartition(embedder, std::string());
 }
 
 net::URLRequestContextGetter* GetSigninContext() {
   content::StoragePartition* signin_partition = GetSigninPartition();
 
   // Special case for unit tests. There's no LoginDisplayHost thus no
   // webview instance. See http://crbug.com/477402
   if (!signin_partition && !LoginDisplayHost::default_host())
     return ProfileHelper::GetSigninProfile()->GetRequestContext();
 
   if (!signin_partition)
     return nullptr;
 
   return signin_partition->GetURLRequestContext();
 }
 
 }  // namespace login
 
 }  // namespace chromeos