Revert of Fix inherited handles conflicting with handle tracker (patchset #5 id:40002 of https://codereview.chromium.org/1318803005/ )

Reason for revert:
Broke
https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/39844

Original issue's description:
> Fix inherited handles conflicting with handle tracker
>
> The inherited handles are stored in a ScopedHandle, so the handle tracker
> marks them as non-closeable for the lifetime of that ScopedHandle in the
> browser process. That non-closeable handle is inherited to the child, but
> should be closeable there the child's is a new, valid handle.
>
> We can't mark only the child copy as closeable, so for the duration of
> target creation, mark the handle as closeable, and after process creation,
> restore the handle protection state.
>
> R=wfh@chromium.org
> BUG=524267
> TEST=HandleInheritanceTests.InheritByValue
>
> Committed: https://crrev.com/bfca1f2f2e75e3d36a171ce9987e7a72bedfbd85
> Cr-Commit-Position: refs/heads/master@{#346835}

TBR=wfh@chromium.org,brucedawson@chromium.org,scottmg@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=524267

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

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

2 files changed, 0 insertions, 60 deletions

diff --git a/sandbox/win/src/broker_services.cc b/sandbox/win/src/broker_services.cc
index 1a4c2f3..1c72ef5 100644
--- a/sandbox/win/src/broker_services.cc
+++ b/sandbox/win/src/broker_services.cc
@@ -498,23 +498,6 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
   if (NULL == thread_pool_)
     thread_pool_ = new Win2kThreadPool();
 
-  // We need to temporarily mark all inherited handles as closeable. The handle
-  // tracker may have marked the handles we're passing to the child as
-  // non-closeable, but the child is getting new copies that it's allowed to
-  // close. We're about to mark these handles as closeable for this process
-  // (when we close them below in ClearSharedHandles()) but that will be too
-  // late -- there will already another copy in the child that's non-closeable.
-  // After launching we restore the non-closability of these handles. We don't
-  // have any way here to affect *only* the child's copy, as the process
-  // launching mechanism takes care of doing the duplication-with-the-same-value
-  // into the child.
-  std::vector<DWORD> inherited_handle_information(inherited_handle_list.size());
-  for (size_t i = 0; i < inherited_handle_list.size(); ++i) {
-    const HANDLE& inherited_handle = inherited_handle_list[i];
-    ::GetHandleInformation(inherited_handle, &inherited_handle_information[i]);
-    ::SetHandleInformation(inherited_handle, HANDLE_FLAG_PROTECT_FROM_CLOSE, 0);
-  }
-
   // Create the TargetProces object and spawn the target suspended. Note that
   // Brokerservices does not own the target object. It is owned by the Policy.
   base::win::ScopedProcessInformation process_info;
@@ -527,13 +510,6 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
                                     policy_base->GetLowBoxSid() ? true : false,
                                     startup_info, &process_info);
 
-  // Restore the previous handle protection values.
-  for (size_t i = 0; i < inherited_handle_list.size(); ++i) {
-    ::SetHandleInformation(inherited_handle_list[i],
-                           HANDLE_FLAG_PROTECT_FROM_CLOSE,
-                           inherited_handle_information[i]);
-  }
-
   policy_base->ClearSharedHandles();
 
   if (ERROR_SUCCESS != win_result) {
diff --git a/sandbox/win/src/handle_inheritance_test.cc b/sandbox/win/src/handle_inheritance_test.cc
index 2bb1612..37974e3 100644
--- a/sandbox/win/src/handle_inheritance_test.cc
+++ b/sandbox/win/src/handle_inheritance_test.cc
@@ -6,9 +6,6 @@
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/shared_memory.h"
-#include "base/strings/string_number_conversions.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/windows_version.h"
 #include "sandbox/win/tests/common/controller.h"
@@ -52,37 +49,4 @@ TEST(HandleInheritanceTests, TestStdoutInheritance) {
   }
 }
 
-SBOX_TESTS_COMMAND int
-HandleInheritanceTests_ValidInheritedHandle(int argc, wchar_t **argv) {
-  if (argc != 1)
-    return SBOX_TEST_FAILED_TO_RUN_TEST;
-  base::SharedMemoryHandle handle = nullptr;
-  base::StringToUint(argv[0], reinterpret_cast<unsigned int *>(&handle));
-
-  // This handle we inherited must be both valid and closeable.
-  DWORD flags = 0;
-  if (!GetHandleInformation(handle, &flags))
-    return SBOX_TEST_FAILED;
-  if ((flags & HANDLE_FLAG_PROTECT_FROM_CLOSE) != 0)
-    return SBOX_TEST_FAILED;
-
-  return SBOX_TEST_SUCCEEDED;
-}
-
-TEST(HandleInheritanceTests, InheritByValue) {
-  base::SharedMemory test_shared_memory;
-  ASSERT_TRUE(test_shared_memory.CreateAnonymous(1000));
-  ASSERT_TRUE(test_shared_memory.Map(0));
-
-  TestRunner runner;
-  void *shared_handle =
-      runner.GetPolicy()->AddHandleToShare(test_shared_memory.handle());
-
-  std::string command_line =
-      "HandleInheritanceTests_ValidInheritedHandle " +
-      base::UintToString(reinterpret_cast<unsigned int>(shared_handle));
-  int result = runner.RunTest(base::UTF8ToUTF16(command_line).c_str());
-  ASSERT_EQ(SBOX_TEST_SUCCEEDED, result);
 }
-
-}  // namespace sandbox