WebSocket Pepper API: error handling improvement

webkit/plugins/ppapi/ppb_websocket_impl.cc

 // Copyright (c) 2011 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 <string>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 46 matching lines @@
     return 0;
 
   uint64_t overhead = kHybiBaseFramingOverhead + kHybiMaskingKeyLength;
   if (payload_size > kMinimumPayloadSizeWithEightByteExtendedPayloadLength)
     overhead += 8;
   else if (payload_size > kMinimumPayloadSizeWithTwoByteExtendedPayloadLength)
     overhead += 2;
   return SaturateAdd(payload_size, overhead);
 }
 
+bool InValidStateToReceive(PP_WebSocketReadyState_Dev state) {
+  if (state != PP_WEBSOCKETREADYSTATE_OPEN_DEV &&
+      state != PP_WEBSOCKETREADYSTATE_CLOSING_DEV)
+    return false;
+  return true;

[dmichael (off chromium), 2011/12/02 16:28:09]

suggestion/nit: could also have just been:
return state == PP_WEBSOCKETREADYSTATE_OPEN_DEV ||
       state == PP_WEBSOCKETREADYSTATE_CLOSING_DEV;

That seems more readable to me.

[Takashi Toyoshima, 2011/12/02 17:15:42]

Sounds good.
Fixed.

+}
+
 }  // namespace
 
 namespace webkit {
 namespace ppapi {
 
 PPB_WebSocket_Impl::PPB_WebSocket_Impl(PP_Instance instance)
     : Resource(instance),
       state_(PP_WEBSOCKETREADYSTATE_INVALID_DEV),
+      error_was_received_(false),
       receive_callback_var_(NULL),
       wait_for_receive_(false),
       close_code_(0),
       close_was_clean_(PP_FALSE),
       buffered_amount_(0),
       buffered_amount_after_close_(0) {
   empty_string_ = new StringVar(
       PpapiGlobals::Get()->GetModuleForInstance(instance), "", 0);
 }
 
@@ skipping 157 matching lines @@
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_WebSocket_Impl::ReceiveMessage(PP_Var* message,
                                            PP_CompletionCallback callback) {
   // Check state.
   if (state_ == PP_WEBSOCKETREADYSTATE_INVALID_DEV ||
       state_ == PP_WEBSOCKETREADYSTATE_CONNECTING_DEV)
     return PP_ERROR_BADARGUMENT;
 
+  // Validate |callback| (Doesn't support blocking callback)
+  if (!callback.func)
+    return PP_ERROR_BLOCKS_MAIN_THREAD;
+
   // Just return received message if any received message is queued.
   if (!received_messages_.empty())
     return DoReceive();
 
+  // Returns PP_ERROR_FAILED after an error is received and received messages
+  // is exhausted.
+  if (error_was_received_)
+    return PP_ERROR_FAILED;
+
   // Or retain |message| as buffer to store and install |callback|.
   wait_for_receive_ = true;
   receive_callback_var_ = message;
   receive_callback_ = callback;
 
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_WebSocket_Impl::SendMessage(PP_Var message) {
   // Check mandatory interfaces.
@@ skipping 84 matching lines @@
   return url_->GetPPVar();
 }
 
 void PPB_WebSocket_Impl::didConnect() {
   DCHECK_EQ(PP_WEBSOCKETREADYSTATE_CONNECTING_DEV, state_);
   state_ = PP_WEBSOCKETREADYSTATE_OPEN_DEV;
   PP_RunAndClearCompletionCallback(&connect_callback_, PP_OK);
 }
 
 void PPB_WebSocket_Impl::didReceiveMessage(const WebString& message) {
+  // Dispose packets after receiving an error or in invalid state.
+  if (error_was_received_ || !InValidStateToReceive(state_))
+    return;
+
   // Append received data to queue.
   std::string string = message.utf8();
   PP_Var var = StringVar::StringToPPVar(
       PpapiGlobals::Get()->GetModuleForInstance(pp_instance()), string);
   received_messages_.push(var);
 
   if (!wait_for_receive_)
     return;
 
   PP_RunAndClearCompletionCallback(&receive_callback_, DoReceive());
 }
 
 void PPB_WebSocket_Impl::didReceiveBinaryData(const WebData& binaryData) {
+  // Dispose packets after receiving an error or in invalid state.
+  if (error_was_received_ || !InValidStateToReceive(state_))
+    return;
+
+  // TODO(toyoshim): Support to receive binary data.
   DLOG(INFO) << "didReceiveBinaryData is not implemented yet.";
-  // TODO(toyoshim): Support to receive binary data.
 }
 
 void PPB_WebSocket_Impl::didReceiveMessageError() {
-  // TODO(toyoshim): Must implement.
-  DLOG(INFO) << "didReceiveMessageError is not implemented yet.";
+  // Ignore error notification in invalid state.
+  if (!InValidStateToReceive(state_))
+    return;
+
+  // Records the error, then stops receiving any frames after this error.
+  // The error will be notified after all queued messages are read via
+  // ReceiveMessage().
+  error_was_received_ = true;
+  if (!wait_for_receive_)
+    return;
+
+  // But, if no messages is queued and ReceiveMessage() is now on going.

[dmichael (off chromium), 2011/12/02 16:28:09]

nit: is -> are

[Takashi Toyoshima, 2011/12/02 17:15:42]

Done.

+  // We must invoke the callback with error code here.
+  wait_for_receive_ = false;
+  PP_RunAndClearCompletionCallback(&receive_callback_, PP_ERROR_FAILED);
 }
 
 void PPB_WebSocket_Impl::didUpdateBufferedAmount(
     unsigned long buffered_amount) {
   if (state_ == PP_WEBSOCKETREADYSTATE_CLOSED_DEV)
     return;
   buffered_amount_ = buffered_amount;
 }
 
 void PPB_WebSocket_Impl::didStartClosingHandshake() {
-  // TODO(toyoshim): Must implement.
-  DLOG(INFO) << "didStartClosingHandshake is not implemented yet.";
+  state_ = PP_WEBSOCKETREADYSTATE_CLOSING_DEV;
 }
 
-void PPB_WebSocket_Impl::didClose(unsigned long buffered_amount,
+void PPB_WebSocket_Impl::didClose(unsigned long unhandled_buffered_amount,
                                   ClosingHandshakeCompletionStatus status,
                                   unsigned short code,
                                   const WebString& reason) {
   // Store code and reason.
   close_code_ = code;
   std::string reason_string = reason.utf8();
   close_reason_ = new StringVar(
       PpapiGlobals::Get()->GetModuleForInstance(pp_instance()), reason_string);
 
-  // TODO(toyoshim): Set close_was_clean_.
+  // Set close_was_clean_.
+  bool was_clean =
+      state_ == PP_WEBSOCKETREADYSTATE_CLOSING_DEV &&
+      !unhandled_buffered_amount &&
+      status == WebSocketClient::ClosingHandshakeComplete;
+  close_was_clean_ = was_clean ? PP_TRUE : PP_FALSE;
+
+  // Update buffered_amount_.
+  buffered_amount_ = unhandled_buffered_amount;
 
   // Handle state transition and invoking callback.
   DCHECK_NE(PP_WEBSOCKETREADYSTATE_CLOSED_DEV, state_);
   PP_WebSocketReadyState_Dev state = state_;
   state_ = PP_WEBSOCKETREADYSTATE_CLOSED_DEV;
 
-  // Update buffered_amount_.
-  buffered_amount_ = buffered_amount;
-
   if (state == PP_WEBSOCKETREADYSTATE_CONNECTING_DEV)
     PP_RunAndClearCompletionCallback(&connect_callback_, PP_OK);
 
   if (state == PP_WEBSOCKETREADYSTATE_CLOSING_DEV)
     PP_RunAndClearCompletionCallback(&close_callback_, PP_OK);
+
+  // Disconnect.
+  if (websocket_.get())
+    websocket_->disconnect();
 }
 
 int32_t PPB_WebSocket_Impl::DoReceive() {
-  // TODO(toyoshim): Check state.
-
   if (!receive_callback_var_)
     return PP_OK;
 
   *receive_callback_var_ = received_messages_.front();
   received_messages_.pop();
   receive_callback_var_ = NULL;
   wait_for_receive_ = false;
   return PP_OK;
 }
 
 }  // namespace ppapi
 }  // namespace webkit