Base: Remove Receive() from ScopedHandle.

Base: Remove Receive() from ScopedHandle.

In general, the OS API contract doesn't guarantee that output variables are
not modified on failure, so a Reeceive pattern is fundamentally insecure.

BUG=318531
TEST=current tests

tbr'ing owners for the consumers.

TBR=jvoung@chromium.org, thakis@chromium.org, sergeyu@chromium.org, grt@chromium.org, gene@chromium.org, youngki@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237459

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237675

[rvargas (doing something else), 2013-11-15 22:17:51]

Please take a look.

[cpu_(ooo_6.6-7.5), 2013-11-20 02:17:15]

slogging thru gigantic CL but happy to see it.

https://codereview.chromium.org/71013004/diff/250001/base/process/launch_win.cc
File base/process/launch_win.cc (right):

https://codereview.chromium.org/71013004/diff/250001/base/process/launch_win....
base/process/launch_win.cc:171: process_info.Set(temp_process_info);
can we make it so we can write:
base::win::ScopedProcessInformation process_info(temp_process_info);

[rvargas (doing something else), 2013-11-20 20:30:45]

https://codereview.chromium.org/71013004/diff/250001/base/process/launch_win.cc
File base/process/launch_win.cc (right):

https://codereview.chromium.org/71013004/diff/250001/base/process/launch_win....
base/process/launch_win.cc:171: process_info.Set(temp_process_info);
On 2013/11/20 02:17:16, cpu wrote:
> can we make it so we can write:
> base::win::ScopedProcessInformation process_info(temp_process_info);

Done.

[cpu_(ooo_6.6-7.5), 2013-11-22 02:42:52]

otherwise lgtm

https://codereview.chromium.org/71013004/diff/740001/chrome/installer/setup/s...
File chrome/installer/setup/setup_util_unittest.cc (right):

https://codereview.chromium.org/71013004/diff/740001/chrome/installer/setup/s...
chrome/installer/setup/setup_util_unittest.cc:66: token.Set(temp_handle);
and for ScopedHandle too?

[rvargas (doing something else), 2013-11-25 19:53:26]

thanks

https://codereview.chromium.org/71013004/diff/740001/chrome/installer/setup/s...
File chrome/installer/setup/setup_util_unittest.cc (right):

https://codereview.chromium.org/71013004/diff/740001/chrome/installer/setup/s...
chrome/installer/setup/setup_util_unittest.cc:66: token.Set(temp_handle);
On 2013/11/22 02:42:53, cpu wrote:
> and for ScopedHandle too?
> 

Done.

[commit-bot: I haz the power, 2013-11-25 19:53:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/990001

[commit-bot: I haz the power, 2013-11-25 19:54:12]

Failed to apply patch for components/nacl/browser/nacl_process_host.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file components/nacl/browser/nacl_process_host.cc
  Hunk #1 FAILED at 272.
  Hunk #2 succeeded at 977 (offset -3 lines).
  Hunk #3 succeeded at 996 (offset -3 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
components/nacl/browser/nacl_process_host.cc.rej

Patch:       components/nacl/browser/nacl_process_host.cc
Index: components/nacl/browser/nacl_process_host.cc
diff --git a/components/nacl/browser/nacl_process_host.cc
b/components/nacl/browser/nacl_process_host.cc
index
9ffda7c23d4a2690273a364ba086833fda77355e..29e8ba441df37e56530266c9528426d531c5a066
100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -272,9 +272,9 @@ NaClProcessHost::~NaClProcessHost() {
         base::StringPrintf("NaCl process exited with status %i (0x%x)",
                            exit_code, exit_code);
     if (exit_code == 0) {
-      LOG(INFO) << message;
+      VLOG(0) << message;
     } else {
-      LOG(ERROR) << message;
+      VLOG(0) << message;
     }
   }
 
@@ -980,7 +980,7 @@ bool NaClProcessHost::AttachDebugExceptionHandler(const
std::string& info,
   debug_exception_handler_requested_ = true;
 
   base::ProcessId nacl_pid = base::GetProcId(process_->GetData().handle);
-  base::win::ScopedHandle process_handle;
+  base::ProcessHandle temp_handle;
   // We cannot use process_->GetData().handle because it does not have
   // the necessary access rights.  We open the new handle here rather
   // than in the NaCl broker process in case the NaCl loader process
@@ -999,10 +999,11 @@ bool NaClProcessHost::AttachDebugExceptionHandler(const
std::string& info,
            base::kProcessAccessVMWrite |
            base::kProcessAccessDuplicateHandle |
            base::kProcessAccessWaitForTermination,
-           process_handle.Receive())) {
+           &temp_handle)) {
     LOG(ERROR) << "Failed to get process handle";
     return false;
   }
+  base::win::ScopedHandle process_handle(temp_handle);
 
   attach_debug_exception_handler_reply_msg_.reset(reply_msg);
   // If the NaCl loader is 64-bit, the process running its debug

[commit-bot: I haz the power, 2013-11-26 02:09:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1080001

[commit-bot: I haz the power, 2013-11-26 02:29:30]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-11-26 02:41:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1080001

[commit-bot: I haz the power, 2013-11-26 05:01:19]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-11-26 22:15:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1100001

[commit-bot: I haz the power, 2013-11-27 01:03:48]

Change committed as 237459

[hashimoto, 2013-11-27 08:21:47]

A revert of this CL has been created in
https://codereview.chromium.org/90963002/ by hashimoto@chromium.org.

The reason for reverting is: Causing compile failure in chrome_util.cc on
"Google Chrome Win"
http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/bu....

[commit-bot: I haz the power, 2013-11-27 21:26:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 01:18:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 02:00:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 02:21:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 02:27:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 02:59:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 03:29:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/71013004/1120001

[commit-bot: I haz the power, 2013-11-28 05:17:06]

Change committed as 237675