IPC: Use ScopedHandle instead of a raw HANDLE for the private members.

BUG=387876
R=cpu@chromium.org

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

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

ipc/ipc_channel_win.cc

@@ -59,7 +59,6 @@ ChannelWin::ChannelWin(const IPC::ChannelHandle &channel_handle,
     : ChannelReader(listener),
       input_state_(this),
       output_state_(this),
-      pipe_(INVALID_HANDLE_VALUE),
       peer_pid_(base::kNullProcessId),
       waiting_connect_(mode & MODE_SERVER_FLAG),
       processing_incoming_(false),
@@ -85,14 +84,12 @@ void ChannelWin::Close() {
   debug_flags_ |= CLOSED;
 
   if (input_state_.is_pending || output_state_.is_pending)
-    CancelIo(pipe_);
+    CancelIo(pipe_.Get());
 
   // Closing the handle at this point prevents us from issuing more requests
   // form OnIOCompleted().
-  if (pipe_ != INVALID_HANDLE_VALUE) {
-    CloseHandle(pipe_);
-    pipe_ = INVALID_HANDLE_VALUE;
-  }
+  if (pipe_.IsValid())
+    pipe_.Close();
 
   if (input_state_.is_pending)
     debug_flags_ |= WAIT_FOR_READ;
@@ -158,12 +155,12 @@ ChannelWin::ReadState ChannelWin::ReadData(
     char* buffer,
     int buffer_len,
     int* /* bytes_read */) {
-  if (INVALID_HANDLE_VALUE == pipe_)
+  if (!pipe_.IsValid())
     return READ_FAILED;
 
   debug_flags_ |= READ_MSG;
   DWORD bytes_read = 0;
-  BOOL ok = ReadFile(pipe_, buffer, buffer_len,
+  BOOL ok = ReadFile(pipe_.Get(), buffer, buffer_len,
                      &bytes_read, &input_state_.context.overlapped);
   if (!ok) {
     DWORD err = GetLastError();
@@ -244,11 +241,15 @@ const base::string16 ChannelWin::PipeName(
 }
 
 bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle,
-                                      Mode mode) {
-  DCHECK_EQ(INVALID_HANDLE_VALUE, pipe_);
+                            Mode mode) {
+  DCHECK(!pipe_.IsValid());
   base::string16 pipe_name;
   // If we already have a valid pipe for channel just copy it.
   if (channel_handle.pipe.handle) {
+    // TODO(rvargas) crbug.com/415294: ChannelHandle should either go away in
+    // favor of two independent entities (name/file), or it should be a move-
+    // only type with a base::File member. In any case, this code should not
+    // call DuplicateHandle.
     DCHECK(channel_handle.name.empty());
     pipe_name = L"Not Available";  // Just used for LOG
     // Check that the given pipe confirms to the specified mode.  We can
@@ -262,46 +263,48 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle,
       LOG(WARNING) << "Inconsistent open mode. Mode :" << mode;
       return false;
     }
+    HANDLE local_handle;
     if (!DuplicateHandle(GetCurrentProcess(),
                          channel_handle.pipe.handle,
                          GetCurrentProcess(),
-                         &pipe_,
+                         &local_handle,
                          0,
                          FALSE,
                          DUPLICATE_SAME_ACCESS)) {
       LOG(WARNING) << "DuplicateHandle failed. Error :" << GetLastError();
       return false;
     }
+    pipe_.Set(local_handle);
   } else if (mode & MODE_SERVER_FLAG) {
     DCHECK(!channel_handle.pipe.handle);
     const DWORD open_mode = PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |
                             FILE_FLAG_FIRST_PIPE_INSTANCE;
     pipe_name = PipeName(channel_handle.name, &client_secret_);
     validate_client_ = !!client_secret_;
-    pipe_ = CreateNamedPipeW(pipe_name.c_str(),
-                             open_mode,
-                             PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
-                             1,
-                             Channel::kReadBufferSize,
-                             Channel::kReadBufferSize,
-                             5000,
-                             NULL);
+    pipe_.Set(CreateNamedPipeW(pipe_name.c_str(),
+                               open_mode,
+                               PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
+                               1,
+                               Channel::kReadBufferSize,
+                               Channel::kReadBufferSize,
+                               5000,
+                               NULL));
   } else if (mode & MODE_CLIENT_FLAG) {
     DCHECK(!channel_handle.pipe.handle);
     pipe_name = PipeName(channel_handle.name, &client_secret_);
-    pipe_ = CreateFileW(pipe_name.c_str(),
-                        GENERIC_READ | GENERIC_WRITE,
-                        0,
-                        NULL,
-                        OPEN_EXISTING,
-                        SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION |
-                            FILE_FLAG_OVERLAPPED,
-                        NULL);
+    pipe_.Set(CreateFileW(pipe_name.c_str(),
+                          GENERIC_READ | GENERIC_WRITE,
+                          0,
+                          NULL,
+                          OPEN_EXISTING,
+                          SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION |
+                              FILE_FLAG_OVERLAPPED,
+                          NULL));
   } else {
     NOTREACHED();
   }
 
-  if (pipe_ == INVALID_HANDLE_VALUE) {
+  if (!pipe_.IsValid()) {
     // If this process is being closed, the pipe may be gone already.
     PLOG(WARNING) << "Unable to create pipe \"" << pipe_name << "\" in "
                   << (mode & MODE_SERVER_FLAG ? "server" : "client") << " mode";
@@ -318,8 +321,7 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle,
   int32 secret = validate_client_ ? 0 : client_secret_;
   if (!m->WriteInt(GetCurrentProcessId()) ||
       (secret && !m->WriteUInt32(secret))) {
-    CloseHandle(pipe_);
-    pipe_ = INVALID_HANDLE_VALUE;
+    pipe_.Close();
     return false;
   }
 
@@ -335,10 +337,10 @@ bool ChannelWin::Connect() {
   if (!thread_check_.get())
     thread_check_.reset(new base::ThreadChecker());
 
-  if (pipe_ == INVALID_HANDLE_VALUE)
+  if (!pipe_.IsValid())
     return false;
 
-  base::MessageLoopForIO::current()->RegisterIOHandler(pipe_, this);
+  base::MessageLoopForIO::current()->RegisterIOHandler(pipe_.Get(), this);
 
   // Check to see if there is a client connected to our pipe...
   if (waiting_connect_)
@@ -368,10 +370,10 @@ bool ChannelWin::ProcessConnection() {
     input_state_.is_pending = false;
 
   // Do we have a client connected to our pipe?
-  if (INVALID_HANDLE_VALUE == pipe_)
+  if (!pipe_.IsValid())
     return false;
 
-  BOOL ok = ConnectNamedPipe(pipe_, &input_state_.context.overlapped);
+  BOOL ok = ConnectNamedPipe(pipe_.Get(), &input_state_.context.overlapped);
   debug_flags_ |= CALLED_CONNECT;
 
   DWORD err = GetLastError();
@@ -427,7 +429,7 @@ bool ChannelWin::ProcessOutgoingMessages(
   if (output_queue_.empty())
     return true;
 
-  if (INVALID_HANDLE_VALUE == pipe_)
+  if (!pipe_.IsValid())
     return false;
 
   // Write to pipe...
@@ -438,7 +440,7 @@ bool ChannelWin::ProcessOutgoingMessages(
   writing_ = true;
   write_size_ = static_cast<uint32>(m->size());
   write_error_ = 0;
-  BOOL ok = WriteFile(pipe_,
+  BOOL ok = WriteFile(pipe_.Get(),
                       m->data(),
                       write_size_,
                       NULL,
@@ -501,7 +503,7 @@ void ChannelWin::OnIOCompleted(
       input_state_.is_pending = false;
       if (!bytes_transfered)
         ok = false;
-      else if (pipe_ != INVALID_HANDLE_VALUE)
+      else if (pipe_.IsValid())
         ok = AsyncReadComplete(bytes_transfered);
     } else {
       DCHECK(!bytes_transfered);
@@ -522,7 +524,7 @@ void ChannelWin::OnIOCompleted(
     }
     ok = ProcessOutgoingMessages(context, bytes_transfered);
   }
-  if (!ok && INVALID_HANDLE_VALUE != pipe_) {
+  if (!ok && pipe_.IsValid()) {
     // We don't want to re-enter Close().
     Close();
     listener()->OnChannelError();

ipc/ipc_channel_win.h

@@ -13,6 +13,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/win/scoped_handle.h"
 #include "ipc/ipc_channel_reader.h"
 
 namespace base {
@@ -73,7 +74,7 @@ class ChannelWin : public Channel,
   State input_state_;
   State output_state_;
 
-  HANDLE pipe_;
+  base::win::ScopedHandle pipe_;
 
   base::ProcessId peer_pid_;