WebSocket Pepper API: allow to close connection without code and reason

webkit/plugins/ppapi/ppb_websocket_impl.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 "webkit/plugins/ppapi/ppb_websocket_impl.h"
 
 #include <set>
 #include <string>
 
 #include "base/basictypes.h"
@@ skipping 186 matching lines @@
 
   websocket_->connect(web_url, web_protocols);
   state_ = PP_WEBSOCKETREADYSTATE_CONNECTING;
 
   // Install callback.
   connect_callback_ = new TrackedCallback(this, callback);
 
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_WebSocket_Impl::Close(uint16_t code,

[dmichael (off chromium), 2012/05/14 16:10:49]

I think this should have changed to enum once the enum got added in the C API. I
think it should be okay to change it; it's a source incompatibility, but not an
ABI compatibility problem. What do you think?

[Takashi Toyoshima, 2012/05/15 10:55:28]

I see. SRC handles the code as int32_t, so we can safely change its type from
uint16_t to enum in the same ABI. That sound great. I'll use enum for this.

                                   PP_Var reason,
                                   PP_CompletionCallback callback) {
   // Check mandarory interfaces.
   if (!websocket_.get())
     return PP_ERROR_FAILED;
 
-  // Validate |code|. Need to cast |CloseEventCodeNotSpecified| which is -1 to
-  // uint16_t for the comparison to work.
+  // Validate |code| and |reason|. Need to cast |CloseEventCodeNotSpecified|
+  // which is -1 to uint16_t for the comparison to work. |reason| is valid only
+  // when |code| is not PP_WEBSOCKETSTATUSCODE_NOT_SPECIFIED.
+  scoped_refptr<StringVar> reason_string;
+  WebString web_reason;
   if (code != static_cast<uint16_t>(WebSocket::CloseEventCodeNotSpecified)) {
     if (!(code == WebSocket::CloseEventCodeNormalClosure ||
         (WebSocket::CloseEventCodeMinimumUserDefined <= code &&
         code <= WebSocket::CloseEventCodeMaximumUserDefined)))
     // RFC 6455 limits applications to use reserved connection close code in
     // section 7.4.2.. The WebSocket API (http://www.w3.org/TR/websockets/)
     // defines this out of range error as InvalidAccessError in JavaScript.
     return PP_ERROR_NOACCESS;
-  }
 
-  scoped_refptr<StringVar> reason_string;
-  WebString web_reason;
-  // |reason| must be ignored if it is PP_VARTYPE_UNDEFINED.
-  if (reason.type != PP_VARTYPE_UNDEFINED) {
-    // Validate |reason|.
-    reason_string = StringVar::FromPPVar(reason);
-    if (!reason_string ||
-        reason_string->value().size() > kMaxReasonSizeInBytes)
-      return PP_ERROR_BADARGUMENT;
-    web_reason = WebString::fromUTF8(reason_string->value());
+    // |reason| must be ignored if it is PP_VARTYPE_UNDEFINED.

[Takashi Toyoshima, 2012/05/14 12:05:04]

I moved reason handling code inside of 'code != not specified' check.
To tell the truth, this change is not mandatory because WebKit will ignore the
reason when code is not specified. But this new version will be better because
it doesn't depend on WebKit side implementation and it can remove redundant
reason string handling.

+    if (reason.type != PP_VARTYPE_UNDEFINED) {
+      // Validate |reason|.
+      reason_string = StringVar::FromPPVar(reason);
+      if (!reason_string ||
+          reason_string->value().size() > kMaxReasonSizeInBytes)
+        return PP_ERROR_BADARGUMENT;
+      web_reason = WebString::fromUTF8(reason_string->value());
+    }
   }
 
   // Check state.
   if (state_ == PP_WEBSOCKETREADYSTATE_CLOSING)
     return PP_ERROR_INPROGRESS;
   if (state_ == PP_WEBSOCKETREADYSTATE_CLOSED)
     return PP_OK;
 
   // Validate |callback| (Doesn't support blocking callback)
   if (!callback.func)
@@ skipping 283 matching lines @@
 
   *receive_callback_var_ = received_messages_.front()->GetPPVar();
   received_messages_.pop();
   receive_callback_var_ = NULL;
   wait_for_receive_ = false;
   return PP_OK;
 }
 
 }  // namespace ppapi
 }  // namespace webkit