cast channel error cleanup

extensions/browser/api/cast_channel/cast_channel_api.cc

Index: extensions/browser/api/cast_channel/cast_channel_api.cc
diff --git a/extensions/browser/api/cast_channel/cast_channel_api.cc b/extensions/browser/api/cast_channel/cast_channel_api.cc
index c2c083c79cfe234a4040486b111dbcf7f4e0963c..d62f49902d0869f39eaf9cfce7e041ec191d18e3 100644
--- a/extensions/browser/api/cast_channel/cast_channel_api.cc
+++ b/extensions/browser/api/cast_channel/cast_channel_api.cc
@@ -173,7 +173,8 @@ void CastChannelAPI::OnMessage(const CastSocket* socket,
 CastChannelAPI::~CastChannelAPI() {}
 
 CastChannelAsyncApiFunction::CastChannelAsyncApiFunction()
-  : manager_(NULL), error_(cast_channel::CHANNEL_ERROR_NONE) { }
+    : manager_(NULL), channel_error_(cast_channel::CHANNEL_ERROR_NONE) {
+}
 
 CastChannelAsyncApiFunction::~CastChannelAsyncApiFunction() { }
 
@@ -183,7 +184,7 @@ bool CastChannelAsyncApiFunction::PrePrepare() {
 }
 
 bool CastChannelAsyncApiFunction::Respond() {
-  return error_ == cast_channel::CHANNEL_ERROR_NONE;
+  return channel_error_ == cast_channel::CHANNEL_ERROR_NONE;

[mark a. foltz, 2014/10/31 22:52:35]

It looks like we only use channel_error_ to indicate whether the function was
successful or not, and don't use the value (after putting it in the ChannelInfo
result).

IMO it would be cleaner to replace it with an error_ boolean that gets set by
SetResultFromError().

Maybe this then becomes:
return error_ || !GetError().empty()

[vadimgo, 2014/10/31 23:32:39]

Yes, it is cleaner with a bool. The logic became

  return !is_channel_error_ && GetError().empty();

 }
 
 CastSocket* CastChannelAsyncApiFunction::GetSocketOrCompleteWithError(
@@ -216,22 +217,29 @@ void CastChannelAsyncApiFunction::SetResultFromSocket(
     const CastSocket& socket) {
   ChannelInfo channel_info;
   FillChannelInfo(socket, &channel_info);
-  error_ = socket.error_state();
+  channel_error_ = socket.error_state();
   SetResultFromChannelInfo(channel_info);
 }
 
-void CastChannelAsyncApiFunction::SetResultFromError(int channel_id,
-                                                     ChannelError error) {
+void CastChannelAsyncApiFunction::SetResultFromError(
+    int channel_id,
+    ChannelError channel_error) {
   ChannelInfo channel_info;
   channel_info.channel_id = channel_id;
   channel_info.url = "";
   channel_info.ready_state = cast_channel::READY_STATE_CLOSED;
-  channel_info.error_state = error;
+  channel_info.error_state = channel_error;
   channel_info.connect_info.ip_address = "";
   channel_info.connect_info.port = 0;
   channel_info.connect_info.auth = cast_channel::CHANNEL_AUTH_TYPE_SSL;
   SetResultFromChannelInfo(channel_info);
-  error_ = error;
+  channel_error_ = channel_error;
+}
+
+void CastChannelAsyncApiFunction::SetChannelError(const std::string& error,

[mark a. foltz, 2014/10/31 22:52:35]

With the change I mentioned this may no longer be necessary.

[vadimgo, 2014/10/31 23:32:39]

Done.

+                                                  ChannelError channel_error) {
+  SetError(error);
+  channel_error_ = channel_error;
 }
 
 CastSocket* CastChannelAsyncApiFunction::GetSocket(int channel_id) {
@@ -321,28 +329,29 @@ bool CastChannelOpenFunction::Prepare() {
       connect_info_.reset(new ConnectInfo);
       if (!ParseChannelUrl(GURL(cast_url), connect_info_.get())) {
         connect_info_.reset();
-        SetError("Invalid connect_info (invalid Cast URL " + cast_url + ")");
+        SetChannelError("Invalid connect_info (invalid Cast URL " + cast_url +
+                        ")");
       }
       break;
     case base::Value::TYPE_DICTIONARY:
       connect_info_ = ConnectInfo::FromValue(*(params_->connect_info));
       if (!connect_info_.get()) {
-        SetError("connect_info.auth is required");
+        SetChannelError("connect_info.auth is required");
       }
       break;
     default:
-      SetError("Invalid connect_info (unknown type)");
+      SetChannelError("Invalid connect_info (unknown type)");
       break;
   }
   if (!connect_info_.get()) {
     return false;
   }
   if (!IsValidConnectInfoPort(*connect_info_)) {
-    SetError("Invalid connect_info (invalid port)");
+    SetChannelError("Invalid connect_info (invalid port)");
   } else if (!IsValidConnectInfoAuth(*connect_info_)) {
-    SetError("Invalid connect_info (invalid auth)");
+    SetChannelError("Invalid connect_info (invalid auth)");
   } else if (!IsValidConnectInfoIpAddress(*connect_info_)) {
-    SetError("Invalid connect_info (invalid IP address)");
+    SetChannelError("Invalid connect_info (invalid IP address)");
   }
   if (!GetError().empty()) {
     return false;
@@ -395,15 +404,15 @@ bool CastChannelSendFunction::Prepare() {
   params_ = Send::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(params_.get());
   if (params_->message.namespace_.empty()) {
-    SetError("message_info.namespace_ is required");
+    SetChannelError("message_info.namespace_ is required");
     return false;
   }
   if (params_->message.source_id.empty()) {
-    SetError("message_info.source_id is required");
+    SetChannelError("message_info.source_id is required");
     return false;
   }
   if (params_->message.destination_id.empty()) {
-    SetError("message_info.destination_id is required");
+    SetChannelError("message_info.destination_id is required");
     return false;
   }
   switch (params_->message.data->GetType()) {
@@ -411,7 +420,7 @@ bool CastChannelSendFunction::Prepare() {
     case base::Value::TYPE_BINARY:
       break;
     default:
-      SetError("Invalid type of message_info.data");
+      SetChannelError("Invalid type of message_info.data");
       return false;
   }
   return true;
@@ -503,7 +512,7 @@ void CastChannelGetLogsFunction::AsyncWorkStart() {
   if (out.get()) {
     SetResult(new base::BinaryValue(out.Pass(), length));
   } else {
-    SetError("Unable to get logs.");
+    SetChannelError("Unable to get logs.");
   }
 
   api_->GetLogger()->Reset();
@@ -528,7 +537,7 @@ void CastChannelSetAuthorityKeysFunction::AsyncWorkStart() {
   std::string& signature = params_->signature;
   if (signature.empty() || keys.empty() ||
       !cast_channel::SetTrustedCertificateAuthorities(keys, signature)) {
-    SetError("Unable to set authority keys.");
+    SetChannelError("Unable to set authority keys.");
   }
 
   AsyncWorkCompleted();