Implementation and fixes for Blimp client/engine E2E communication.

blimp/net/blimp_connection.cc

 // Copyright 2015 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 "blimp/net/blimp_connection.h"
 
 #include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "blimp/common/proto/blimp_message.pb.h"
 #include "blimp/net/blimp_message_processor.h"
 #include "blimp/net/blimp_message_pump.h"
 #include "blimp/net/common.h"
 #include "blimp/net/connection_error_observer.h"
 #include "blimp/net/packet_reader.h"
 #include "blimp/net/packet_writer.h"
 #include "net/base/completion_callback.h"
 
@@ skipping 11 matching lines @@
   }
 
   // BlimpMessageProcessor implementation.
   void ProcessMessage(scoped_ptr<BlimpMessage> message,
                       const net::CompletionCallback& callback) override;
 
  private:
   void OnWritePacketComplete(int result);
 
   PacketWriter* writer_;
-  ConnectionErrorObserver* error_observer_;
-  scoped_refptr<net::DrainableIOBuffer> buffer_;

[haibinlu, 2015/12/29 00:51:45]

can you sync to the head? The base seems out-of-date.

[Kevin M, 2015/12/30 23:08:49]

Done.

+  ConnectionErrorObserver* error_observer_ = nullptr;
+  scoped_refptr<net::IOBuffer> buffer_;
   net::CompletionCallback pending_process_msg_callback_;
+  base::WeakPtrFactory<BlimpMessageSender> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BlimpMessageSender);
 };
 
 BlimpMessageSender::BlimpMessageSender(PacketWriter* writer)
     : writer_(writer),
-      buffer_(new net::DrainableIOBuffer(
-          new net::IOBuffer(kMaxPacketPayloadSizeBytes),
-          kMaxPacketPayloadSizeBytes)) {
+      buffer_(new net::IOBuffer(kMaxPacketPayloadSizeBytes)),
+      weak_factory_(this) {
   DCHECK(writer_);
 }
 
-BlimpMessageSender::~BlimpMessageSender() {}
+BlimpMessageSender::~BlimpMessageSender() {
+  DVLOG(1) << "BlimpMessageSender destroyed.";
+}
 
 void BlimpMessageSender::ProcessMessage(
     scoped_ptr<BlimpMessage> message,
     const net::CompletionCallback& callback) {
+  DCHECK(error_observer_);
+  DVLOG(2) << "ProcessMessage, size=" << message->ByteSize();
   if (message->ByteSize() > static_cast<int>(kMaxPacketPayloadSizeBytes)) {
     DLOG(ERROR) << "Message is too big, size=" << message->ByteSize();
     callback.Run(net::ERR_MSG_TOO_BIG);
     return;
   }
 
-  buffer_->SetOffset(0);
-  if (!message->SerializeToArray(buffer_->data(), message->ByteSize())) {
+  scoped_refptr<net::DrainableIOBuffer> drainable_buffer(
+      new net::DrainableIOBuffer(buffer_.get(), message->ByteSize()));
+  if (!message->SerializeToArray(drainable_buffer->data(),
+                                 message->GetCachedSize())) {
     DLOG(ERROR) << "Failed to serialize message.";
     callback.Run(net::ERR_INVALID_ARGUMENT);
     return;
   }
 
+  // Check that no other message writes are in-flight at this time.
+  DCHECK(pending_process_msg_callback_.is_null());
   pending_process_msg_callback_ = callback;
-  writer_->WritePacket(buffer_,
+
+  writer_->WritePacket(drainable_buffer,
                        base::Bind(&BlimpMessageSender::OnWritePacketComplete,
-                                  base::Unretained(this)));
+                                  weak_factory_.GetWeakPtr()));
 }
 
 void BlimpMessageSender::OnWritePacketComplete(int result) {
+  DVLOG(2) << "OnWritePacketComplete, result=" << result;
   DCHECK_NE(net::ERR_IO_PENDING, result);
   base::ResetAndReturn(&pending_process_msg_callback_).Run(result);
   if (result != net::OK) {
     error_observer_->OnConnectionError(result);
   }
 }
 
 }  // namespace
 
 BlimpConnection::BlimpConnection(scoped_ptr<PacketReader> reader,
                                  scoped_ptr<PacketWriter> writer)
     : reader_(std::move(reader)),
       message_pump_(new BlimpMessagePump(reader_.get())),
       writer_(std::move(writer)),
       outgoing_msg_processor_(new BlimpMessageSender(writer_.get())) {
   DCHECK(writer_);
 }
 
 BlimpConnection::BlimpConnection() {}
 
-BlimpConnection::~BlimpConnection() {}
+BlimpConnection::~BlimpConnection() {
+  DVLOG(1) << "BlimpConnection destroyed.";
+}
 
 void BlimpConnection::SetConnectionErrorObserver(
     ConnectionErrorObserver* observer) {
   message_pump_->set_error_observer(observer);
   BlimpMessageSender* sender =
       static_cast<BlimpMessageSender*>(outgoing_msg_processor_.get());
   sender->set_error_observer(observer);
 }
 
 void BlimpConnection::SetIncomingMessageProcessor(
     BlimpMessageProcessor* processor) {
   message_pump_->SetMessageProcessor(processor);
 }
 
 BlimpMessageProcessor* BlimpConnection::GetOutgoingMessageProcessor() {
   return outgoing_msg_processor_.get();
 }
 
 }  // namespace blimp