WebSocket Pepper API: PPB_WebSocket::close must accept undefined reason

webkit/plugins/ppapi/ppb_websocket_impl.cc

Index: webkit/plugins/ppapi/ppb_websocket_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_websocket_impl.cc b/webkit/plugins/ppapi/ppb_websocket_impl.cc
index 9157b67c92f896a90d622c407f718452cbc658de..25e42ee61baf805f9bdb921418c61596c37853a6 100644
--- a/webkit/plugins/ppapi/ppb_websocket_impl.cc
+++ b/webkit/plugins/ppapi/ppb_websocket_impl.cc
@@ -223,12 +223,19 @@ int32_t PPB_WebSocket_Impl::Close(uint16_t code,
     return PP_ERROR_NOACCESS;
   }
 
-  // Validate |reason|.
-  // TODO(toyoshim): Returns PP_ERROR_BADARGUMENT if |reason| contains any
-  // surrogates.
-  scoped_refptr<StringVar> reason_string = StringVar::FromPPVar(reason);
-  if (!reason_string || reason_string->value().size() > kMaxReasonSizeInBytes)
-    return PP_ERROR_BADARGUMENT;
+  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|.
+    // TODO(toyoshim): Returns PP_ERROR_BADARGUMENT if |reason| contains any
+    // surrogates.

[dmichael (off chromium), 2012/04/23 15:14:46]

I'm not sure what you mean here, but I'm guessing it suffices to check that the
string is valid UTF-8? VarFromUTF8 does that, so by the time you get the PP_Var,
it's already validated. I think you can remove this comment.

[Takashi Toyoshima, 2012/04/24 06:44:57]

Thank you.
I check VarFromUTF8 implementation. As you said, it includes surrogates check.

FYI; Surrogate pair is defined in UTF-16 to expand the number of characters in
16-bit. It means UTF-16 has multi-word characters. These pairs consist of
special characters from 0xd800 to 0xdfff. UTF-8 must not include these codes
after it is decoded to Unicode.

+    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 ||
@@ -265,7 +272,6 @@ int32_t PPB_WebSocket_Impl::Close(uint16_t code,
 
   // Close connection.
   state_ = PP_WEBSOCKETREADYSTATE_CLOSING;
-  WebString web_reason = WebString::fromUTF8(reason_string->value());
   websocket_->close(code, web_reason);
 
   return PP_OK_COMPLETIONPENDING;