net: FileStream cleanup

chrome/browser/extensions/api/messaging/native_message_process_host.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/api/messaging/native_message_process_host.h"
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
-#include "base/platform_file.h"
 #include "base/prefs/pref_service.h"
 #include "base/process/kill.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/messaging/native_messaging_host_manifest.h"
 #include "chrome/browser/extensions/api/messaging/native_process_launcher.h"
 #include "chrome/common/chrome_version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/constants.h"
@@ skipping 77 matching lines @@
     int destination_port,
     scoped_ptr<NativeProcessLauncher> launcher)
     : weak_client_ui_(weak_client_ui),
       source_extension_id_(source_extension_id),
       native_host_name_(native_host_name),
       destination_port_(destination_port),
       launcher_(launcher.Pass()),
       closed_(false),
       process_handle_(base::kNullProcessHandle),
 #if defined(OS_POSIX)
-      read_file_(base::kInvalidPlatformFileValue),
+      read_file_(-1),

[wtc, 2014/06/19 18:20:49]

Nit: the original code (base::kInvalidPlatformFileValue) seems fine to me.

[rvargas (doing something else), 2014/06/19 18:46:42]

Unfortunately, my next patch will completely remove platform_file.h, so
base::kInvalidPlatformFileValue will be undefined. After I remove that file,
base::PlatformFile is only defined by file.h, but not to keep variables of that
type (which mean initializations, checks for validity etc) but to use the
underlying handle of a base::File on a lower level call.

That's the detail about this code that I was trying to fix: for POSIX, it stores
a file descriptor that is owned by another object. What makes it acceptable is
that the owner of that file is a FileStream that this class also owns.

 #endif
       read_pending_(false),
       write_pending_(false) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // It's safe to use base::Unretained() here because NativeMessagePort always
   // deletes us on the IO thread.
   content::BrowserThread::PostTask(content::BrowserThread::IO, FROM_HERE,
       base::Bind(&NativeMessageProcessHost::LaunchHostProcess,
                  base::Unretained(this)));
@@ skipping 287 matching lines @@
     content::BrowserThread::PostBlockingPoolTask(
         FROM_HERE, base::Bind(&base::EnsureProcessTerminated, process_handle_));
 #else
     base::EnsureProcessTerminated(process_handle_);
 #endif
     process_handle_ = base::kNullProcessHandle;
   }
 }
 
 }  // namespace extensions