Use ConnectionErrorObserver, not callbacks, for error handling.

blimp/net/blimp_message_output_buffer.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_message_output_buffer.h"
 
 #include <algorithm>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
@@ skipping 126 matching lines @@
   DVLOG(3) << "Writing message (id="
            << write_buffer_.front()->message->message_id()
            << ", type=" << message_to_write->type() << ")";
 
   output_processor_->ProcessMessage(std::move(message_to_write),
                                     write_complete_cb_.callback());
   DVLOG(3) << "Queue size: " << write_buffer_.size();
 }
 
 void BlimpMessageOutputBuffer::OnWriteComplete(int result) {
-  DCHECK_LE(result, net::OK);
-  VLOG(2) << "Write complete, result=" << result;
-
-  if (result == net::OK) {
-    ack_buffer_.push_back(std::move(write_buffer_.front()));
-    write_buffer_.pop_front();
-    WriteNextMessageIfReady();
-  } else {
-    // An error occurred while writing to the network connection.
-    // Stop writing more messages until a new connection is established.
-    DLOG(WARNING) << "Write error (result=" << result << ")";
-  }
+  DCHECK_EQ(result, net::OK);

[Wez, 2016/04/21 22:44:29]

It seems that you're changing the MessageProcessor contract such that processing
a message can never fail, so why do we still have |result| in the callback
signature?

nit: DCHECK_EQ(expected, actual) is equivalent but preferred, since it matches
the gmock EXPECT_EQ() required ordering.

nit: Functions like this are easier to read if you use blank lines to e.g.
separate the pre-condition checks (this DCHECK) from the body, and generally to
break up the body into logically blocks.

[Kevin M, 2016/04/22 22:10:41]

Done.

+  VLOG(2) << "Write complete.";

[Wez, 2016/04/21 22:44:29]

nit: None of our other log messages, above, seem to have terminating periods.

[Kevin M, 2016/04/22 22:10:41]

Done.

+  ack_buffer_.push_back(std::move(write_buffer_.front()));
+  write_buffer_.pop_front();
+  WriteNextMessageIfReady();
 }
 
 }  // namespace blimp