Enforce handle ownership in base::Process

chrome/browser/renderer_host/render_process_host_chrome_browsertest.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 "base/command_line.h"
+#include "base/process/process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ skipping 35 matching lines @@
       continue;
     RenderViewHost* host = RenderViewHost::From(widget);
     WebContents* contents = WebContents::FromRenderViewHost(host);
     GURL url = contents->GetURL();
     if (url.SchemeIs(content::kChromeDevToolsScheme))
       return contents;
   }
   return NULL;
 }
 
+// TODO(rvargas) crbug.com/417532: Remove this code.
+base::Process ProcessFromHandle(base::ProcessHandle handle) {
+#if defined(OS_WIN)
+    if (handle == GetCurrentProcess())

[Lei Zhang, 2014/10/16 02:50:08]

nit: funny indentation

[rvargas (doing something else), 2014/10/16 19:06:10]

Done.

+      return base::Process::Current();
+
+    base::ProcessHandle out_handle;
+    if (!::DuplicateHandle(GetCurrentProcess(), handle, GetCurrentProcess(),
+                           &out_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+      return base::Process();
+    }
+    handle = out_handle;
+#endif  // defined(OS_WIN)
+    return base::Process(handle);
+}
+
 }  // namespace
 
 class ChromeRenderProcessHostTest : public InProcessBrowserTest {
  public:
   ChromeRenderProcessHostTest() {}
 
   // Show a tab, activating the current one if there is one, and wait for
   // the renderer process to be created or foregrounded, returning the process
   // handle.
-  base::ProcessHandle ShowSingletonTab(const GURL& page) {
+  base::Process ShowSingletonTab(const GURL& page) {
     chrome::ShowSingletonTab(browser(), page);
     WebContents* wc = browser()->tab_strip_model()->GetActiveWebContents();
     CHECK(wc->GetURL() == page);
 
     WaitForLauncherThread();
     WaitForMessageProcessing(wc);
-    return wc->GetRenderProcessHost()->GetHandle();
+    return ProcessFromHandle(wc->GetRenderProcessHost()->GetHandle());
   }
 
   // Loads the given url in a new background tab and returns the handle of its
   // renderer.
-  base::ProcessHandle OpenBackgroundTab(const GURL& page) {
+  base::Process OpenBackgroundTab(const GURL& page) {
     ui_test_utils::NavigateToURLWithDisposition(browser(), page,
         NEW_BACKGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
 
     TabStripModel* tab_strip = browser()->tab_strip_model();
     WebContents* wc = tab_strip->GetWebContentsAt(
         tab_strip->active_index() + 1);
     CHECK(wc->GetVisibleURL() == page);
 
     WaitForLauncherThread();
     WaitForMessageProcessing(wc);
-    return wc->GetRenderProcessHost()->GetHandle();
+    return ProcessFromHandle(wc->GetRenderProcessHost()->GetHandle());
   }
 
   // Ensures that the backgrounding / foregrounding gets a chance to run.
   void WaitForLauncherThread() {
     content::BrowserThread::PostTaskAndReply(
         content::BrowserThread::PROCESS_LAUNCHER, FROM_HERE,
         base::Bind(&base::DoNothing), base::MessageLoop::QuitClosure());
     base::MessageLoop::current()->Run();
   }
 
@@ skipping 175 matching lines @@
   }
   CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
   parsed_command_line.AppendSwitch(switches::kProcessPerTab);
 
   // Change the first tab to be the omnibox page (TYPE_WEBUI).
   GURL omnibox(chrome::kChromeUIOmniboxURL);
   ui_test_utils::NavigateToURL(browser(), omnibox);
 
   // Create a new tab. It should be foreground.
   GURL page1("data:text/html,hello world1");
-  base::ProcessHandle pid1 = ShowSingletonTab(page1);
-  EXPECT_FALSE(base::Process(pid1).IsProcessBackgrounded());
+  base::Process process1 = ShowSingletonTab(page1);
+  ASSERT_TRUE(process1.IsValid());
+  EXPECT_FALSE(process1.IsProcessBackgrounded());
 
   // Create another tab. It should be foreground, and the first tab should
   // now be background.
   GURL page2("data:text/html,hello world2");
-  base::ProcessHandle pid2 = ShowSingletonTab(page2);
-  EXPECT_NE(pid1, pid2);
-  EXPECT_TRUE(base::Process(pid1).IsProcessBackgrounded());
-  EXPECT_FALSE(base::Process(pid2).IsProcessBackgrounded());
+  base::Process process2 = ShowSingletonTab(page2);
+  ASSERT_TRUE(process2.IsValid());
+  EXPECT_NE(process1.pid(), process2.pid());
+  EXPECT_TRUE(process1.IsProcessBackgrounded());
+  EXPECT_FALSE(process2.IsProcessBackgrounded());
 
   // Load another tab in background. The renderer of the new tab should be
   // backgrounded, while visibility of the other renderers should not change.
   GURL page3("data:text/html,hello world3");
-  base::ProcessHandle pid3 = OpenBackgroundTab(page3);
-  EXPECT_NE(pid3, pid1);
-  EXPECT_NE(pid3, pid2);
-  EXPECT_TRUE(base::Process(pid1).IsProcessBackgrounded());
-  EXPECT_FALSE(base::Process(pid2).IsProcessBackgrounded());
-  EXPECT_TRUE(base::Process(pid3).IsProcessBackgrounded());
+  base::Process process3 = OpenBackgroundTab(page3);
+  ASSERT_TRUE(process3.IsValid());
+  EXPECT_NE(process3.pid(), process1.pid());
+  EXPECT_NE(process3.pid(), process2.pid());
+  EXPECT_TRUE(process1.IsProcessBackgrounded());
+  EXPECT_FALSE(process2.IsProcessBackgrounded());
+  EXPECT_TRUE(process3.IsProcessBackgrounded());
 
   // Navigate back to the first page. Its renderer should be in foreground
   // again while the other renderers should be backgrounded.
-  EXPECT_EQ(pid1, ShowSingletonTab(page1));
-  EXPECT_FALSE(base::Process(pid1).IsProcessBackgrounded());
-  EXPECT_TRUE(base::Process(pid2).IsProcessBackgrounded());
-  EXPECT_TRUE(base::Process(pid3).IsProcessBackgrounded());
+  EXPECT_EQ(process1.pid(), ShowSingletonTab(page1).pid());
+  EXPECT_FALSE(process1.IsProcessBackgrounded());
+  EXPECT_TRUE(process2.IsProcessBackgrounded());
+  EXPECT_TRUE(process3.IsProcessBackgrounded());
 }
 #endif
 
 // TODO(nasko): crbug.com/173137
 #if defined(OS_WIN)
 #define MAYBE_ProcessOverflow DISABLED_ProcessOverflow
 #else
 #define MAYBE_ProcessOverflow ProcessOverflow
 #endif
 
@@ skipping 161 matching lines @@
       chrome::NOTIFICATION_BROWSER_CLOSED,
       content::NotificationService::AllSources());
 
   // Kill the renderer process, simulating a crash. This should the ProcessDied
   // method to be called. Alternatively, RenderProcessHost::OnChannelError can
   // be called to directly force a call to ProcessDied.
   base::KillProcess(wc1->GetRenderProcessHost()->GetHandle(), -1, true);
 
   observer.Wait();
 }