Fix shutdown assert with the new Mojo EDK.

This was seen on Windows tryjobs enabling the new EDK. The problem was that RoutedRawChannel was destroying the channel on write errors, while normally (without multiplexing) this is only done on read errors. So MessagePipeDispatcher thought it had a channel_ (which it didn't use because it knew it had seen a write error) and it was asserting at shutdown. The fix is to make RoutedRawChannel match non-multiplexed MessagePipeDispatcher's destruction of the channel only when read errors occur.

BUG=561803

Review URL: https://codereview.chromium.org/1526563004

Cr-Commit-Position: refs/heads/master@{#365175}

2 files changed, 9 insertions, 4 deletions

diff --git a/mojo/edk/system/message_pipe_dispatcher.cc b/mojo/edk/system/message_pipe_dispatcher.cc
index b4938fe..beb1824 100644
--- a/mojo/edk/system/message_pipe_dispatcher.cc
+++ b/mojo/edk/system/message_pipe_dispatcher.cc
@@ -618,7 +618,7 @@ MojoResult MessagePipeDispatcher::ReadMessageImplNoLock(
     uint32_t* num_dispatchers,
     MojoReadMessageFlags flags) {
   lock().AssertAcquired();
-  if (channel_) {
+  if (transferable_ && channel_) {
     channel_->EnsureLazyInitialized();
   } else if (!transferable_) {
     if (non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) {
@@ -718,7 +718,7 @@ MojoResult MessagePipeDispatcher::AddAwakableImplNoLock(
     uintptr_t context,
     HandleSignalsState* signals_state) {
   lock().AssertAcquired();
-  if (channel_) {
+  if (transferable_ && channel_) {
     channel_->EnsureLazyInitialized();
   } else if (!transferable_ &&
              non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) {
diff --git a/mojo/edk/system/routed_raw_channel.cc b/mojo/edk/system/routed_raw_channel.cc
index 825fc2fa..b985f56 100644
--- a/mojo/edk/system/routed_raw_channel.cc
+++ b/mojo/edk/system/routed_raw_channel.cc
@@ -132,8 +132,13 @@ void RoutedRawChannel::OnReadMessage(
 void RoutedRawChannel::OnError(Error error) {
   DCHECK(internal::g_io_thread_task_runner->RunsTasksOnCurrentThread());
 
-  channel_->Shutdown();
-  channel_ = nullptr;
+  // This needs to match non-multiplexed MessagePipeDispatcher's destruction of
+  // the channel only when read errors occur.
+  if (error != ERROR_WRITE) {
+    channel_->Shutdown();
+    channel_ = nullptr;
+  }
+
   if (routes_.empty()) {
     delete this;
     return;