[Fuchsia] Fix leak of kernel job object per-Context.

ContextProviderImpl incorrectly assumed that LaunchProcess() would
take ownership of the zx::job specified in the LaunchOptions. Fix the
provider not let the zx::job handle be closed after launch.

To support testing of this fix, add a ScopedDefaultJobForTest helper to
//base, to allow the handle returned by base::GetDefaultJob() (which
ContextProviderImpl creates a per-Context job under) to be overridden
for a given scope.

Bug: 927403
Change-Id: I83cc6119657d82fc8d4c7a62081a1c04938a70b5
Reviewed-on: https://chromium-review.googlesource.com/c/1450614
Auto-Submit: Wez <wez@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628499}

base/fuchsia/default_job.cc

@@ -25,4 +25,16 @@ void SetDefaultJob(zx::job job) {
   g_job = job.release();
 }
 
+ScopedDefaultJobForTest::ScopedDefaultJobForTest(zx::job new_default_job) {
+  DCHECK(new_default_job.is_valid());
+  old_default_job_.reset(g_job);
+  g_job = new_default_job.release();
+}
+
+ScopedDefaultJobForTest::~ScopedDefaultJobForTest() {
+  DCHECK_NE(g_job, ZX_HANDLE_INVALID);
+  zx::job my_default_job(g_job);
+  g_job = old_default_job_.release();
+}
+
 }  // namespace base

base/fuchsia/default_job.h

@@ -8,6 +8,7 @@
 #include <lib/zx/job.h>
 
 #include "base/base_export.h"
+#include "base/macros.h"
 
 namespace base {
 
@@ -18,6 +19,19 @@ namespace base {
 BASE_EXPORT zx::unowned_job GetDefaultJob();
 BASE_EXPORT void SetDefaultJob(zx::job job);
 
+// Replaces the current default job (if any) with the specified zx::job, and
+// restores the original default job when going out-of-scope.
+// Note that replacing the default job is not thread-safe!
+class BASE_EXPORT ScopedDefaultJobForTest {
+ public:
+  ScopedDefaultJobForTest(zx::job new_default_job);
+  ~ScopedDefaultJobForTest();
+
+ private:
+  zx::job old_default_job_;
+  DISALLOW_COPY_AND_ASSIGN(ScopedDefaultJobForTest);
+};
+
 }  // namespace base
 
 #endif  // BASE_FUCHSIA_DEFAULT_JOB_H_

fuchsia/service/context_provider_impl.cc

@@ -131,7 +131,6 @@ void ContextProviderImpl::Create(
   ignore_result(launch_.Run(std::move(launch_command), launch_options));
 
   ignore_result(context_handle.release());
-  ignore_result(job.release());
 }
 
 void ContextProviderImpl::Bind(

fuchsia/service/context_provider_impl_unittest.cc

@@ -7,6 +7,7 @@
 #include <lib/fdio/util.h>
 #include <lib/fidl/cpp/binding.h>
 #include <zircon/processargs.h>
+#include <zircon/types.h>
 
 #include <functional>
 #include <string>
@@ -22,12 +23,14 @@
 #include "base/files/file.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/fuchsia/default_job.h"
 #include "base/fuchsia/file_utils.h"
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/fuchsia/service_directory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/test/multiprocess_test.h"
+#include "base/test/test_timeouts.h"
 #include "fuchsia/fidl/chromium/web/cpp/fidl_test_base.h"
 #include "fuchsia/service/common.h"
 #include "fuchsia/test/fake_context.h"
@@ -268,4 +271,48 @@ TEST_F(ContextProviderImplTest, FailsDataDirectoryIsFile) {
   CheckContextUnresponsive(&context);
 }
 
+static bool WaitUntilJobIsEmpty(zx::unowned_job job, zx::duration timeout) {
+  zx_signals_t observed = 0;
+  zx_status_t status =
+      job->wait_one(ZX_JOB_NO_JOBS, zx::deadline_after(timeout), &observed);
+  ZX_CHECK(status == ZX_OK || status == ZX_ERR_TIMED_OUT, status);
+  return observed & ZX_JOB_NO_JOBS;
+}
+
+// Regression test for https://crbug.com/927403 (Job leak per-Context).
+TEST_F(ContextProviderImplTest, CleansUpContextJobs) {
+  // Replace the default job with one that is guaranteed to be empty.
+  zx::job job;
+  ASSERT_EQ(base::GetDefaultJob()->duplicate(ZX_RIGHT_SAME_RIGHTS, &job),
+            ZX_OK);
+  base::ScopedDefaultJobForTest empty_default_job(std::move(job));
+
+  // Bind to the ContextProvider.
+  chromium::web::ContextProviderPtr provider;
+  provider_->Bind(provider.NewRequest());
+
+  // Verify that our current default job is still empty.
+  ASSERT_TRUE(WaitUntilJobIsEmpty(base::GetDefaultJob(), zx::duration()));
+
+  // Create a Context and verify that it is functional.
+  chromium::web::ContextPtr context;
+  provider->Create(BuildCreateContextParams(), context.NewRequest());
+  CheckContextResponsive(&context);
+
+  // Verify that there is at least one job under our default job.
+  ASSERT_FALSE(WaitUntilJobIsEmpty(base::GetDefaultJob(), zx::duration()));
+
+  // Detach from the Context and ContextProvider, and spin the loop to allow
+  // those to be handled.
+  context.Unbind();
+  provider.Unbind();
+  base::RunLoop().RunUntilIdle();
+
+  // Wait until the default job signals that it no longer contains any child
+  // jobs; this should occur shortly after the Context process terminates.
+  EXPECT_TRUE(WaitUntilJobIsEmpty(
+      base::GetDefaultJob(),
+      zx::duration(TestTimeouts::action_timeout().InNanoseconds())));
+}
+
 }  // namespace webrunner