From a8729ccdbc01416165d36cc27856f380dab91aa0 Mon Sep 17 00:00:00 2001 From: "aa@chromium.org" Date: Mon, 4 Nov 2013 19:24:21 +0000 Subject: Say "noto" goto. Also, fix DCHECK due to incorrect cleanup when app_library fails to load. R=ben@chromium.org, darin@chromium.org Review URL: https://codereview.chromium.org/56553002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@232756 0039d316-1c4b-4281-b951-d872f2087c98 --- mojo/mojo.gyp | 12 ++++++++++++ mojo/public/utility/scoped_handle.cc | 25 +++++++++++++++++++++++++ mojo/public/utility/scoped_handle.h | 27 +++++++++++++++++++++++++++ mojo/shell/app_container.cc | 35 +++++++++++++++++------------------ 4 files changed, 81 insertions(+), 18 deletions(-) create mode 100644 mojo/public/utility/scoped_handle.cc create mode 100644 mojo/public/utility/scoped_handle.h (limited to 'mojo') diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp index 3cf650f..afda2d0 100644 --- a/mojo/mojo.gyp +++ b/mojo/mojo.gyp @@ -135,6 +135,7 @@ '../net/net.gyp:net', '../url/url.gyp:url_lib', 'mojo_system', + 'mojo_utility', 'native_viewport', ], 'sources': [ @@ -215,6 +216,17 @@ ], }, { + 'target_name': 'mojo_utility', + 'type': 'static_library', + 'dependencies': [ + 'mojo_system' + ], + 'sources': [ + 'public/utility/scoped_handle.cc', + 'public/utility/scoped_handle.h', + ], + }, + { 'target_name': 'sample_app', 'type': 'shared_library', 'dependencies': [ diff --git a/mojo/public/utility/scoped_handle.cc b/mojo/public/utility/scoped_handle.cc new file mode 100644 index 0000000..cba4b8a --- /dev/null +++ b/mojo/public/utility/scoped_handle.cc @@ -0,0 +1,25 @@ +// Copyright 2013 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 "mojo/public/utility/scoped_handle.h" + +namespace mojo { + +ScopedHandle::ScopedHandle(Handle handle) : handle_(handle) { +} + +ScopedHandle::~ScopedHandle() { + if (handle_.value == kInvalidHandle.value) + return; + Close(handle_); + Release(); +} + +Handle ScopedHandle::Release() { + Handle temp = handle_; + handle_ = kInvalidHandle; + return temp; +} + +} // namespace mojo diff --git a/mojo/public/utility/scoped_handle.h b/mojo/public/utility/scoped_handle.h new file mode 100644 index 0000000..e153baa --- /dev/null +++ b/mojo/public/utility/scoped_handle.h @@ -0,0 +1,27 @@ +// Copyright 2013 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. + +#ifndef MOJO_PUBLIC_UTILITY_SCOPED_HANDLE_H_ +#define MOJO_PUBLIC_UTILITY_SCOPED_HANDLE_H_ + +#include "mojo/public/system/core.h" + +namespace mojo { + +// Closes handle_ when deleted. +// This probably wants tons of improvements, but those can be made as needed. +class ScopedHandle { + public: + ScopedHandle(Handle handle); + ~ScopedHandle(); + Handle get() const { return handle_; } + Handle Release(); + + private: + Handle handle_; +}; + +} // namespace mojo + +#endif // MOJO_PUBLIC_SYSTEM_SCOPED_HANDLE_H_ diff --git a/mojo/shell/app_container.cc b/mojo/shell/app_container.cc index 234f06d..221c871 100644 --- a/mojo/shell/app_container.cc +++ b/mojo/shell/app_container.cc @@ -6,12 +6,15 @@ #include "base/bind.h" #include "base/callback_forward.h" +#include "base/callback_helpers.h" #include "base/file_util.h" #include "base/files/file_path.h" #include "base/native_library.h" +#include "base/scoped_native_library.h" #include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" #include "mojo/public/system/core.h" +#include "mojo/public/utility/scoped_handle.h" #include "mojo/services/native_viewport/native_viewport_controller.h" #include "mojo/shell/context.h" @@ -22,36 +25,32 @@ namespace shell { void LaunchAppOnThread( const base::FilePath& app_path, - Handle app_handle) { - MojoResult result = MOJO_RESULT_OK; - MojoMainFunction main_function = NULL; - - base::NativeLibrary app_library = base::LoadNativeLibrary(app_path, NULL); - if (!app_library) { + Handle app_handle_raw) { + base::ScopedClosureRunner app_deleter( + base::Bind(base::IgnoreResult(&base::DeleteFile), app_path, false)); + ScopedHandle app_handle(app_handle_raw); + + base::ScopedNativeLibrary app_library( + base::LoadNativeLibrary(app_path, NULL)); + if (!app_library.is_valid()) { LOG(ERROR) << "Failed to load library: " << app_path.value().c_str(); - goto completed; + return; } - main_function = reinterpret_cast( - base::GetFunctionPointerFromNativeLibrary(app_library, "MojoMain")); + MojoMainFunction main_function = reinterpret_cast( + app_library.GetFunctionPointer("MojoMain")); if (!main_function) { LOG(ERROR) << "Entrypoint MojoMain not found."; - goto completed; + return; } - result = main_function(app_handle); + MojoResult result = main_function(app_handle.get()); if (result < MOJO_RESULT_OK) { LOG(ERROR) << "MojoMain returned an error: " << result; - // TODO(*): error handling? - goto completed; + return; } LOG(INFO) << "MojoMain succeeded: " << result; - -completed: - base::UnloadNativeLibrary(app_library); - base::DeleteFile(app_path, false); - Close(app_handle); } AppContainer::AppContainer(Context* context) -- cgit v1.1