diff --git a/DEPS b/DEPS index a9fe226..299a692f 100644 --- a/DEPS +++ b/DEPS
@@ -43,7 +43,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other. - 'v8_revision': '1f168dc0b2346e0596b0e4939ca20bf316fc4fae', + 'v8_revision': '96f1766e51a8bd9428b876de118f03c762f1a591', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling swarming_client # and whatever else without interference from each other.
diff --git a/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc b/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc index fe68492..d24b8e2 100644 --- a/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc +++ b/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc
@@ -328,6 +328,7 @@ bool has_selection, const PrintingContextLinux::PrintSettingsCallback& callback) { callback_ = callback; + DCHECK(!callback_.is_null()); dialog_ = gtk_print_unix_dialog_new(NULL, NULL); libgtk2ui::SetGtkTransientForAura(dialog_, parent_view); @@ -550,5 +551,8 @@ libgtk2ui::ClearAuraTransientParent(dialog_); window->RemoveObserver(this); - Release(); + if (!callback_.is_null()) { + callback_.Run(PrintingContextLinux::CANCEL); + callback_.Reset(); + } }
diff --git a/components/filesystem/BUILD.gn b/components/filesystem/BUILD.gn index f9d9d05..fe7bb2d 100644 --- a/components/filesystem/BUILD.gn +++ b/components/filesystem/BUILD.gn
@@ -11,6 +11,8 @@ "directory_impl.h", "file_impl.cc", "file_impl.h", + "file_system_app.cc", + "file_system_app.h", "file_system_impl.cc", "file_system_impl.h", "lock_table.cc", @@ -25,6 +27,7 @@ "//mojo/common", "//mojo/common:common_base", "//mojo/platform_handle", + "//mojo/services/tracing/public/cpp", "//mojo/shell/public/cpp", "//mojo/shell/public/interfaces", "//url", @@ -33,8 +36,6 @@ mojo_native_application("filesystem") { sources = [ - "file_system_app.cc", - "file_system_app.h", "main.cc", ] @@ -42,13 +43,11 @@ ":lib", ":manifest", "//base", - "//components/filesystem/public/interfaces", "//mojo/common", "//mojo/environment:chromium", "//mojo/platform_handle:for_shared_library", "//mojo/public/cpp/bindings", "//mojo/public/cpp/system", - "//mojo/services/tracing/public/cpp", "//mojo/shell/public/cpp", ] }
diff --git a/components/filesystem/file_system_app.cc b/components/filesystem/file_system_app.cc index d5d577a..344a7ca 100644 --- a/components/filesystem/file_system_app.cc +++ b/components/filesystem/file_system_app.cc
@@ -4,13 +4,17 @@ #include "components/filesystem/file_system_app.h" +#include <utility> + +#include "base/bind.h" +#include "base/logging.h" #include "mojo/shell/public/cpp/connection.h" #include "mojo/shell/public/cpp/shell.h" namespace filesystem { FileSystemApp::FileSystemApp() - : shell_(nullptr), lock_table_(new LockTable) {} + : shell_(nullptr), lock_table_(new LockTable), in_shutdown_(false) {} FileSystemApp::~FileSystemApp() {} @@ -25,10 +29,70 @@ return true; } +void FileSystemApp::RegisterDirectoryToClient(DirectoryImpl* directory, + FileSystemClientPtr client) { + directory->set_connection_error_handler( + base::Bind(&FileSystemApp::OnDirectoryConnectionError, + base::Unretained(this), + directory)); + client_mapping_.emplace_back(directory, std::move(client)); +} + +bool FileSystemApp::ShellConnectionLost() { + if (client_mapping_.empty()) { + // If we have no current connections, we can shutdown immediately. + return true; + } + + in_shutdown_ = true; + + // We have live connections. Send a notification to each one indicating that + // they should shutdown. + for (std::vector<Client>::iterator it = client_mapping_.begin(); + it != client_mapping_.end(); ++it) { + it->fs_client_->OnFileSystemShutdown(); + } + + return false; +} + // |InterfaceFactory<Files>| implementation: void FileSystemApp::Create(mojo::Connection* connection, mojo::InterfaceRequest<FileSystem> request) { - new FileSystemImpl(connection, std::move(request), lock_table_.get()); + new FileSystemImpl(this, connection, std::move(request), lock_table_.get()); +} + +void FileSystemApp::OnDirectoryConnectionError(DirectoryImpl* directory) { + for (std::vector<Client>::iterator it = client_mapping_.begin(); + it != client_mapping_.end(); ++it) { + if (it->directory_ == directory) { + client_mapping_.erase(it); + + if (in_shutdown_ && client_mapping_.empty()) { + // We just cleared the last directory after our shell connection went + // away. Time to shut ourselves down. + shell_->Quit(); + } + + return; + } + } +} + +FileSystemApp::Client::Client(DirectoryImpl* directory, + FileSystemClientPtr fs_client) + : directory_(directory), fs_client_(std::move(fs_client)) {} + +FileSystemApp::Client::Client(Client&& rhs) + : directory_(rhs.directory_), fs_client_(std::move(rhs.fs_client_)) {} + +FileSystemApp::Client::~Client() {} + +FileSystemApp::Client& FileSystemApp::Client::operator=( + FileSystemApp::Client&& rhs) { + directory_ = rhs.directory_; + fs_client_ = std::move(rhs.fs_client_); + return *this; } } // namespace filesystem
diff --git a/components/filesystem/file_system_app.h b/components/filesystem/file_system_app.h index ae62469..402c359 100644 --- a/components/filesystem/file_system_app.h +++ b/components/filesystem/file_system_app.h
@@ -26,21 +26,50 @@ FileSystemApp(); ~FileSystemApp() override; + // Called by individual FileSystem objects to register lifetime events. + void RegisterDirectoryToClient(DirectoryImpl* directory, + FileSystemClientPtr client); + private: + // We set the DirectoryImpl's error handler to this function. We do this so + // that we can QuitNow() once the last DirectoryImpl has closed itself. + void OnDirectoryConnectionError(DirectoryImpl* directory); + // |mojo::ShellClient| override: void Initialize(mojo::Shell* shell, const std::string& url, uint32_t id) override; bool AcceptConnection(mojo::Connection* connection) override; + bool ShellConnectionLost() override; // |InterfaceFactory<Files>| implementation: void Create(mojo::Connection* connection, mojo::InterfaceRequest<FileSystem> request) override; + // Use a vector to work around map not letting us use FileSystemClientPtr as + // a value in a std::map. The move constructors are to allow us to deal with + // FileSystemClientPtr inside a vector. + struct Client { + Client(DirectoryImpl* directory, FileSystemClientPtr fs_client); + Client(Client&& rhs); + ~Client(); + + Client& operator=(Client&& rhs); + + DirectoryImpl* directory_; + FileSystemClientPtr fs_client_; + }; + std::vector<Client> client_mapping_; + mojo::Shell* shell_; mojo::TracingImpl tracing_; scoped_ptr<LockTable> lock_table_; + // Set to true when our shell connection is closed. On connection error, we + // then broadcast a notification to all FileSystemClients that they should + // shut down. Once the final one does, then we QuitNow(). + bool in_shutdown_; + DISALLOW_COPY_AND_ASSIGN(FileSystemApp); };
diff --git a/components/filesystem/file_system_impl.cc b/components/filesystem/file_system_impl.cc index cf444b3..5a60c5c 100644 --- a/components/filesystem/file_system_impl.cc +++ b/components/filesystem/file_system_impl.cc
@@ -16,6 +16,7 @@ #include "base/memory/scoped_ptr.h" #include "build/build_config.h" #include "components/filesystem/directory_impl.h" +#include "components/filesystem/file_system_app.h" #include "mojo/shell/public/cpp/connection.h" #include "url/gurl.h" @@ -44,10 +45,12 @@ } // namespace filesystem -FileSystemImpl::FileSystemImpl(mojo::Connection* connection, +FileSystemImpl::FileSystemImpl(FileSystemApp* app, + mojo::Connection* connection, mojo::InterfaceRequest<FileSystem> request, LockTable* lock_table) - : remote_application_url_(connection->GetRemoteApplicationURL()), + : app_(app), + remote_application_url_(connection->GetRemoteApplicationURL()), binding_(this, std::move(request)), lock_table_(lock_table) {} @@ -86,8 +89,9 @@ } if (!path.empty()) { - new DirectoryImpl( + DirectoryImpl* dir_impl = new DirectoryImpl( std::move(directory), path, std::move(temp_dir), lock_table_); + app_->RegisterDirectoryToClient(dir_impl, std::move(client)); callback.Run(FileError::OK); } else { callback.Run(FileError::FAILED);
diff --git a/components/filesystem/file_system_impl.h b/components/filesystem/file_system_impl.h index 4c18dd5..11ac05d 100644 --- a/components/filesystem/file_system_impl.h +++ b/components/filesystem/file_system_impl.h
@@ -25,7 +25,8 @@ class FileSystemImpl : public FileSystem { public: - FileSystemImpl(mojo::Connection* connection, + FileSystemImpl(FileSystemApp* app, + mojo::Connection* connection, mojo::InterfaceRequest<FileSystem> request, LockTable* lock_table); ~FileSystemImpl() override; @@ -51,6 +52,7 @@ void BuildSanitizedOrigin(const std::string& origin, std::string* sanitized_origin); + FileSystemApp* app_; const std::string remote_application_url_; mojo::StrongBinding<FileSystem> binding_; LockTable* lock_table_;
diff --git a/components/filesystem/filesystem.gyp b/components/filesystem/filesystem.gyp deleted file mode 100644 index 396dac5d..0000000 --- a/components/filesystem/filesystem.gyp +++ /dev/null
@@ -1,67 +0,0 @@ -# Copyright 2016 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. - -{ - 'variables': { - # This turns on e.g. the filename-based detection of which - # platforms to include source files on (e.g. files ending in - # _mac.h or _mac.cc are only compiled on MacOSX). - 'chromium_code': 1, - }, - 'targets': [ - { - # GN version: //components/filesystem:lib - 'target_name': 'filesystem_lib', - 'type': 'static_library', - 'include_dirs': [ - '../..', - ], - 'sources': [ - 'directory_impl.cc', - 'directory_impl.h', - 'file_impl.cc', - 'file_impl.h', - 'file_system_impl.cc', - 'file_system_impl.h', - 'lock_table.cc', - 'lock_table.h', - 'util.cc', - 'util.h', - ], - 'dependencies': [ - 'filesystem_bindings', - '../../mojo/mojo_edk.gyp:mojo_system_impl', - '../../mojo/mojo_public.gyp:mojo_cpp_bindings', - '../../mojo/mojo_platform_handle.gyp:platform_handle', - '../../url/url.gyp:url_lib', - ], - 'export_dependent_settings': [ - 'filesystem_bindings', - ], - }, - { - # GN version: //components/filesystem/public/interfaces - 'target_name': 'filesystem_bindings', - 'type': 'static_library', - 'dependencies': [ - 'filesystem_bindings_mojom', - ], - }, - { - 'target_name': 'filesystem_bindings_mojom', - 'type': 'none', - 'variables': { - 'mojom_files': [ - 'public/interfaces/directory.mojom', - 'public/interfaces/file.mojom', - 'public/interfaces/file_system.mojom', - 'public/interfaces/types.mojom', - ], - }, - 'includes': [ - '../../mojo/mojom_bindings_generator_explicit.gypi', - ], - } - ], -}
diff --git a/components/leveldb/BUILD.gn b/components/leveldb/BUILD.gn index 5bdef0a1..db5dd1e 100644 --- a/components/leveldb/BUILD.gn +++ b/components/leveldb/BUILD.gn
@@ -9,12 +9,12 @@ sources = [ "env_mojo.cc", "env_mojo.h", + "leveldb_app.cc", + "leveldb_app.h", "leveldb_database_impl.cc", "leveldb_database_impl.h", "leveldb_file_thread.cc", "leveldb_file_thread.h", - "leveldb_service_impl.cc", - "leveldb_service_impl.h", "util.cc", "util.h", ] @@ -25,6 +25,7 @@ "//mojo/common", "//mojo/message_pump", "//mojo/platform_handle", + "//mojo/services/tracing/public/cpp", "//mojo/shell/public/cpp", "//third_party/leveldatabase", ] @@ -32,21 +33,17 @@ mojo_native_application("leveldb") { sources = [ - "leveldb_app.cc", - "leveldb_app.h", "main.cc", ] deps = [ ":lib", ":manifest", - "//components/leveldb/public/interfaces", "//mojo/common", "//mojo/environment:chromium", "//mojo/platform_handle:for_shared_library", "//mojo/public/cpp/bindings", "//mojo/public/cpp/system", - "//mojo/services/tracing/public/cpp", "//mojo/shell/public/cpp", ] }
diff --git a/components/leveldb/leveldb.gyp b/components/leveldb/leveldb.gyp deleted file mode 100644 index 5d39a72..0000000 --- a/components/leveldb/leveldb.gyp +++ /dev/null
@@ -1,63 +0,0 @@ -# Copyright 2016 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. - -{ - 'variables': { - # This turns on e.g. the filename-based detection of which - # platforms to include source files on (e.g. files ending in - # _mac.h or _mac.cc are only compiled on MacOSX). - 'chromium_code': 1, - }, - 'targets': [ - { - # GN version: //components/leveldb:lib - 'target_name': 'leveldb_lib', - 'type': 'static_library', - 'include_dirs': [ - '../..', - ], - 'sources': [ - 'env_mojo.cc', - 'env_mojo.h', - 'leveldb_database_impl.cc', - 'leveldb_database_impl.h', - 'leveldb_file_thread.cc', - 'leveldb_file_thread.h', - 'leveldb_service_impl.cc', - 'leveldb_service_impl.h', - 'util.cc', - 'util.h', - ], - 'dependencies': [ - 'leveldb_bindings_mojom', - '../../components/filesystem/filesystem.gyp:filesystem_lib', - '../../mojo/mojo_public.gyp:mojo_cpp_bindings', - '../../third_party/leveldatabase/leveldatabase.gyp:leveldatabase', - ] - }, - { - # GN version: //components/leveldb/public/interfaces - 'target_name': 'leveldb_bindings', - 'type': 'static_library', - 'dependencies': [ - 'leveldb_bindings_mojom', - ], - }, - { - 'target_name': 'leveldb_bindings_mojom', - 'type': 'none', - 'variables': { - 'mojom_files': [ - 'public/interfaces/leveldb.mojom', - ], - }, - 'dependencies': [ - '../../components/filesystem/filesystem.gyp:filesystem_bindings_mojom', - ], - 'includes': [ - '../../mojo/mojom_bindings_generator_explicit.gypi', - ], - } - ], -}
diff --git a/components/leveldb/leveldb_app.cc b/components/leveldb/leveldb_app.cc index 1e9d1682..bd8ac71 100644 --- a/components/leveldb/leveldb_app.cc +++ b/components/leveldb/leveldb_app.cc
@@ -4,8 +4,15 @@ #include "components/leveldb/leveldb_app.h" -#include "components/leveldb/leveldb_service_impl.h" +#include "components/leveldb/env_mojo.h" +#include "components/leveldb/leveldb_database_impl.h" +#include "components/leveldb/util.h" #include "mojo/shell/public/cpp/connection.h" +#include "third_party/leveldatabase/env_chromium.h" +#include "third_party/leveldatabase/src/include/leveldb/db.h" +#include "third_party/leveldatabase/src/include/leveldb/env.h" +#include "third_party/leveldatabase/src/include/leveldb/filter_policy.h" +#include "third_party/leveldatabase/src/include/leveldb/slice.h" namespace leveldb { @@ -17,7 +24,7 @@ const std::string& url, uint32_t id) { tracing_.Initialize(shell, url); - service_.reset(new LevelDBServiceImpl); + thread_ = new LevelDBFileThread; } bool LevelDBApp::AcceptConnection(mojo::Connection* connection) { @@ -27,7 +34,41 @@ void LevelDBApp::Create(mojo::Connection* connection, mojo::InterfaceRequest<LevelDBService> request) { - bindings_.AddBinding(service_.get(), std::move(request)); + bindings_.AddBinding(this, std::move(request)); +} + +void LevelDBApp::Open(filesystem::DirectoryPtr directory, + const mojo::String& dbname, + mojo::InterfaceRequest<LevelDBDatabase> database, + const OpenCallback& callback) { + // This is the place where we open a database. + leveldb::Options options; + options.create_if_missing = true; + options.paranoid_checks = true; + // TODO(erg): Do we need a filter policy? + options.reuse_logs = leveldb_env::kDefaultLogReuseOptionValue; + options.compression = leveldb::kSnappyCompression; + + // For info about the troubles we've run into with this parameter, see: + // https://code.google.com/p/chromium/issues/detail?id=227313#c11 + options.max_open_files = 80; + + // Register our directory with the file thread. + LevelDBFileThread::OpaqueDir* dir = + thread_->RegisterDirectory(std::move(directory)); + + scoped_ptr<MojoEnv> env_mojo(new MojoEnv(thread_, dir)); + options.env = env_mojo.get(); + + leveldb::DB* db = nullptr; + leveldb::Status s = leveldb::DB::Open(options, dbname.To<std::string>(), &db); + + if (s.ok()) { + new LevelDBDatabaseImpl(std::move(database), std::move(env_mojo), + scoped_ptr<leveldb::DB>(db)); + } + + callback.Run(LeveldbStatusToError(s)); } } // namespace leveldb
diff --git a/components/leveldb/leveldb_app.h b/components/leveldb/leveldb_app.h index 7f5157a..64cec0b 100644 --- a/components/leveldb/leveldb_app.h +++ b/components/leveldb/leveldb_app.h
@@ -5,6 +5,7 @@ #ifndef COMPONENTS_LEVELDB_LEVELDB_APP_H_ #define COMPONENTS_LEVELDB_LEVELDB_APP_H_ +#include "components/leveldb/leveldb_file_thread.h" #include "components/leveldb/public/interfaces/leveldb.mojom.h" #include "mojo/public/cpp/bindings/weak_binding_set.h" #include "mojo/services/tracing/public/cpp/tracing_impl.h" @@ -14,7 +15,8 @@ namespace leveldb { class LevelDBApp : public mojo::ShellClient, - public mojo::InterfaceFactory<LevelDBService> { + public mojo::InterfaceFactory<LevelDBService>, + public LevelDBService { public: LevelDBApp(); ~LevelDBApp() override; @@ -33,10 +35,20 @@ void Create(mojo::Connection* connection, mojo::InterfaceRequest<LevelDBService> request) override; + // Overridden from LevelDBService: + void Open(filesystem::DirectoryPtr directory, + const mojo::String& dbname, + mojo::InterfaceRequest<LevelDBDatabase> database, + const OpenCallback& callback) override; + mojo::TracingImpl tracing_; - scoped_ptr<LevelDBService> service_; mojo::WeakBindingSet<LevelDBService> bindings_; + // Thread to own the mojo message pipe. Because leveldb spawns multiple + // threads that want to call file stuff, we create a dedicated thread to send + // and receive mojo message calls. + scoped_refptr<LevelDBFileThread> thread_; + DISALLOW_COPY_AND_ASSIGN(LevelDBApp); };
diff --git a/components/leveldb/leveldb_service_impl.cc b/components/leveldb/leveldb_service_impl.cc deleted file mode 100644 index 404019e9..0000000 --- a/components/leveldb/leveldb_service_impl.cc +++ /dev/null
@@ -1,58 +0,0 @@ -// Copyright 2016 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 "components/leveldb/leveldb_service_impl.h" - -#include "components/leveldb/env_mojo.h" -#include "components/leveldb/leveldb_database_impl.h" -#include "components/leveldb/util.h" -#include "third_party/leveldatabase/env_chromium.h" -#include "third_party/leveldatabase/src/include/leveldb/db.h" -#include "third_party/leveldatabase/src/include/leveldb/env.h" -#include "third_party/leveldatabase/src/include/leveldb/filter_policy.h" -#include "third_party/leveldatabase/src/include/leveldb/slice.h" - -namespace leveldb { - -LevelDBServiceImpl::LevelDBServiceImpl() - : thread_(new LevelDBFileThread) { -} - -LevelDBServiceImpl::~LevelDBServiceImpl() {} - -void LevelDBServiceImpl::Open(filesystem::DirectoryPtr directory, - const mojo::String& dbname, - mojo::InterfaceRequest<LevelDBDatabase> database, - const OpenCallback& callback) { - // This is the place where we open a database. - leveldb::Options options; - options.create_if_missing = true; - options.paranoid_checks = true; - // TODO(erg): Do we need a filter policy? - options.reuse_logs = leveldb_env::kDefaultLogReuseOptionValue; - options.compression = leveldb::kSnappyCompression; - - // For info about the troubles we've run into with this parameter, see: - // https://code.google.com/p/chromium/issues/detail?id=227313#c11 - options.max_open_files = 80; - - // Register our directory with the file thread. - LevelDBFileThread::OpaqueDir* dir = - thread_->RegisterDirectory(std::move(directory)); - - scoped_ptr<MojoEnv> env_mojo(new MojoEnv(thread_, dir)); - options.env = env_mojo.get(); - - leveldb::DB* db = nullptr; - leveldb::Status s = leveldb::DB::Open(options, dbname.To<std::string>(), &db); - - if (s.ok()) { - new LevelDBDatabaseImpl(std::move(database), std::move(env_mojo), - scoped_ptr<leveldb::DB>(db)); - } - - callback.Run(LeveldbStatusToError(s)); -} - -} // namespace leveldb
diff --git a/components/leveldb/leveldb_service_impl.h b/components/leveldb/leveldb_service_impl.h deleted file mode 100644 index 54b2942..0000000 --- a/components/leveldb/leveldb_service_impl.h +++ /dev/null
@@ -1,37 +0,0 @@ -// Copyright 2016 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 COMPONENTS_LEVELDB_LEVELDB_SERVICE_IMPL_H_ -#define COMPONENTS_LEVELDB_LEVELDB_SERVICE_IMPL_H_ - -#include "base/memory/ref_counted.h" -#include "components/leveldb/leveldb_file_thread.h" -#include "components/leveldb/public/interfaces/leveldb.mojom.h" - -namespace leveldb { - -// Creates LevelDBDatabases based scoped to a |directory|/|dbname|. -class LevelDBServiceImpl : public LevelDBService { - public: - LevelDBServiceImpl(); - ~LevelDBServiceImpl() override; - - // Overridden from LevelDBService: - void Open(filesystem::DirectoryPtr directory, - const mojo::String& dbname, - mojo::InterfaceRequest<LevelDBDatabase> database, - const OpenCallback& callback) override; - - private: - // Thread to own the mojo message pipe. Because leveldb spawns multiple - // threads that want to call file stuff, we create a dedicated thread to send - // and receive mojo message calls. - scoped_refptr<LevelDBFileThread> thread_; - - DISALLOW_COPY_AND_ASSIGN(LevelDBServiceImpl); -}; - -} // namespace leveldb - -#endif // COMPONENTS_LEVELDB_LEVELDB_SERVICE_IMPL_H_
diff --git a/components/nacl/renderer/plugin/plugin.cc b/components/nacl/renderer/plugin/plugin.cc index 2635fbb..929ad5e08 100644 --- a/components/nacl/renderer/plugin/plugin.cc +++ b/components/nacl/renderer/plugin/plugin.cc
@@ -33,12 +33,6 @@ main_subprocess_.Shutdown(); } -void Plugin::StartSelLdr(ServiceRuntime* service_runtime, - const SelLdrStartParams& params, - pp::CompletionCallback callback) { - service_runtime->StartSelLdr(params, callback); -} - void Plugin::LoadNaClModule(PP_NaClFileInfo file_info, bool uses_nonsfi_mode, PP_NaClAppProcessType process_type) { @@ -60,8 +54,8 @@ this, pp_instance(), true, uses_nonsfi_mode); main_subprocess_.set_service_runtime(service_runtime); - StartSelLdr(service_runtime, params, - pp::CompletionCallback(NoOpCallback, NULL)); + service_runtime->StartSelLdr(params, + pp::CompletionCallback(NoOpCallback, NULL)); } void Plugin::LoadHelperNaClModule(const std::string& helper_url, @@ -79,7 +73,7 @@ false, // Not main_service_runtime. false); // No non-SFI mode (i.e. in SFI-mode). subprocess_to_init->set_service_runtime(service_runtime); - StartSelLdr(service_runtime, params, callback); + service_runtime->StartSelLdr(params, callback); } // All failures of this function will show up as "Missing Plugin-in", so
diff --git a/components/nacl/renderer/plugin/plugin.h b/components/nacl/renderer/plugin/plugin.h index 83574bb20..e3ce0ce 100644 --- a/components/nacl/renderer/plugin/plugin.h +++ b/components/nacl/renderer/plugin/plugin.h
@@ -104,11 +104,6 @@ // in this order, for the main nacl subprocess. void ShutDownSubprocesses(); - // Start sel_ldr given the start params. This is invoked on the main thread. - void StartSelLdr(ServiceRuntime* service_runtime, - const SelLdrStartParams& params, - pp::CompletionCallback callback); - // Callback used when getting the URL for the .nexe file. If the URL loading // is successful, the file descriptor is opened and can be passed to sel_ldr // with the sandbox on.
diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn index 8ae5275..7559b7f 100644 --- a/content/browser/BUILD.gn +++ b/content/browser/BUILD.gn
@@ -117,7 +117,6 @@ deps += [ "//cc", "//cc/surfaces", - "//components/leveldb:lib", "//components/scheduler:common", "//content/app/resources", "//content/app/strings",
diff --git a/content/browser/frame_host/frame_mojo_shell.cc b/content/browser/frame_host/frame_mojo_shell.cc index b72123b4..9541672 100644 --- a/content/browser/frame_host/frame_mojo_shell.cc +++ b/content/browser/frame_host/frame_mojo_shell.cc
@@ -13,6 +13,7 @@ #include "content/public/browser/render_frame_host.h" #include "content/public/browser/site_instance.h" #include "content/public/common/content_client.h" +#include "mojo/common/url_type_converters.h" #if defined(OS_ANDROID) && defined(ENABLE_MOJO_CDM) #include "content/browser/media/android/provision_fetcher_impl.h" @@ -47,12 +48,12 @@ // TODO(xhwang): Currently no callers are exposing |exposed_services|. So we // drop it and replace it with services we provide in the browser. In the // future we may need to support both. -void FrameMojoShell::ConnectToApplication( - mojo::URLRequestPtr application_url, +void FrameMojoShell::Connect( + const mojo::String& application_url, mojo::shell::mojom::InterfaceProviderRequest services, mojo::shell::mojom::InterfaceProviderPtr /* exposed_services */, mojo::shell::mojom::CapabilityFilterPtr filter, - const ConnectToApplicationCallback& callback) { + const ConnectCallback& callback) { mojo::shell::mojom::InterfaceProviderPtr frame_services; service_provider_bindings_.AddBinding(GetServiceRegistry(), GetProxy(&frame_services)); @@ -62,7 +63,7 @@ if (!filter.is_null()) capability_filter = filter->filter.To<mojo::shell::CapabilityFilter>(); MojoShellContext::ConnectToApplication( - GURL(application_url->url.get()), + application_url.To<GURL>(), frame_host_->GetSiteInstance()->GetSiteURL(), std::move(services), std::move(frame_services), capability_filter, callback); }
diff --git a/content/browser/frame_host/frame_mojo_shell.h b/content/browser/frame_host/frame_mojo_shell.h index 0f53a011..9c41fb2 100644 --- a/content/browser/frame_host/frame_mojo_shell.h +++ b/content/browser/frame_host/frame_mojo_shell.h
@@ -29,12 +29,12 @@ private: // mojo::Shell: - void ConnectToApplication( - mojo::URLRequestPtr application_url, + void Connect( + const mojo::String& application_url, mojo::shell::mojom::InterfaceProviderRequest services, mojo::shell::mojom::InterfaceProviderPtr exposed_services, mojo::shell::mojom::CapabilityFilterPtr filter, - const ConnectToApplicationCallback& callback) override; + const ConnectCallback& callback) override; void QuitApplication() override; ServiceRegistryImpl* GetServiceRegistry();
diff --git a/content/browser/mojo/mojo_shell_context.cc b/content/browser/mojo/mojo_shell_context.cc index 980c7a6..13459912 100644 --- a/content/browser/mojo/mojo_shell_context.cc +++ b/content/browser/mojo/mojo_shell_context.cc
@@ -25,7 +25,7 @@ #include "mojo/public/cpp/bindings/interface_request.h" #include "mojo/public/cpp/bindings/string.h" #include "mojo/shell/application_loader.h" -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" #include "mojo/shell/identity.h" #include "mojo/shell/public/cpp/shell_client.h" @@ -160,7 +160,7 @@ mojo::shell::mojom::InterfaceProviderRequest request, mojo::shell::mojom::InterfaceProviderPtr exposed_services, const mojo::shell::CapabilityFilter& filter, - const mojo::shell::mojom::Shell::ConnectToApplicationCallback& callback) { + const mojo::shell::mojom::Shell::ConnectCallback& callback) { if (task_runner_ == base::ThreadTaskRunnerHandle::Get()) { if (shell_context_) { shell_context_->ConnectToApplicationOnOwnThread( @@ -259,7 +259,7 @@ mojo::shell::mojom::InterfaceProviderRequest request, mojo::shell::mojom::InterfaceProviderPtr exposed_services, const mojo::shell::CapabilityFilter& filter, - const mojo::shell::mojom::Shell::ConnectToApplicationCallback& callback) { + const mojo::shell::mojom::Shell::ConnectCallback& callback) { proxy_.Get()->ConnectToApplication(url, requestor_url, std::move(request), std::move(exposed_services), filter, callback); @@ -271,18 +271,16 @@ mojo::shell::mojom::InterfaceProviderRequest request, mojo::shell::mojom::InterfaceProviderPtr exposed_services, const mojo::shell::CapabilityFilter& filter, - const mojo::shell::mojom::Shell::ConnectToApplicationCallback& callback) { - scoped_ptr<mojo::shell::ConnectToApplicationParams> params( - new mojo::shell::ConnectToApplicationParams); + const mojo::shell::mojom::Shell::ConnectCallback& callback) { + scoped_ptr<mojo::shell::ConnectParams> params(new mojo::shell::ConnectParams); params->set_source( mojo::shell::Identity(requestor_url, std::string(), mojo::shell::GetPermissiveCapabilityFilter())); params->set_target(mojo::shell::Identity(url, std::string(), filter)); params->set_remote_interfaces(std::move(request)); params->set_local_interfaces(std::move(exposed_services)); - params->set_on_application_end(base::Bind(&base::DoNothing)); params->set_connect_callback(callback); - application_manager_->ConnectToApplication(std::move(params)); + application_manager_->Connect(std::move(params)); } } // namespace content
diff --git a/content/browser/mojo/mojo_shell_context.h b/content/browser/mojo/mojo_shell_context.h index b0bda137..d542654 100644 --- a/content/browser/mojo/mojo_shell_context.h +++ b/content/browser/mojo/mojo_shell_context.h
@@ -44,7 +44,7 @@ mojo::shell::mojom::InterfaceProviderRequest request, mojo::shell::mojom::InterfaceProviderPtr exposed_services, const mojo::shell::CapabilityFilter& filter, - const mojo::shell::mojom::Shell::ConnectToApplicationCallback& callback); + const mojo::shell::mojom::Shell::ConnectCallback& callback); static void SetApplicationsForTest(const StaticApplicationMap* apps); @@ -58,7 +58,7 @@ mojo::shell::mojom::InterfaceProviderRequest request, mojo::shell::mojom::InterfaceProviderPtr exposed_services, const mojo::shell::CapabilityFilter& filter, - const mojo::shell::mojom::Shell::ConnectToApplicationCallback& callback); + const mojo::shell::mojom::Shell::ConnectCallback& callback); static base::LazyInstance<scoped_ptr<Proxy>> proxy_;
diff --git a/content/content_browser.gypi b/content/content_browser.gypi index 95ba8499..ab3d253 100644 --- a/content/content_browser.gypi +++ b/content/content_browser.gypi
@@ -7,8 +7,6 @@ '../base/base.gyp:base_static', '../cc/cc.gyp:cc', '../cc/cc.gyp:cc_surfaces', - '../components/filesystem/filesystem.gyp:filesystem_lib', - '../components/leveldb/leveldb.gyp:leveldb_lib', '../components/mime_util/mime_util.gyp:mime_util', '../components/scheduler/scheduler.gyp:scheduler_common', '../components/url_formatter/url_formatter.gyp:url_formatter',
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index e2afa2a..1ef1619 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc
@@ -6022,15 +6022,13 @@ if (!mojo_shell_) GetServiceRegistry()->ConnectToRemoteService(mojo::GetProxy(&mojo_shell_)); mojo::shell::mojom::InterfaceProviderPtr interface_provider; - mojo::URLRequestPtr request(mojo::URLRequest::New()); - request->url = mojo::String::From(url); mojo::shell::mojom::CapabilityFilterPtr filter( mojo::shell::mojom::CapabilityFilter::New()); mojo::Array<mojo::String> all_interfaces; all_interfaces.push_back("*"); filter->filter.insert("*", std::move(all_interfaces)); - mojo_shell_->ConnectToApplication( - std::move(request), GetProxy(&interface_provider), nullptr, + mojo_shell_->Connect( + url.spec(), GetProxy(&interface_provider), nullptr, std::move(filter), base::Bind(&OnGotInstanceID)); return interface_provider; }
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn index 6132d49..dac93bc 100644 --- a/content/test/BUILD.gn +++ b/content/test/BUILD.gn
@@ -3,6 +3,7 @@ # found in the LICENSE file. import("//build/config/chrome_build.gni") +import("//build/config/compiler/compiler.gni") import("//build/config/crypto.gni") import("//build/config/features.gni") import("//build/config/ui.gni") @@ -392,6 +393,10 @@ # TODO(GYP): Add this once //content/shell:copy_test_netscape_plugin # exists. #data += [ "$root_out_dir/plugins/np_test_netscape_plugin.dll" ] + + if (symbol_level != 0) { + data += [ "$root_out_dir/content_browsertests.exe.pdb" ] + } } } }
diff --git a/content/test/data/web_ui_mojo_shell_test.js b/content/test/data/web_ui_mojo_shell_test.js index 42486e19e..6b0f7e1 100644 --- a/content/test/data/web_ui_mojo_shell_test.js +++ b/content/test/data/web_ui_mojo_shell_test.js
@@ -30,8 +30,7 @@ domAutomationController.setAutomationId(0); var shellPipe = serviceRegistry.connectToService(shellMojom.Shell.name); var shell = new shellMojom.Shell.proxyClass(new router.Router(shellPipe)); - var urlRequest = new urlMojom.URLRequest({ url: TEST_APP_URL }); - shell.connectToApplication(urlRequest, + shell.connect(TEST_APP_URL, function (services) { var test = connectToService(services, testMojom.TestMojoService); test.getRequestorURL().then(function(response) {
diff --git a/mojo/mojo_base.gyp b/mojo/mojo_base.gyp index e2d3727..cd4d46c 100644 --- a/mojo/mojo_base.gyp +++ b/mojo/mojo_base.gyp
@@ -213,12 +213,6 @@ 'shell/public/interfaces/shell_client_factory.mojom', ], }, - 'dependencies': [ - 'mojo_services.gyp:network_service_bindings_generation', - ], - 'export_dependent_settings': [ - 'mojo_services.gyp:network_service_bindings_generation', - ], 'includes': [ 'mojom_bindings_generator_explicit.gypi' ], }, {
diff --git a/mojo/mojo_platform_handle.gyp b/mojo/mojo_platform_handle.gyp deleted file mode 100644 index 247c699..0000000 --- a/mojo/mojo_platform_handle.gyp +++ /dev/null
@@ -1,34 +0,0 @@ -# Copyright 2016 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. - -{ - 'variables': { - # This turns on e.g. the filename-based detection of which - # platforms to include source files on (e.g. files ending in - # _mac.h or _mac.cc are only compiled on MacOSX). - 'chromium_code': 1, - }, - 'targets': [ - { - # GN version: //mojo/platform_handle:platform_handle/platform_handle_impl - 'target_name': 'platform_handle', - 'type': 'static_library', - 'include_dirs': [ - '../..', - ], - 'defines': [ - 'PLATFORM_HANDLE_IMPLEMENTATION' - ], - 'sources': [ - 'platform_handle/platform_handle.h', - 'platform_handle/platform_handle_functions.h', - 'platform_handle/platform_handle_functions.cc', - ], - 'dependencies': [ - 'mojo_edk.gyp:mojo_system_impl', - 'mojo_public.gyp:mojo_cpp_bindings', - ] - } - ] -}
diff --git a/mojo/mojo_shell.gyp b/mojo/mojo_shell.gyp index 69074b448..8298d8e 100644 --- a/mojo/mojo_shell.gyp +++ b/mojo/mojo_shell.gyp
@@ -20,8 +20,8 @@ 'shell/application_manager.h', 'shell/capability_filter.cc', 'shell/capability_filter.h', - 'shell/connect_to_application_params.cc', - 'shell/connect_to_application_params.h', + 'shell/connect_params.cc', + 'shell/connect_params.h', 'shell/connect_util.cc', 'shell/connect_util.h', 'shell/identity.cc',
diff --git a/mojo/shell/BUILD.gn b/mojo/shell/BUILD.gn index 892029d..8b1882a 100644 --- a/mojo/shell/BUILD.gn +++ b/mojo/shell/BUILD.gn
@@ -27,8 +27,8 @@ "application_manager.h", "capability_filter.cc", "capability_filter.h", - "connect_to_application_params.cc", - "connect_to_application_params.h", + "connect_params.cc", + "connect_params.h", "connect_util.cc", "connect_util.h", "identity.cc", @@ -42,7 +42,6 @@ "//base", "//mojo/common", "//mojo/public/cpp/bindings", - "//mojo/services/network/public/interfaces", "//mojo/services/package_manager/public/interfaces", "//mojo/services/updater", "//mojo/shell/public/interfaces",
diff --git a/mojo/shell/application_instance.cc b/mojo/shell/application_instance.cc index cd2bc5ea..41b7cfeff 100644 --- a/mojo/shell/application_instance.cc +++ b/mojo/shell/application_instance.cc
@@ -22,14 +22,12 @@ mojom::ShellClientPtr shell_client, ApplicationManager* manager, const Identity& identity, - const base::Closure& on_application_end, const String& application_name) : manager_(manager), id_(GenerateUniqueID()), identity_(identity), allow_any_application_(identity.filter().size() == 1 && identity.filter().count("*") == 1), - on_application_end_(on_application_end), shell_client_(std::move(shell_client)), binding_(this), pid_receiver_binding_(this), @@ -52,8 +50,7 @@ binding_.set_connection_error_handler([this]() { OnConnectionError(); }); } -void ApplicationInstance::ConnectToClient( - scoped_ptr<ConnectToApplicationParams> params) { +void ApplicationInstance::ConnectToClient(scoped_ptr<ConnectParams> params) { if (queue_requests_) { queued_client_requests_.push_back(params.release()); return; @@ -72,16 +69,15 @@ } // Shell implementation: -void ApplicationInstance::ConnectToApplication( - URLRequestPtr app_request, +void ApplicationInstance::Connect( + const String& app_url, shell::mojom::InterfaceProviderRequest remote_interfaces, shell::mojom::InterfaceProviderPtr local_interfaces, mojom::CapabilityFilterPtr filter, - const ConnectToApplicationCallback& callback) { - std::string url_string = app_request->url.To<std::string>(); - GURL url(url_string); + const ConnectCallback& callback) { + GURL url = app_url.To<GURL>(); if (!url.is_valid()) { - LOG(ERROR) << "Error: invalid URL: " << url_string; + LOG(ERROR) << "Error: invalid URL: " << app_url; callback.Run(kInvalidApplicationID); return; } @@ -91,15 +87,13 @@ if (!filter.is_null()) capability_filter = filter->filter.To<CapabilityFilter>(); - scoped_ptr<ConnectToApplicationParams> params( - new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->SetSource(this); - GURL app_url(app_request->url.get()); - params->set_target(Identity(app_url, std::string(), capability_filter)); + params->set_target(Identity(url, std::string(), capability_filter)); params->set_remote_interfaces(std::move(remote_interfaces)); params->set_local_interfaces(std::move(local_interfaces)); params->set_connect_callback(callback); - manager_->ConnectToApplication(std::move(params)); + manager_->Connect(std::move(params)); } else { LOG(WARNING) << "CapabilityFilter prevented connection from: " << identity_.url() << " to: " << url.spec(); @@ -127,7 +121,7 @@ } void ApplicationInstance::CallAcceptConnection( - scoped_ptr<ConnectToApplicationParams> params) { + scoped_ptr<ConnectParams> params) { params->connect_callback().Run(id_); AllowedInterfaces interfaces; interfaces.insert("*"); @@ -144,7 +138,7 @@ } void ApplicationInstance::OnConnectionError() { - std::vector<ConnectToApplicationParams*> queued_client_requests; + std::vector<ConnectParams*> queued_client_requests; queued_client_requests_.swap(queued_client_requests); auto manager = manager_; manager_->OnApplicationInstanceError(this); @@ -153,7 +147,7 @@ // If any queued requests came to shell during time it was shutting down, // start them now. for (auto request : queued_client_requests) - manager->ConnectToApplication(make_scoped_ptr(request)); + manager->Connect(make_scoped_ptr(request)); } void ApplicationInstance::OnQuitRequestedResult(bool can_quit) {
diff --git a/mojo/shell/application_instance.h b/mojo/shell/application_instance.h index 4e97173..621761a7 100644 --- a/mojo/shell/application_instance.h +++ b/mojo/shell/application_instance.h
@@ -15,7 +15,7 @@ #include "base/process/process_handle.h" #include "mojo/public/cpp/bindings/binding.h" #include "mojo/shell/capability_filter.h" -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" #include "mojo/shell/identity.h" #include "mojo/shell/public/interfaces/application_manager.mojom.h" #include "mojo/shell/public/interfaces/shell.mojom.h" @@ -37,14 +37,12 @@ mojom::ShellClientPtr shell_client, ApplicationManager* manager, const Identity& identity, - const base::Closure& on_application_end, const String& application_name); - ~ApplicationInstance() override; void InitializeApplication(); - void ConnectToClient(scoped_ptr<ConnectToApplicationParams> params); + void ConnectToClient(scoped_ptr<ConnectParams> params); // Required before GetProcessId can be called. void SetNativeRunner(NativeRunner* native_runner); @@ -56,17 +54,15 @@ uint32_t id() const { return id_; } base::ProcessId pid() const { return pid_; } void set_pid(base::ProcessId pid) { pid_ = pid; } - base::Closure on_application_end() const { return on_application_end_; } const String& application_name() const { return application_name_; } private: // Shell implementation: - void ConnectToApplication( - URLRequestPtr app_request, - shell::mojom::InterfaceProviderRequest remote_interfaces, - shell::mojom::InterfaceProviderPtr local_interfaces, - mojom::CapabilityFilterPtr filter, - const ConnectToApplicationCallback& callback) override; + void Connect(const String& app_url, + shell::mojom::InterfaceProviderRequest remote_interfaces, + shell::mojom::InterfaceProviderPtr local_interfaces, + mojom::CapabilityFilterPtr filter, + const ConnectCallback& callback) override; void QuitApplication() override; // PIDReceiver implementation: @@ -74,7 +70,7 @@ uint32_t GenerateUniqueID() const; - void CallAcceptConnection(scoped_ptr<ConnectToApplicationParams> params); + void CallAcceptConnection(scoped_ptr<ConnectParams> params); void OnConnectionError(); @@ -89,12 +85,11 @@ const uint32_t id_; const Identity identity_; const bool allow_any_application_; - base::Closure on_application_end_; mojom::ShellClientPtr shell_client_; Binding<mojom::Shell> binding_; Binding<mojom::PIDReceiver> pid_receiver_binding_; bool queue_requests_; - std::vector<ConnectToApplicationParams*> queued_client_requests_; + std::vector<ConnectParams*> queued_client_requests_; NativeRunner* native_runner_; const String application_name_; base::ProcessId pid_;
diff --git a/mojo/shell/application_loader.h b/mojo/shell/application_loader.h index 3b22dc28..dee95447 100644 --- a/mojo/shell/application_loader.h +++ b/mojo/shell/application_loader.h
@@ -7,7 +7,6 @@ #include "base/callback.h" #include "mojo/public/cpp/system/core.h" -#include "mojo/services/network/public/interfaces/url_loader.mojom.h" #include "mojo/shell/public/interfaces/shell.mojom.h" #include "mojo/shell/public/interfaces/shell_client.mojom.h" #include "url/gurl.h" @@ -26,8 +25,7 @@ public: virtual ~ApplicationLoader() {} - virtual void Load(const GURL& url, - InterfaceRequest<mojom::ShellClient> request) = 0; + virtual void Load(const GURL& url, mojom::ShellClientRequest request) = 0; protected: ApplicationLoader() {}
diff --git a/mojo/shell/application_manager.cc b/mojo/shell/application_manager.cc index 8d01333..b2c4a49 100644 --- a/mojo/shell/application_manager.cc +++ b/mojo/shell/application_manager.cc
@@ -31,8 +31,6 @@ namespace { -void OnEmptyOnConnectCallback(uint32_t remote_id) {} - class ShellApplicationLoader : public ApplicationLoader { public: explicit ShellApplicationLoader(ApplicationManager* manager) @@ -54,10 +52,6 @@ } // namespace -mojom::Shell::ConnectToApplicationCallback EmptyConnectCallback() { - return base::Bind(&OnEmptyOnConnectCallback); -} - // static ApplicationManager::TestAPI::TestAPI(ApplicationManager* manager) : manager_(manager) { @@ -75,8 +69,7 @@ //////////////////////////////////////////////////////////////////////////////// // ApplicationManager, public: -ApplicationManager::ApplicationManager( - bool register_mojo_url_schemes) +ApplicationManager::ApplicationManager(bool register_mojo_url_schemes) : ApplicationManager(nullptr, nullptr, register_mojo_url_schemes) {} ApplicationManager::ApplicationManager( @@ -99,15 +92,19 @@ runner.reset(); } -void ApplicationManager::ConnectToApplication( - scoped_ptr<ConnectToApplicationParams> params) { - TRACE_EVENT_INSTANT1("mojo_shell", "ApplicationManager::ConnectToApplication", +void ApplicationManager::SetInstanceQuitCallback( + base::Callback<void(const Identity&)> callback) { + instance_quit_callback_ = callback; +} + +void ApplicationManager::Connect(scoped_ptr<ConnectParams> params) { + TRACE_EVENT_INSTANT1("mojo_shell", "ApplicationManager::Connect", TRACE_EVENT_SCOPE_THREAD, "original_url", params->target().url().spec()); DCHECK(params->target().url().is_valid()); // Connect to an existing matching instance, if possible. - if (ConnectToRunningApplication(¶ms)) + if (ConnectToExistingInstance(¶ms)) return; std::string url = params->target().url().spec(); @@ -131,9 +128,7 @@ void ApplicationManager::OnApplicationInstanceError( ApplicationInstance* instance) { - // Called from ~ApplicationInstance, so we do not need to call Destroy here. const Identity identity = instance->identity(); - base::Closure on_application_end = instance->on_application_end(); // Remove the shell. auto it = identity_to_instance_.find(identity); DCHECK(it != identity_to_instance_.end()); @@ -144,12 +139,12 @@ [this, id](mojom::ApplicationManagerListener* listener) { listener->ApplicationInstanceDestroyed(id); }); - if (!on_application_end.is_null()) - on_application_end.Run(); + if (!instance_quit_callback_.is_null()) + instance_quit_callback_.Run(identity); } ApplicationInstance* ApplicationManager::GetApplicationInstance( - const Identity& identity) const { + const Identity& identity) const { const auto& it = identity_to_instance_.find(identity); return it != identity_to_instance_.end() ? it->second : nullptr; } @@ -181,9 +176,8 @@ // ApplicationManager, InterfaceFactory<mojom::ApplicationManager> // implementation: -void ApplicationManager::Create( - Connection* connection, - mojom::ApplicationManagerRequest request) { +void ApplicationManager::Create(Connection* connection, + mojom::ApplicationManagerRequest request) { bindings_.AddBinding(this, std::move(request)); } @@ -203,8 +197,7 @@ Identity target_id(url.To<GURL>(), std::string(), local_filter); mojom::ShellClientRequest request; // TODO(beng): do better than url.spec() for application name. - ApplicationInstance* instance = - CreateInstance(target_id, base::Closure(), url, &request); + ApplicationInstance* instance = CreateInstance(target_id, url, &request); instance->BindPIDReceiver(std::move(pid_receiver)); scoped_ptr<NativeRunner> runner = native_runner_factory_->Create(base::FilePath()); @@ -232,7 +225,7 @@ mojom::ShellClientRequest request; GURL url("mojo://package_manager/"); - CreateInstance(Identity(url), base::Closure(), url.spec(), &request); + CreateInstance(Identity(url), url.spec(), &request); loader->Load(url, std::move(request)); SetLoaderForURL(std::move(loader), url); @@ -240,28 +233,23 @@ ConnectToInterface(this, CreateShellIdentity(), url, &shell_resolver_); } -bool ApplicationManager::ConnectToRunningApplication( - scoped_ptr<ConnectToApplicationParams>* params) { +bool ApplicationManager::ConnectToExistingInstance( + scoped_ptr<ConnectParams>* params) { ApplicationInstance* instance = GetApplicationInstance((*params)->target()); if (!instance) return false; - - // TODO(beng): CHECK() that the target URL is already in the application - // catalog. instance->ConnectToClient(std::move(*params)); return true; } ApplicationInstance* ApplicationManager::CreateInstance( const Identity& target_id, - const base::Closure& on_application_end, const String& application_name, mojom::ShellClientRequest* request) { mojom::ShellClientPtr shell_client; *request = GetProxy(&shell_client); ApplicationInstance* instance = new ApplicationInstance( - std::move(shell_client), this, target_id, on_application_end, - application_name); + std::move(shell_client), this, target_id, application_name); DCHECK(identity_to_instance_.find(target_id) == identity_to_instance_.end()); identity_to_instance_[target_id] = instance; @@ -313,7 +301,7 @@ } void ApplicationManager::OnGotResolvedURL( - scoped_ptr<ConnectToApplicationParams> params, + scoped_ptr<ConnectParams> params, const String& resolved_url, const String& qualifier, const String& file_url, @@ -322,14 +310,13 @@ // It's possible that when this manifest request was issued, another one was // already in-progress and completed by the time this one did, and so the // requested application may already be running. - if (ConnectToRunningApplication(¶ms)) + if (ConnectToExistingInstance(¶ms)) return; Identity source = params->source(), target = params->target(); mojom::ShellClientRequest request; - ApplicationInstance* instance = CreateInstance( - params->target(), params->on_application_end(), application_name, - &request); + ApplicationInstance* instance = + CreateInstance(params->target(), application_name, &request); instance->ConnectToClient(std::move(params)); if (LoadWithLoader(target, &request))
diff --git a/mojo/shell/application_manager.h b/mojo/shell/application_manager.h index 2708c642..f09dbe3 100644 --- a/mojo/shell/application_manager.h +++ b/mojo/shell/application_manager.h
@@ -16,7 +16,7 @@ #include "mojo/services/package_manager/public/interfaces/shell_resolver.mojom.h" #include "mojo/shell/application_loader.h" #include "mojo/shell/capability_filter.h" -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" #include "mojo/shell/identity.h" #include "mojo/shell/native_runner.h" #include "mojo/shell/public/cpp/interface_factory.h" @@ -37,7 +37,6 @@ namespace shell { class ApplicationInstance; -class ShellClientFactoryConnection; class ApplicationManager : public ShellClient, public InterfaceFactory<mojom::ApplicationManager>, @@ -72,10 +71,15 @@ bool register_mojo_url_schemes); ~ApplicationManager() override; - // Loads a service if necessary and establishes a new client connection. - // Please see the comments in connect_to_application_params.h for more details - // about the parameters. - void ConnectToApplication(scoped_ptr<ConnectToApplicationParams> params); + // Provide a callback to be notified whenever an instance is destroyed. + // Typically the creator of the ApplicationManager will use this to determine + // when some set of instances it created are destroyed, so it can shut down. + void SetInstanceQuitCallback(base::Callback<void(const Identity&)> callback); + + // Completes a connection between a source and target application as defined + // by |params|, exchanging InterfaceProviders between them. If no existing + // instance of the target application is running, one will be loaded. + void Connect(scoped_ptr<ConnectParams> params); // Sets the default Loader to be used if not overridden by SetLoaderForURL(). void set_default_loader(scoped_ptr<ApplicationLoader> loader) { @@ -107,32 +111,25 @@ bool AcceptConnection(Connection* connection) override; // InterfaceFactory<mojom::ApplicationManager>: - void Create( - Connection* connection, - InterfaceRequest<mojom::ApplicationManager> request) override; + void Create(Connection* connection, + InterfaceRequest<mojom::ApplicationManager> request) override; // mojom::ApplicationManager: - void CreateInstanceForHandle( - ScopedHandle channel, - const String& url, - mojom::CapabilityFilterPtr filter, - InterfaceRequest<mojom::PIDReceiver> pid_receiver) override; - void AddListener( - mojom::ApplicationManagerListenerPtr listener) override; + void CreateInstanceForHandle(ScopedHandle channel, + const String& url, + mojom::CapabilityFilterPtr filter, + mojom::PIDReceiverRequest pid_receiver) override; + void AddListener(mojom::ApplicationManagerListenerPtr listener) override; void InitPackageManager(bool register_mojo_url_schemes); - // Takes the contents of |params| only when it returns true. - bool ConnectToRunningApplication( - scoped_ptr<ConnectToApplicationParams>* params); + // Attempt to complete the connection requested by |params| by connecting to + // an existing instance. If there is an existing instance, |params| is taken, + // and this function returns true. + bool ConnectToExistingInstance(scoped_ptr<ConnectParams>* params); - ApplicationInstance* CreateAndConnectToInstance( - scoped_ptr<ConnectToApplicationParams> params, - const std::string& application_name, - mojom::ShellClientRequest* request); ApplicationInstance* CreateInstance( const Identity& target_id, - const base::Closure& on_application_end, const String& application_name, mojom::ShellClientRequest* request); @@ -156,7 +153,7 @@ // manifest. // |base_filter| is the CapabilityFilter the requested application should be // run with, from its manifest. - void OnGotResolvedURL(scoped_ptr<ConnectToApplicationParams> params, + void OnGotResolvedURL(scoped_ptr<ConnectParams> params, const String& resolved_url, const String& qualifier, const String& file_url, @@ -192,6 +189,7 @@ WeakInterfacePtrSet<mojom::ApplicationManagerListener> listeners_; + base::Callback<void(const Identity&)> instance_quit_callback_; base::TaskRunner* task_runner_; scoped_ptr<NativeRunnerFactory> native_runner_factory_; std::vector<scoped_ptr<NativeRunner>> native_runners_; @@ -201,7 +199,7 @@ DISALLOW_COPY_AND_ASSIGN(ApplicationManager); }; -mojom::Shell::ConnectToApplicationCallback EmptyConnectCallback(); +mojom::Shell::ConnectCallback EmptyConnectCallback(); } // namespace shell } // namespace mojo
diff --git a/mojo/shell/background/background_shell.cc b/mojo/shell/background/background_shell.cc index 50eef9ce..0fdba197 100644 --- a/mojo/shell/background/background_shell.cc +++ b/mojo/shell/background/background_shell.cc
@@ -15,7 +15,7 @@ #include "mojo/shell/application_loader.h" #include "mojo/shell/application_manager.h" #include "mojo/shell/capability_filter.h" -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" #include "mojo/shell/public/cpp/shell_client.h" #include "mojo/shell/public/cpp/shell_connection.h" #include "mojo/shell/standalone/context.h" @@ -79,7 +79,7 @@ ~MojoThread() override {} void CreateShellClientRequest(base::WaitableEvent* signal, - scoped_ptr<ConnectToApplicationParams> params, + scoped_ptr<ConnectParams> params, mojom::ShellClientRequest* request) { // Only valid to call this on the background thread. DCHECK_EQ(message_loop_, base::MessageLoop::current()); @@ -91,7 +91,7 @@ url, signal, request)); context_->application_manager()->SetLoaderForURL(make_scoped_ptr(loader), url); - context_->application_manager()->ConnectToApplication(std::move(params)); + context_->application_manager()->Connect(std::move(params)); // The request is asynchronously processed. When processed // OnGotApplicationRequest() is called and we'll signal |signal|. } @@ -176,7 +176,7 @@ mojom::ShellClientRequest BackgroundShell::CreateShellClientRequest( const GURL& url) { - scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->set_target( Identity(url, std::string(), GetPermissiveCapabilityFilter())); mojom::ShellClientRequest request;
diff --git a/mojo/shell/connect_to_application_params.cc b/mojo/shell/connect_params.cc similarity index 60% rename from mojo/shell/connect_to_application_params.cc rename to mojo/shell/connect_params.cc index 481c2a63..92d1a87 100644 --- a/mojo/shell/connect_to_application_params.cc +++ b/mojo/shell/connect_params.cc
@@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" #include <utility> @@ -11,11 +11,11 @@ namespace mojo { namespace shell { -ConnectToApplicationParams::ConnectToApplicationParams() {} + ConnectParams::ConnectParams() {} -ConnectToApplicationParams::~ConnectToApplicationParams() {} + ConnectParams::~ConnectParams() {} -void ConnectToApplicationParams::SetSource(ApplicationInstance* source) { + void ConnectParams::SetSource(ApplicationInstance* source) { if (!source) { source_ = Identity(); return; @@ -24,7 +24,7 @@ source_ = source->identity(); } -void ConnectToApplicationParams::SetTargetURL(const GURL& target_url) { +void ConnectParams::SetTargetURL(const GURL& target_url) { target_ = Identity(target_url, target_.qualifier(), target_.filter()); }
diff --git a/mojo/shell/connect_to_application_params.h b/mojo/shell/connect_params.h similarity index 72% rename from mojo/shell/connect_to_application_params.h rename to mojo/shell/connect_params.h index 8bfa21d..172df1f 100644 --- a/mojo/shell/connect_to_application_params.h +++ b/mojo/shell/connect_params.h
@@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef MOJO_SHELL_CONNECT_TO_APPLICATION_PARAMS_H_ -#define MOJO_SHELL_CONNECT_TO_APPLICATION_PARAMS_H_ +#ifndef MOJO_SHELL_CONNECT_PARAMS_H_ +#define MOJO_SHELL_CONNECT_PARAMS_H_ #include <string> #include <utility> @@ -23,10 +23,10 @@ // This class represents a request for the application manager to connect to an // application. -class ConnectToApplicationParams { +class ConnectParams { public: - ConnectToApplicationParams(); - ~ConnectToApplicationParams(); + ConnectParams(); + ~ConnectParams(); // Sets |source_|. If |source| is null, |source_| is reset. void SetSource(ApplicationInstance* source); @@ -53,19 +53,10 @@ return std::move(local_interfaces_); } - void set_on_application_end(const base::Closure& value) { - on_application_end_ = value; - } - const base::Closure& on_application_end() const { - return on_application_end_; - } - - void set_connect_callback( - const shell::mojom::Shell::ConnectToApplicationCallback& value) { + void set_connect_callback(const shell::mojom::Shell::ConnectCallback& value) { connect_callback_ = value; } - const shell::mojom::Shell::ConnectToApplicationCallback& - connect_callback() const { + const shell::mojom::Shell::ConnectCallback& connect_callback() const { return connect_callback_; } @@ -78,13 +69,12 @@ shell::mojom::InterfaceProviderRequest remote_interfaces_; shell::mojom::InterfaceProviderPtr local_interfaces_; - base::Closure on_application_end_; - shell::mojom::Shell::ConnectToApplicationCallback connect_callback_; + shell::mojom::Shell::ConnectCallback connect_callback_; - DISALLOW_COPY_AND_ASSIGN(ConnectToApplicationParams); + DISALLOW_COPY_AND_ASSIGN(ConnectParams); }; } // namespace shell } // namespace mojo -#endif // MOJO_SHELL_CONNECT_TO_APPLICATION_PARAMS_H_ +#endif // MOJO_SHELL_CONNECT_PARAMS_H_
diff --git a/mojo/shell/connect_util.cc b/mojo/shell/connect_util.cc index f435635..ecc1cd37 100644 --- a/mojo/shell/connect_util.cc +++ b/mojo/shell/connect_util.cc
@@ -8,7 +8,7 @@ #include "mojo/shell/application_manager.h" #include "mojo/shell/capability_filter.h" -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" namespace mojo { namespace shell { @@ -19,11 +19,11 @@ const Identity& target, const std::string& interface_name) { shell::mojom::InterfaceProviderPtr remote_interfaces; - scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->set_source(source); params->set_target(target); params->set_remote_interfaces(GetProxy(&remote_interfaces)); - application_manager->ConnectToApplication(std::move(params)); + application_manager->Connect(std::move(params)); MessagePipe pipe; remote_interfaces->GetInterface(interface_name, std::move(pipe.handle1)); return std::move(pipe.handle0);
diff --git a/mojo/shell/identity.cc b/mojo/shell/identity.cc index c0e12fb..2c05b3f 100644 --- a/mojo/shell/identity.cc +++ b/mojo/shell/identity.cc
@@ -53,6 +53,11 @@ return qualifier_ < other.qualifier_; } +bool Identity::operator==(const Identity& other) const { + return other.url_ == url_ && other.qualifier_ == qualifier_ && + other.filter_ == filter_; +} + Identity CreateShellIdentity() { return Identity(GURL("mojo://shell/"), std::string(), GetPermissiveCapabilityFilter());
diff --git a/mojo/shell/identity.h b/mojo/shell/identity.h index e7a2d27..e5a349b6 100644 --- a/mojo/shell/identity.h +++ b/mojo/shell/identity.h
@@ -30,6 +30,7 @@ bool operator<(const Identity& other) const; bool is_null() const { return url_.is_empty(); } + bool operator==(const Identity& other) const; const GURL& url() const { return url_; } const std::string& qualifier() const { return qualifier_; }
diff --git a/mojo/shell/public/cpp/BUILD.gn b/mojo/shell/public/cpp/BUILD.gn index eeea0d1..5e814fb 100644 --- a/mojo/shell/public/cpp/BUILD.gn +++ b/mojo/shell/public/cpp/BUILD.gn
@@ -43,7 +43,6 @@ deps = [ "//base:i18n", "//mojo/common", - "//mojo/converters/network", "//mojo/environment:chromium", "//mojo/message_pump", ] @@ -53,6 +52,7 @@ "//mojo/public/cpp/bindings", "//mojo/public/cpp/system", "//mojo/shell/public/interfaces", + "//url", ] }
diff --git a/mojo/shell/public/cpp/lib/connection_impl.cc b/mojo/shell/public/cpp/lib/connection_impl.cc index d0faa51..47f9b9f5 100644 --- a/mojo/shell/public/cpp/lib/connection_impl.cc +++ b/mojo/shell/public/cpp/lib/connection_impl.cc
@@ -46,8 +46,7 @@ ConnectionImpl::~ConnectionImpl() {} -shell::mojom::Shell::ConnectToApplicationCallback -ConnectionImpl::GetConnectToApplicationCallback() { +shell::mojom::Shell::ConnectCallback ConnectionImpl::GetConnectCallback() { return base::Bind(&ConnectionImpl::OnGotInstanceID, weak_factory_.GetWeakPtr()); }
diff --git a/mojo/shell/public/cpp/lib/connection_impl.h b/mojo/shell/public/cpp/lib/connection_impl.h index 0cc2247..4fc34e7 100644 --- a/mojo/shell/public/cpp/lib/connection_impl.h +++ b/mojo/shell/public/cpp/lib/connection_impl.h
@@ -35,8 +35,7 @@ const std::set<std::string>& allowed_interfaces); ~ConnectionImpl() override; - shell::mojom::Shell::ConnectToApplicationCallback - GetConnectToApplicationCallback(); + shell::mojom::Shell::ConnectCallback GetConnectCallback(); private: // Connection:
diff --git a/mojo/shell/public/cpp/lib/shell_connection.cc b/mojo/shell/public/cpp/lib/shell_connection.cc index acecb48a..f9fca2f 100644 --- a/mojo/shell/public/cpp/lib/shell_connection.cc +++ b/mojo/shell/public/cpp/lib/shell_connection.cc
@@ -8,7 +8,6 @@ #include "base/bind.h" #include "base/logging.h" #include "base/message_loop/message_loop.h" -#include "mojo/converters/network/network_type_converters.h" #include "mojo/public/cpp/bindings/interface_ptr.h" #include "mojo/shell/public/cpp/lib/connection_impl.h" #include "mojo/shell/public/cpp/shell_client.h" @@ -87,10 +86,7 @@ ShellConnection::ConnectParams::ConnectParams(const std::string& url) - : ConnectParams(URLRequest::From(url)) {} -ShellConnection::ConnectParams::ConnectParams(URLRequestPtr request) - : request_(std::move(request)), - filter_(shell::mojom::CapabilityFilter::New()) { + : url_(url), filter_(shell::mojom::CapabilityFilter::New()) { filter_->filter.SetToEmpty(); } ShellConnection::ConnectParams::~ConnectParams() {} @@ -136,8 +132,7 @@ if (!shell_) return nullptr; DCHECK(params); - URLRequestPtr request = params->TakeRequest(); - std::string application_url = request->url.To<std::string>(); + std::string application_url = params->url().spec(); // We allow all interfaces on outgoing connections since we are presumably in // a position to know who we're talking to. // TODO(beng): is this a valid assumption or do we need to figure some way to @@ -154,11 +149,11 @@ application_url, application_url, shell::mojom::Shell::kInvalidApplicationID, std::move(remote_interfaces), std::move(local_request), allowed)); - shell_->ConnectToApplication(std::move(request), - std::move(remote_request), - std::move(local_interfaces), - params->TakeFilter(), - registry->GetConnectToApplicationCallback()); + shell_->Connect(application_url, + std::move(remote_request), + std::move(local_interfaces), + params->TakeFilter(), + registry->GetConnectCallback()); return std::move(registry); }
diff --git a/mojo/shell/public/cpp/shell.h b/mojo/shell/public/cpp/shell.h index 7775cbb..3b12cc9 100644 --- a/mojo/shell/public/cpp/shell.h +++ b/mojo/shell/public/cpp/shell.h
@@ -8,6 +8,7 @@ #include "mojo/shell/public/cpp/connection.h" #include "mojo/shell/public/interfaces/shell.mojom.h" #include "mojo/shell/public/interfaces/shell_client.mojom.h" +#include "url/gurl.h" namespace mojo { @@ -37,10 +38,9 @@ class ConnectParams { public: explicit ConnectParams(const std::string& url); - explicit ConnectParams(URLRequestPtr request); ~ConnectParams(); - URLRequestPtr TakeRequest() { return std::move(request_); } + const GURL& url() { return url_; } shell::mojom::CapabilityFilterPtr TakeFilter() { return std::move(filter_); } @@ -49,7 +49,7 @@ } private: - URLRequestPtr request_; + GURL url_; shell::mojom::CapabilityFilterPtr filter_; DISALLOW_COPY_AND_ASSIGN(ConnectParams);
diff --git a/mojo/shell/public/interfaces/BUILD.gn b/mojo/shell/public/interfaces/BUILD.gn index d014121..39c678e 100644 --- a/mojo/shell/public/interfaces/BUILD.gn +++ b/mojo/shell/public/interfaces/BUILD.gn
@@ -15,8 +15,4 @@ ] import_dirs = [ "//mojo/services" ] - - deps = [ - "//mojo/services/network/public/interfaces", - ] }
diff --git a/mojo/shell/public/interfaces/shell.mojom b/mojo/shell/public/interfaces/shell.mojom index c512606..ed3cbd02 100644 --- a/mojo/shell/public/interfaces/shell.mojom +++ b/mojo/shell/public/interfaces/shell.mojom
@@ -5,7 +5,6 @@ module mojo.shell.mojom; import "mojo/shell/public/interfaces/interface_provider.mojom"; -import "network/public/interfaces/url_loader.mojom"; // Specifies a whitelist of applications and services an application can connect // to. Connections to applications not explicitly specified here as a key are @@ -27,11 +26,8 @@ const uint32 kInvalidApplicationID = 0; // Establishes a connection with another application ("target application") - // (located at |request->url|) through which the calling application and the + // (located at |url|) through which the calling application and the // target application may request services from one another. - // |application_url| is a URLRequest in case this is called for an HTTP - // navigation, in which case HTTP specific information like POST data, - // referrer header etc... needed. // // If the calling application would like to request services from the target // application, it should pass a valid interface request in the |services| @@ -56,10 +52,10 @@ // |filter| is a whitelist of application URLs and services that the target // application is permitted to connect to. See documentation for // CapabilityFilter above. - ConnectToApplication(mojo.URLRequest application_url, - InterfaceProvider&? remote_interfaces, - InterfaceProvider? local_interfaces, - CapabilityFilter filter) => (uint32 application_id); + Connect(string url, + InterfaceProvider&? remote_interfaces, + InterfaceProvider? local_interfaces, + CapabilityFilter filter) => (uint32 application_id); // When there are no more instantiated services in an application, it should // start its shutdown process by calling this method. Additionally, it should
diff --git a/mojo/shell/runner/child/BUILD.gn b/mojo/shell/runner/child/BUILD.gn index 55779bf..b243df8 100644 --- a/mojo/shell/runner/child/BUILD.gn +++ b/mojo/shell/runner/child/BUILD.gn
@@ -88,7 +88,6 @@ "//base", "//base/test:test_config", "//mojo/common:common_base", - "//mojo/converters/network:network", "//mojo/shell/public/cpp:sources", "//mojo/shell/public/cpp:test_support", ]
diff --git a/mojo/shell/runner/child/native_apptest.cc b/mojo/shell/runner/child/native_apptest.cc index 77f69651..7368fbc 100644 --- a/mojo/shell/runner/child/native_apptest.cc +++ b/mojo/shell/runner/child/native_apptest.cc
@@ -4,7 +4,6 @@ #include "base/bind.h" #include "base/macros.h" -#include "mojo/converters/network/network_type_converters.h" #include "mojo/shell/public/cpp/application_test_base.h" #include "mojo/shell/runner/child/test_native_service.mojom.h"
diff --git a/mojo/shell/standalone/context.cc b/mojo/shell/standalone/context.cc index b33a27f..a04d259 100644 --- a/mojo/shell/standalone/context.cc +++ b/mojo/shell/standalone/context.cc
@@ -33,7 +33,7 @@ #include "mojo/services/tracing/public/cpp/tracing_impl.h" #include "mojo/services/tracing/public/interfaces/tracing.mojom.h" #include "mojo/shell/application_loader.h" -#include "mojo/shell/connect_to_application_params.h" +#include "mojo/shell/connect_params.h" #include "mojo/shell/runner/host/command_line_switch.h" #include "mojo/shell/runner/host/in_process_native_runner.h" #include "mojo/shell/runner/host/out_of_process_native_runner.h" @@ -90,6 +90,11 @@ return thread; } +void OnInstanceQuit(const GURL& url, const Identity& identity) { + if (url == identity.url()) + base::MessageLoop::current()->QuitWhenIdle(); +} + } // namespace Context::Context() @@ -143,20 +148,20 @@ blocking_pool_.get(), command_line_switches_)); } application_manager_.reset(new ApplicationManager( - std::move(runner_factory), blocking_pool_.get(), true)); + std::move(runner_factory), blocking_pool_.get(), true)); shell::mojom::InterfaceProviderPtr tracing_remote_interfaces; shell::mojom::InterfaceProviderPtr tracing_local_interfaces; new TracingInterfaceProvider(&tracer_, GetProxy(&tracing_local_interfaces)); - scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->set_source(Identity(GURL("mojo:shell"), std::string(), GetPermissiveCapabilityFilter())); params->set_target(Identity(GURL("mojo:tracing"), std::string(), GetPermissiveCapabilityFilter())); params->set_remote_interfaces(GetProxy(&tracing_remote_interfaces)); params->set_local_interfaces(std::move(tracing_local_interfaces)); - application_manager_->ConnectToApplication(std::move(params)); + application_manager_->Connect(std::move(params)); if (command_line.HasSwitch(tracing::kTraceStartup)) { tracing::TraceCollectorPtr coordinator; @@ -202,50 +207,31 @@ base::MessageLoop::current()->QuitWhenIdle(); } -void Context::Run(const GURL& url) { - DCHECK(app_complete_callback_.is_null()); - shell::mojom::InterfaceProviderPtr remote_interfaces; - shell::mojom::InterfaceProviderPtr local_interfaces; - - app_urls_.insert(url); - - scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams); - params->set_target( - Identity(url, std::string(), GetPermissiveCapabilityFilter())); - params->set_remote_interfaces(GetProxy(&remote_interfaces)); - params->set_local_interfaces(std::move(local_interfaces)); - params->set_on_application_end( - base::Bind(&Context::OnApplicationEnd, base::Unretained(this), url)); - application_manager_->ConnectToApplication(std::move(params)); -} - -void Context::RunCommandLineApplication(const base::Closure& callback) { - DCHECK(app_urls_.empty()); - DCHECK(app_complete_callback_.is_null()); +void Context::RunCommandLineApplication() { base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); base::CommandLine::StringVector args = command_line->GetArgs(); for (size_t i = 0; i < args.size(); ++i) { GURL possible_app(args[i]); if (possible_app.SchemeIs("mojo")) { Run(possible_app); - app_complete_callback_ = callback; break; } } } -void Context::OnApplicationEnd(const GURL& url) { - if (app_urls_.find(url) != app_urls_.end()) { - app_urls_.erase(url); - if (app_urls_.empty() && base::MessageLoop::current()->is_running()) { - DCHECK_EQ(base::MessageLoop::current()->task_runner(), shell_runner_); - if (app_complete_callback_.is_null()) { - base::MessageLoop::current()->QuitWhenIdle(); - } else { - app_complete_callback_.Run(); - } - } - } +void Context::Run(const GURL& url) { + application_manager_->SetInstanceQuitCallback( + base::Bind(&OnInstanceQuit, url)); + + shell::mojom::InterfaceProviderPtr remote_interfaces; + shell::mojom::InterfaceProviderPtr local_interfaces; + + scoped_ptr<ConnectParams> params(new ConnectParams); + params->set_target( + Identity(url, std::string(), GetPermissiveCapabilityFilter())); + params->set_remote_interfaces(GetProxy(&remote_interfaces)); + params->set_local_interfaces(std::move(local_interfaces)); + application_manager_->Connect(std::move(params)); } } // namespace shell
diff --git a/mojo/shell/standalone/context.h b/mojo/shell/standalone/context.h index a659009..19df61a 100644 --- a/mojo/shell/standalone/context.h +++ b/mojo/shell/standalone/context.h
@@ -5,8 +5,6 @@ #ifndef MOJO_SHELL_STANDALONE_CONTEXT_H_ #define MOJO_SHELL_STANDALONE_CONTEXT_H_ -#include <set> -#include <string> #include <vector> #include "base/callback_forward.h" @@ -28,7 +26,6 @@ namespace mojo { namespace shell { struct CommandLineSwitch; -class NativeApplicationLoader; // The "global" context for the shell's main process. class Context : public edk::ProcessDelegate { @@ -50,28 +47,19 @@ // If Init() was called and succeeded, this must be called before destruction. void Shutdown(); - // NOTE: call either Run() or RunCommandLineApplication(), but not both. - - // Runs the app specified by |url|. - void Run(const GURL& url); - - // Run the application specified on the commandline. When the app finishes, - // |callback| is run if not null, otherwise the message loop is quit. - void RunCommandLineApplication(const base::Closure& callback); + // Run the application specified on the command line. + void RunCommandLineApplication(); ApplicationManager* application_manager() { return application_manager_.get(); } private: - class NativeViewportApplicationLoader; - - // ProcessDelegate implementation. + // edk::ProcessDelegate: void OnShutdownComplete() override; - void OnApplicationEnd(const GURL& url); - - std::set<GURL> app_urls_; + // Runs the app specified by |url|. + void Run(const GURL& url); scoped_refptr<base::SingleThreadTaskRunner> shell_runner_; scoped_ptr<base::Thread> io_thread_; @@ -81,7 +69,6 @@ // that needs the IO thread to destruct cleanly. Tracer tracer_; scoped_ptr<ApplicationManager> application_manager_; - base::Closure app_complete_callback_; base::Time main_entry_time_; std::vector<CommandLineSwitch> command_line_switches_;
diff --git a/mojo/shell/standalone/desktop/launcher_process.cc b/mojo/shell/standalone/desktop/launcher_process.cc index cdbb2f9b..efb6fd6f7 100644 --- a/mojo/shell/standalone/desktop/launcher_process.cc +++ b/mojo/shell/standalone/desktop/launcher_process.cc
@@ -5,7 +5,6 @@ #include <stdio.h> #include <string.h> -#include <algorithm> #include <iostream> #include "base/base_switches.h" @@ -24,7 +23,7 @@ namespace mojo { namespace shell { -int LauncherProcessMain(const GURL& mojo_url, const base::Closure& callback) { +int LauncherProcessMain() { #if !defined(OFFICIAL_BUILD) base::debug::EnableInProcessStackDumping(); #endif @@ -44,16 +43,11 @@ CHECK(base::i18n::InitializeICU()); shell_context.Init(shell_dir); - if (mojo_url.is_empty()) { - message_loop.PostTask( - FROM_HERE, - base::Bind(&Context::RunCommandLineApplication, - base::Unretained(&shell_context), base::Closure())); - } else { - message_loop.PostTask( - FROM_HERE, base::Bind(&mojo::shell::Context::Run, - base::Unretained(&shell_context), mojo_url)); - } + message_loop.PostTask( + FROM_HERE, + base::Bind(&Context::RunCommandLineApplication, + base::Unretained(&shell_context))); + message_loop.Run(); // Must be called before |message_loop| is destroyed.
diff --git a/mojo/shell/standalone/desktop/launcher_process.h b/mojo/shell/standalone/desktop/launcher_process.h index 351cc79..8c32775 100644 --- a/mojo/shell/standalone/desktop/launcher_process.h +++ b/mojo/shell/standalone/desktop/launcher_process.h
@@ -13,8 +13,7 @@ namespace shell { // Main method for the launcher process. -// See commit in main_helper.h for explanation of the parameters. -int LauncherProcessMain(const GURL& mojo_url, const base::Closure& callback); +int LauncherProcessMain(); } // namespace shell } // namespace mojo
diff --git a/mojo/shell/standalone/desktop/main.cc b/mojo/shell/standalone/desktop/main.cc index d25f03bc..a6b3878 100644 --- a/mojo/shell/standalone/desktop/main.cc +++ b/mojo/shell/standalone/desktop/main.cc
@@ -5,5 +5,5 @@ #include "mojo/shell/standalone/desktop/main_helper.h" int main(int argc, char** argv) { - return mojo::shell::StandaloneShellMain(argc, argv, GURL(), base::Closure()); + return mojo::shell::StandaloneShellMain(argc, argv); }
diff --git a/mojo/shell/standalone/desktop/main_helper.cc b/mojo/shell/standalone/desktop/main_helper.cc index 062e036..abbf6c28 100644 --- a/mojo/shell/standalone/desktop/main_helper.cc +++ b/mojo/shell/standalone/desktop/main_helper.cc
@@ -28,10 +28,7 @@ namespace mojo { namespace shell { -int StandaloneShellMain(int argc, - char** argv, - const GURL& mojo_url, - const base::Closure& callback) { +int StandaloneShellMain(int argc, char** argv) { base::CommandLine::Init(argc, argv); const base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); @@ -47,7 +44,7 @@ if (command_line.HasSwitch(switches::kChildProcess)) return ChildProcessMain(); - return LauncherProcessMain(mojo_url, callback); + return LauncherProcessMain(); } } // namespace shell
diff --git a/mojo/shell/standalone/desktop/main_helper.h b/mojo/shell/standalone/desktop/main_helper.h index 4872707..644f19e 100644 --- a/mojo/shell/standalone/desktop/main_helper.h +++ b/mojo/shell/standalone/desktop/main_helper.h
@@ -12,13 +12,7 @@ namespace shell { // Helper method to start Mojo standalone shell code. -// If |mojo_url| is not empty, the given mojo application is started. Otherwise, -// an application must have been specified on the command line and it is run. -// |callback| is only called in the later case. -int StandaloneShellMain(int argc, - char** argv, - const GURL& mojo_url, - const base::Closure& callback); +int StandaloneShellMain(int argc, char** argv); } // namespace shell } // namespace mojo
diff --git a/mojo/shell/tests/BUILD.gn b/mojo/shell/tests/BUILD.gn index d50cf5ec..5ef1e7b 100644 --- a/mojo/shell/tests/BUILD.gn +++ b/mojo/shell/tests/BUILD.gn
@@ -78,7 +78,6 @@ "//base", "//base/test:test_config", "//mojo/common:common_base", - "//mojo/converters/network", "//mojo/shell/public/cpp:sources", "//mojo/shell/public/cpp:test_support", "//mojo/shell/public/interfaces", @@ -148,7 +147,6 @@ "//base:base_static", "//build/config/sanitizers:deps", "//mojo/common:common_base", - "//mojo/converters/network", "//mojo/edk/system", "//mojo/shell/public/cpp", "//mojo/shell/public/interfaces", @@ -179,7 +177,6 @@ "//base", "//build/config/sanitizers:deps", "//mojo/common:common_base", - "//mojo/converters/network", "//mojo/shell/public/cpp", "//mojo/shell/runner/child:test_native_main", ]
diff --git a/mojo/shell/tests/application_manager_apptest.cc b/mojo/shell/tests/application_manager_apptest.cc index 80be147..0c7190d 100644 --- a/mojo/shell/tests/application_manager_apptest.cc +++ b/mojo/shell/tests/application_manager_apptest.cc
@@ -11,7 +11,6 @@ #include "base/macros.h" #include "base/message_loop/message_loop.h" #include "base/process/process_handle.h" -#include "mojo/converters/network/network_type_converters.h" #include "mojo/public/cpp/bindings/weak_binding_set.h" #include "mojo/shell/public/cpp/application_test_base.h" #include "mojo/shell/public/cpp/interface_factory.h"
diff --git a/mojo/shell/tests/application_manager_apptest_driver.cc b/mojo/shell/tests/application_manager_apptest_driver.cc index 09e5999..39fe9ed 100644 --- a/mojo/shell/tests/application_manager_apptest_driver.cc +++ b/mojo/shell/tests/application_manager_apptest_driver.cc
@@ -18,7 +18,6 @@ #include "base/path_service.h" #include "base/process/process.h" #include "base/thread_task_runner_handle.h" -#include "mojo/converters/network/network_type_converters.h" #include "mojo/edk/embedder/embedder.h" #include "mojo/edk/embedder/platform_channel_pair.h" #include "mojo/edk/embedder/scoped_platform_handle.h"
diff --git a/mojo/shell/tests/application_manager_unittest.cc b/mojo/shell/tests/application_manager_unittest.cc index 3ab66877..ac4133d9 100644 --- a/mojo/shell/tests/application_manager_unittest.cc +++ b/mojo/shell/tests/application_manager_unittest.cc
@@ -37,9 +37,13 @@ int num_loader_deletes; }; -void QuitClosure(bool* value) { - *value = true; - base::MessageLoop::current()->QuitWhenIdle(); +void QuitClosure(const Identity& expected, + bool* value, + const Identity& actual) { + if (expected == actual) { + *value = true; + base::MessageLoop::current()->QuitWhenIdle(); + } } class TestServiceImpl : public TestService { @@ -423,14 +427,13 @@ void ConnectToInterface(const GURL& url, InterfacePtr<Interface>* ptr) { base::RunLoop loop; mojom::InterfaceProviderPtr remote_interfaces; - scoped_ptr<ConnectToApplicationParams> params( - new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->set_source(CreateShellIdentity()); params->set_target(Identity(url)); params->set_remote_interfaces(GetProxy(&remote_interfaces)); params->set_connect_callback( base::Bind(&OnConnect, base::Unretained(&loop))); - application_manager_->ConnectToApplication(std::move(params)); + application_manager_->Connect(std::move(params)); loop.Run(); mojo::GetInterface(remote_interfaces.get(), ptr); @@ -596,11 +599,11 @@ scoped_ptr<ApplicationLoader>(loader), GURL("test:test")); bool called = false; - scoped_ptr<ConnectToApplicationParams> params(new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->SetTargetURL(GURL("test:test")); - params->set_on_application_end( - base::Bind(&QuitClosure, base::Unretained(&called))); - application_manager_->ConnectToApplication(std::move(params)); + application_manager_->SetInstanceQuitCallback( + base::Bind(&QuitClosure, params->target(), &called)); + application_manager_->Connect(std::move(params)); loop_.Run(); EXPECT_TRUE(called); }
diff --git a/mojo/shell/tests/capability_filter_test.cc b/mojo/shell/tests/capability_filter_test.cc index 4ed390a3..9bce8923 100644 --- a/mojo/shell/tests/capability_filter_test.cc +++ b/mojo/shell/tests/capability_filter_test.cc
@@ -281,6 +281,9 @@ void CapabilityFilterTest::SetUp() { application_manager_.reset(new ApplicationManager(true)); + application_manager_->SetInstanceQuitCallback( + base::Bind(&CapabilityFilterTest::OnInstanceQuit, + base::Unretained(this))); CreateLoader<ServiceApplication>("test:service"); CreateLoader<ServiceApplication>("test:service2"); } @@ -323,13 +326,12 @@ // Adding it to the CapabilityFilter would interfere with the test. shell::mojom::InterfaceProviderPtr local_interfaces; new InterfaceProviderImpl(GetProxy(&local_interfaces), validator_); - scoped_ptr<ConnectToApplicationParams> params( - new ConnectToApplicationParams); + scoped_ptr<ConnectParams> params(new ConnectParams); params->set_target(Identity(GURL(url), std::string(), filter)); params->set_remote_interfaces(GetProxy(&remote_interfaces)); params->set_local_interfaces(std::move(local_interfaces)); - params->set_on_application_end(base::MessageLoop::QuitWhenIdleClosure()); - application_manager_->ConnectToApplication(std::move(params)); + quit_identities_.insert(params->target()); + application_manager_->Connect(std::move(params)); } void CapabilityFilterTest::InitValidator( @@ -346,6 +348,14 @@ validator_->PrintUnmetExpectations(); } +void CapabilityFilterTest::OnInstanceQuit(const Identity& identity) { + auto it = quit_identities_.find(identity); + if (it != quit_identities_.end()) + quit_identities_.erase(it); + if (quit_identities_.empty()) + base::MessageLoop::current()->QuitWhenIdle(); +} + } // namespace test } // namespace shell } // namespace mojo
diff --git a/mojo/shell/tests/capability_filter_test.h b/mojo/shell/tests/capability_filter_test.h index 3e9baa80..0efc62c1 100644 --- a/mojo/shell/tests/capability_filter_test.h +++ b/mojo/shell/tests/capability_filter_test.h
@@ -89,6 +89,8 @@ void InitValidator(const std::set<std::string>& expectations); void RunTest(); + void OnInstanceQuit(const Identity& identity); + template<class T> scoped_ptr<ShellClient> CreateShellClient() { return scoped_ptr<ShellClient>(new T); @@ -98,6 +100,7 @@ base::MessageLoop loop_; scoped_ptr<ApplicationManager> application_manager_; ConnectionValidator* validator_; + std::set<Identity> quit_identities_; DISALLOW_COPY_AND_ASSIGN(CapabilityFilterTest); };
diff --git a/net/tools/flip_server/acceptor_thread.cc b/net/tools/flip_server/acceptor_thread.cc index 4e78663..74839c4 100644 --- a/net/tools/flip_server/acceptor_thread.cc +++ b/net/tools/flip_server/acceptor_thread.cc
@@ -6,7 +6,6 @@ #include <errno.h> #include <netinet/in.h> -#include <netinet/tcp.h> // For TCP_NODELAY #include <string.h> // For strerror #include <sys/socket.h> #include <sys/types.h> @@ -17,6 +16,7 @@ #include "net/tools/flip_server/flip_config.h" #include "net/tools/flip_server/sm_connection.h" #include "net/tools/flip_server/spdy_ssl.h" +#include "net/tools/flip_server/tcp_socket_util.h" #include "openssl/err.h" #include "openssl/ssl.h" @@ -82,17 +82,8 @@ void SMAcceptorThread::HandleConnection(int server_fd, struct sockaddr_in* remote_addr) { - int on = 1; - int rc; if (acceptor_->disable_nagle_) { - rc = setsockopt(server_fd, - IPPROTO_TCP, - TCP_NODELAY, - reinterpret_cast<char*>(&on), - sizeof(on)); - if (rc < 0) { - close(server_fd); - LOG(ERROR) << "setsockopt() failed fd=" << server_fd; + if (!SetDisableNagle(server_fd)) { return; } }
diff --git a/net/tools/flip_server/tcp_socket_util.h b/net/tools/flip_server/tcp_socket_util.h index 2a090928..5b09c3e 100644 --- a/net/tools/flip_server/tcp_socket_util.h +++ b/net/tools/flip_server/tcp_socket_util.h
@@ -9,6 +9,12 @@ namespace net { +// This function disables buffering in the kernel. By default, TCP sockets +// will wait up to 200ms for more data to complete a packet before transmitting. +// After calling this function, the kernel will not wait. See TCP_NODELAY in +// `man 7 tcp`. +int SetDisableNagle(int fd); + // Summary: // creates a socket for listening, and bind()s and listen()s it. // Args:
diff --git a/remoting/host/security_key/gnubby_extension.cc b/remoting/host/security_key/gnubby_extension.cc index d225f310..156ffac 100644 --- a/remoting/host/security_key/gnubby_extension.cc +++ b/remoting/host/security_key/gnubby_extension.cc
@@ -8,7 +8,7 @@ namespace { // TODO(joedow): Update this once clients support sending a gnubby capabililty. -// Tracked via: crbug.com/585835 +// Tracked via: crbug.com/587485 const char kCapability[] = ""; }
diff --git a/remoting/remoting_webapp_files.gypi b/remoting/remoting_webapp_files.gypi index cab6a2a2..4a98cbb 100644 --- a/remoting/remoting_webapp_files.gypi +++ b/remoting/remoting_webapp_files.gypi
@@ -113,6 +113,7 @@ 'webapp/crd/js/combined_host_list_api_unittest.js', 'webapp/crd/js/gcd_client_unittest.js', 'webapp/crd/js/gcd_client_with_mock_xhr_unittest.js', + "webapp/crd/js/gnubby_auth_handler_unittest.js", 'webapp/crd/js/host_controller_unittest.js', 'webapp/crd/js/host_daemon_facade_unittest.js', 'webapp/crd/js/host_table_entry_unittest.js',
diff --git a/remoting/webapp/base/js/client_session.js b/remoting/webapp/base/js/client_session.js index 8fbbdce..882a763 100644 --- a/remoting/webapp/base/js/client_session.js +++ b/remoting/webapp/base/js/client_session.js
@@ -261,6 +261,9 @@ // Indicates desktop shape support. DESKTOP_SHAPE: 'desktopShape', + + // Indicates whether the client supports security key request forwarding. + SECURITY_KEY: 'securityKey', }; /**
diff --git a/remoting/webapp/crd/js/cast_extension_handler.js b/remoting/webapp/crd/js/cast_extension_handler.js index 72cac12..cf01bed 100644 --- a/remoting/webapp/crd/js/cast_extension_handler.js +++ b/remoting/webapp/crd/js/cast_extension_handler.js
@@ -44,7 +44,7 @@ /** @private {string} */ remoting.CastExtensionHandler.EXTENSION_TYPE = 'cast_message'; -/** @return {Array<string>} */ +/** @override @return {Array<string>} */ remoting.CastExtensionHandler.prototype.getExtensionTypes = function() { return [remoting.CastExtensionHandler.EXTENSION_TYPE]; };
diff --git a/remoting/webapp/crd/js/desktop_remoting_activity.js b/remoting/webapp/crd/js/desktop_remoting_activity.js index cf516a8..a5a39c8 100644 --- a/remoting/webapp/crd/js/desktop_remoting_activity.js +++ b/remoting/webapp/crd/js/desktop_remoting_activity.js
@@ -16,20 +16,23 @@ * * @param {remoting.Activity} parentActivity * @param {remoting.SessionLogger} logger + * @param {Array<string>=} opt_additionalCapabilities * @implements {remoting.ClientSession.EventHandler} * @implements {base.Disposable} * @constructor */ -remoting.DesktopRemotingActivity = function(parentActivity, logger) { +remoting.DesktopRemotingActivity = function( + parentActivity, logger, opt_additionalCapabilities) { /** @private */ this.parentActivity_ = parentActivity; /** @private {remoting.DesktopConnectedView} */ this.connectedView_ = null; + opt_additionalCapabilities = opt_additionalCapabilities || []; /** @private */ this.sessionFactory_ = new remoting.ClientSessionFactory( document.querySelector('#client-container .client-plugin-container'), - [/* No special capabilities required. */]); + opt_additionalCapabilities); /** @private {remoting.ClientSession} */ this.session_ = null;
diff --git a/remoting/webapp/crd/js/gnubby_auth_handler.js b/remoting/webapp/crd/js/gnubby_auth_handler.js index 724bbdc0..a8cb834 100644 --- a/remoting/webapp/crd/js/gnubby_auth_handler.js +++ b/remoting/webapp/crd/js/gnubby_auth_handler.js
@@ -20,12 +20,71 @@ remoting.GnubbyAuthHandler = function() { /** @private {?function(string,string)} */ this.sendMessageToHostCallback_ = null; + + /** @private {string} */ + this.gnubbyExtensionId_ = ''; + + /** @private {Promise} */ + this.gnubbyExtensionPromise_ = null; }; -/** @private {string} */ +/** @private @const {string} */ remoting.GnubbyAuthHandler.EXTENSION_TYPE = 'gnubby-auth'; -/** @return {Array<string>} */ +/** @private @const {string} */ +remoting.GnubbyAuthHandler.GNUBBY_DEV_EXTENSION_ID = + 'dlfcjilkjfhdnfiecknlnddkmmiofjbg'; + +/** @private @const {string} */ +remoting.GnubbyAuthHandler.GNUBBY_STABLE_EXTENSION_ID = + 'beknehfpfkghjoafdifaflglpjkojoco'; + +/** + * Determines whether any supported Gnubby extensions are installed. + * + * @return {Promise<boolean>} Promise that resolves after we have either found + * an extension or checked for the known extensions IDs without success. + * Returns true if an applicable gnubby extension was found. + */ +remoting.GnubbyAuthHandler.prototype.isGnubbyExtensionInstalled = function() { + if (this.gnubbyExtensionPromise_) { + return this.gnubbyExtensionPromise_; + } + + var findGnubbyExtension = function(extensionId, resolve, reject) { + var message_callback = function(response) { + if (response) { + this.gnubbyExtensionId_ = extensionId; + resolve(true); + } else { + reject(); + } + }.bind(this) + + chrome.runtime.sendMessage(extensionId, "HELLO", message_callback); + } + + var findDevGnubbyExtension = findGnubbyExtension.bind(this, + remoting.GnubbyAuthHandler.GNUBBY_DEV_EXTENSION_ID) + + var findStableGnubbyExtension = findGnubbyExtension.bind(this, + remoting.GnubbyAuthHandler.GNUBBY_STABLE_EXTENSION_ID) + + this.gnubbyExtensionPromise_ = new Promise( + findStableGnubbyExtension + ).catch(function () { + return new Promise(findDevGnubbyExtension); + } + ).catch(function () { + // If no extensions are found, return false. + return Promise.resolve(false); + } + ); + + return this.gnubbyExtensionPromise_; +}; + +/** @override @return {Array<string>} */ remoting.GnubbyAuthHandler.prototype.getExtensionTypes = function() { return [remoting.GnubbyAuthHandler.EXTENSION_TYPE]; }; @@ -100,37 +159,19 @@ }; /** - * Send data to the gnubbyd extension. + * Send data to the installed gnubbyd extension. * @param {Object} jsonObject The JSON object to send to the gnubbyd extension. * @param {function(Object)} callback The callback to invoke with reply data. * @private */ remoting.GnubbyAuthHandler.prototype.sendMessageToGnubbyd_ = function(jsonObject, callback) { - var kGnubbydDevExtensionId = 'dlfcjilkjfhdnfiecknlnddkmmiofjbg'; - - chrome.runtime.sendMessage( - kGnubbydDevExtensionId, - jsonObject, - onGnubbydDevReply_.bind(this, jsonObject, callback)); + this.isGnubbyExtensionInstalled().then( + function (extensionInstalled) { + if (extensionInstalled) { + chrome.runtime.sendMessage( + this.gnubbyExtensionId_, jsonObject, callback); + } + }.bind(this) + ); }; - -/** - * Callback invoked as a result of sending a message to the gnubbyd-dev - * extension. If that extension is not installed, reply will be undefined; - * otherwise it will be the JSON response object. - * @param {Object} jsonObject The JSON object to send to the gnubbyd extension. - * @param {function(Object)} callback The callback to invoke with reply data. - * @param {Object} reply The reply from the extension (or Chrome, if the - * extension does not exist. - * @private - */ -function onGnubbydDevReply_(jsonObject, callback, reply) { - var kGnubbydStableExtensionId = 'beknehfpfkghjoafdifaflglpjkojoco'; - - if (reply) { - callback(reply); - } else { - chrome.runtime.sendMessage(kGnubbydStableExtensionId, jsonObject, callback); - } -}
diff --git a/remoting/webapp/crd/js/gnubby_auth_handler_unittest.js b/remoting/webapp/crd/js/gnubby_auth_handler_unittest.js new file mode 100644 index 0000000..7b63da0e --- /dev/null +++ b/remoting/webapp/crd/js/gnubby_auth_handler_unittest.js
@@ -0,0 +1,193 @@ +// Copyright 2016 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. + +/** + * @fileoverview + */ + +(function() { + +'use strict'; + +/** @const {string} */ +var EXTENSION_TYPE = 'gnubby-auth'; + +/** @const {string} */ +var GNUBBY_DEV_EXTENSION_ID = 'dlfcjilkjfhdnfiecknlnddkmmiofjbg'; + +/** @const {string} */ +var GNUBBY_STABLE_EXTENSION_ID = 'beknehfpfkghjoafdifaflglpjkojoco'; + +/** @type {sinon.TestStub} */ +var sendMessageStub = null; + +/** @type {remoting.GnubbyAuthHandler} */ +var gnubbyAuthHandler = null; + +var sendMessageHandler = null; + +/** @const */ +var messageData = { + 'type': 'data', + 'data': ['Blergh!', 'Blargh!', 'Bleh!'], + 'connectionId': 42, +}; + +/** @const {string} */ +var messageJSON = JSON.stringify(messageData); + +QUnit.module('GnubbyAuthHandler', { + beforeEach: function(/** QUnit.Assert */ assert) { + // Configurable handler used for test customization. + sendMessageStub = sinon.stub(chromeMocks.runtime, 'sendMessage', + function(extensionId, message, responseCallback) { + sendMessageHandler(extensionId, message, responseCallback); + } + ); + }, + afterEach: function(/** QUnit.Assert */ assert) { + gnubbyAuthHandler = null; + sendMessageStub.restore(); + } +}); + +QUnit.test( + 'isGnubbyExtensionInstalled() with no gnubby extensions installed', + function(assert) { + assert.expect(1); + var done = assert.async(); + + gnubbyAuthHandler = new remoting.GnubbyAuthHandler(); + sendMessageHandler = function(extensionId, message, responseCallback) { + Promise.resolve().then(function() { responseCallback(null); }); + } + + gnubbyAuthHandler.isGnubbyExtensionInstalled().then( + function(isInstalled) { + assert.ok(!isInstalled); + done(); + } + ); +}); + +QUnit.test( + 'isGnubbyExtensionInstalled() with Dev gnubby extensions installed', + function(assert) { + assert.expect(1); + var done = assert.async(); + + sendMessageHandler = function(extensionId, message, responseCallback) { + var response = null; + if (extensionId === GNUBBY_DEV_EXTENSION_ID) { + response = { data: "W00t!" }; + } + Promise.resolve().then(function() { responseCallback(response); }); + }; + + gnubbyAuthHandler = new remoting.GnubbyAuthHandler(); + gnubbyAuthHandler.isGnubbyExtensionInstalled().then( + function(isInstalled) { + assert.ok(isInstalled); + done(); + } + ); +}); + +QUnit.test( + 'isGnubbyExtensionInstalled() with Stable gnubby extensions installed', + function(assert) { + assert.expect(1); + var done = assert.async(); + + sendMessageHandler = function(extensionId, message, responseCallback) { + var response = null; + if (extensionId === GNUBBY_STABLE_EXTENSION_ID) { + response = { data: "W00t!" }; + } + Promise.resolve().then(function() { responseCallback(response); }); + }; + + gnubbyAuthHandler = new remoting.GnubbyAuthHandler(); + gnubbyAuthHandler.isGnubbyExtensionInstalled().then( + function(isInstalled) { + assert.ok(isInstalled); + done(); + } + ); +}); + +QUnit.test('startExtension() sends message to host.', + function(assert) { + assert.expect(3); + var done = assert.async(); + + var sendMessageToHostCallback = function(extensionType, message) { + assert.equal(extensionType, EXTENSION_TYPE); + var messageObject = JSON.parse(message); + assert.equal('control', messageObject['type']); + assert.equal('auth-v1', messageObject['option']); + done(); + } + gnubbyAuthHandler = new remoting.GnubbyAuthHandler(); + gnubbyAuthHandler.startExtension(sendMessageToHostCallback); +}); + +QUnit.test( + 'onExtensionMessage() sends message to stable gnubby extension.', + function(assert) { + assert.expect(6); + var done1 = assert.async(); + var done2 = assert.async(); + + var isGnubbyExtensionInstalledHandler = + function(extensionId, message, responseCallback) { + if (extensionId === GNUBBY_STABLE_EXTENSION_ID) { + Promise.resolve().then(function() { + responseCallback({ data: "W00t!" }); + }); + } + }; + + var onExtensionMessageHandler = + function(extensionId, message, responseCallback) { + assert.equal(extensionId, GNUBBY_STABLE_EXTENSION_ID, + 'Expected the stable gnubby extension ID.'); + Promise.resolve().then(function() { responseCallback(message); }); + }; + + var sendMessageToHostCallback = function(extensionType, message) { + assert.equal(extensionType, EXTENSION_TYPE); + var messageObject = JSON.parse(message); + var messageType = messageObject['type']; + if (messageType === 'control') { + // This is the first control message sent to the host. + done1(); + } else if (messageType === 'data') { + // Response from gnubby extension. + assert.equal(messageData.connectionId, messageObject['connectionId']); + assert.deepEqual(messageData.data, messageObject['data']); + done2(); + } else { + assert.ok(false); + } + }; + + gnubbyAuthHandler = new remoting.GnubbyAuthHandler(); + gnubbyAuthHandler.startExtension(sendMessageToHostCallback); + + sendMessageHandler = isGnubbyExtensionInstalledHandler; + return gnubbyAuthHandler.isGnubbyExtensionInstalled().then( + function(isInstalled) { + assert.ok(isInstalled); + // The promise has completed for isGnubbyExtensionInstalled() so now we + // can switch the handler out to work with the onExtensionMessage call. + sendMessageHandler = onExtensionMessageHandler; + return Promise.resolve().then(function() { + gnubbyAuthHandler.onExtensionMessage(EXTENSION_TYPE, messageData); + }); + } + ); +}); + +})();
diff --git a/remoting/webapp/crd/js/me2me_activity.js b/remoting/webapp/crd/js/me2me_activity.js index aa43d8a..66530a0 100644 --- a/remoting/webapp/crd/js/me2me_activity.js +++ b/remoting/webapp/crd/js/me2me_activity.js
@@ -39,6 +39,12 @@ /** @private {remoting.DesktopRemotingActivity} */ this.desktopActivity_ = null; + + /** @private {remoting.GnubbyAuthHandler} */ + this.gnubbyAuthHandler_ = new remoting.GnubbyAuthHandler(); + + /** @private {Array<string>} */ + this.additionalCapabilities_ = []; }; remoting.Me2MeActivity.prototype.dispose = function() { @@ -74,6 +80,16 @@ // User cancels out of the Host upgrade dialog. Report it as bad version. throw new remoting.Error(remoting.Error.Tag.BAD_VERSION); })).then( + this.gnubbyAuthHandler_.isGnubbyExtensionInstalled.bind( + this.gnubbyAuthHandler_) + ).then( + function (extensionInstalled) { + if (extensionInstalled) { + this.additionalCapabilities_.push( + remoting.ClientSession.Capability.SECURITY_KEY); + } + }.bind(this) + ).then( this.connect_.bind(this) ).catch(remoting.Error.handler(handleError)); }; @@ -124,8 +140,9 @@ */ remoting.Me2MeActivity.prototype.connect_ = function() { base.dispose(this.desktopActivity_); - this.desktopActivity_ = - new remoting.DesktopRemotingActivity(this, this.logger_); + + this.desktopActivity_ = new remoting.DesktopRemotingActivity( + this, this.logger_, this.additionalCapabilities_); this.desktopActivity_.getConnectingDialog().show(); this.desktopActivity_.start(this.host_, this.createCredentialsProvider_()); }; @@ -240,7 +257,9 @@ if (plugin.hasCapability(remoting.ClientSession.Capability.CAST)) { plugin.extensions().register(new remoting.CastExtensionHandler()); } - plugin.extensions().register(new remoting.GnubbyAuthHandler()); + // TODO(joedow): Do not register the GnubbyAuthHandler extension if the host + // does not support security key forwarding. + plugin.extensions().register(this.gnubbyAuthHandler_); this.pinDialog_.requestPairingIfNecessary(connectionInfo.plugin()); // Drop the session after 30s of suspension. If this timeout is too short, we
diff --git a/remoting/webapp/crd/js/remoting_activity_test_driver.js b/remoting/webapp/crd/js/remoting_activity_test_driver.js index 92b22aa3..3771371 100644 --- a/remoting/webapp/crd/js/remoting_activity_test_driver.js +++ b/remoting/webapp/crd/js/remoting_activity_test_driver.js
@@ -97,6 +97,10 @@ sinon.stub(remoting.NetworkConnectivityDetector, 'create', function(){ return new MockNetworkConnectivityDetector(); }); + /** @private */ + this.isGnubbyExtensionInstalledStub_ = sinon.stub( + remoting.GnubbyAuthHandler.prototype, 'isGnubbyExtensionInstalled', + function() { return Promise.resolve(false); }); /** * Use fake timers to prevent the generation of session ID expiration events. @@ -125,6 +129,7 @@ this.eventWriterMock_.restore(); this.desktopConnectedViewCreateStub_.restore(); this.createNetworkConnectivityDetectorStub_.restore(); + this.isGnubbyExtensionInstalledStub_.restore(); if (Boolean(this.mockConnection_)) { this.mockConnection_.restore(); this.mockConnection_ = null;
diff --git a/remoting/webapp/crd/js/video_frame_recorder.js b/remoting/webapp/crd/js/video_frame_recorder.js index 1b53b5c..6064c4a2 100644 --- a/remoting/webapp/crd/js/video_frame_recorder.js +++ b/remoting/webapp/crd/js/video_frame_recorder.js
@@ -27,7 +27,7 @@ /** @private {string} */ remoting.VideoFrameRecorder.EXTENSION_TYPE = 'video-recorder'; -/** @return {Array<string>} */ +/** @override @return {Array<string>} */ remoting.VideoFrameRecorder.prototype.getExtensionTypes = function() { return [remoting.VideoFrameRecorder.EXTENSION_TYPE]; };
diff --git a/remoting/webapp/files.gni b/remoting/webapp/files.gni index 7f43734..df837ae2 100644 --- a/remoting/webapp/files.gni +++ b/remoting/webapp/files.gni
@@ -110,6 +110,7 @@ "crd/js/combined_host_list_api_unittest.js", "crd/js/gcd_client_unittest.js", "crd/js/gcd_client_with_mock_xhr_unittest.js", + "crd/js/gnubby_auth_handler_unittest.js", "crd/js/host_controller_unittest.js", "crd/js/host_daemon_facade_unittest.js", "crd/js/host_table_entry_unittest.js",
diff --git a/testing/buildbot/chromium.win.json b/testing/buildbot/chromium.win.json index ae4a5f4..9bfcdb29 100644 --- a/testing/buildbot/chromium.win.json +++ b/testing/buildbot/chromium.win.json
@@ -424,62 +424,39 @@ "Win x64 GN": { "additional_compile_targets": [ "gn_all" + ], + "gtest_tests": [ + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "content_browsertests" + }, + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "events_unittests" + } ] }, "Win x64 GN (dbg)": { "additional_compile_targets": [ - "accessibility_unittests", - "app_list_unittests", - "app_shell_unittests", - "ash_shell_with_content", - "ash_unittests", - "aura_unittests", - "battor_agent", - "cacheinvalidation_unittests", - "cast_unittests", - "cc_unittests", - "chrome", - "chrome_elf_unittests", - "chromedriver_unittests", - "components_browsertests", - "components_unittests", - "compositor_unittests", - "content_browsertests", - "content_unittests", - "courgette_unittests", - "crypto_unittests", - "device_unittests", - "events_unittests", - "extensions_browsertests", - "extensions_unittests", - "gcm_unit_tests", - "gfx_unittests", - "google_apis_unittests", - "gpu_unittests", - "interactive_ui_tests", - "ipc_mojo_unittests", - "ipc_tests", - "jingle_unittests", - "keyboard_unittests", - "mash:all", - "media_unittests", - "media_blink_unittests", - "message_center_unittests", - "ppapi_unittests", - "printing_unittests", - "remoting_all", - "sbox_integration_tests", - "sbox_unittests", - "sbox_validation_tests", - "skia_unittests", - "sql_unittests", - "sync_integration_tests", - "sync_unit_tests", - "ui_base_unittests", - "ui_touch_selection_unittests", - "url_unittests", - "views_unittests", - "wm_unittests" + "gn_all" + ], + "gtest_tests": [ + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "content_browsertests" + }, + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "events_unittests" + } ] }, "Win7 (32) Tests": { @@ -1396,9 +1373,15 @@ "test": "battor_agent_unittests" }, { + "swarming": { + "can_use_on_swarming_builders": true + }, "test": "content_browsertests" }, { + "swarming": { + "can_use_on_swarming_builders": true + }, "test": "events_unittests" }, { @@ -1453,58 +1436,21 @@ }, "Win8 GN (dbg)": { "additional_compile_targets": [ - "accessibility_unittests", - "app_list_unittests", - "app_shell_unittests", - "ash_shell_with_content", - "ash_unittests", - "aura_unittests", - "battor_agent", - "cacheinvalidation_unittests", - "cast_unittests", - "cc_unittests", - "chrome", - "chrome_elf_unittests", - "chromedriver_unittests", - "components_browsertests", - "components_unittests", - "compositor_unittests", - "content_browsertests", - "content_unittests", - "courgette_unittests", - "crypto_unittests", - "device_unittests", - "events_unittests", - "extensions_browsertests", - "extensions_unittests", - "gcm_unit_tests", - "gfx_unittests", - "google_apis_unittests", - "gpu_unittests", - "interactive_ui_tests", - "ipc_mojo_unittests", - "ipc_tests", - "jingle_unittests", - "keyboard_unittests", - "mash:all", - "media_unittests", - "media_blink_unittests", - "message_center_unittests", - "ppapi_unittests", - "printing_unittests", - "remoting_all", - "sbox_integration_tests", - "sbox_unittests", - "sbox_validation_tests", - "skia_unittests", - "sql_unittests", - "sync_integration_tests", - "sync_unit_tests", - "ui_base_unittests", - "ui_touch_selection_unittests", - "url_unittests", - "views_unittests", - "wm_unittests" + "gn_all" + ], + "gtest_tests": [ + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "content_browsertests" + }, + { + "swarming": { + "can_use_on_swarming_builders": true + }, + "test": "events_unittests" + } ] } }
diff --git a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp index de0568a..6155dc0 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp +++ b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
@@ -634,7 +634,7 @@ long long size = buffer->getSize() - offset; // If size is negative, or size is not large enough to store pixels, those cases // are handled by validateReadPixelsFuncParameters to generate INVALID_OPERATION. - if (!validateReadPixelsFuncParameters(width, height, format, type, size)) + if (!validateReadPixelsFuncParameters(width, height, format, type, nullptr, size)) return; clearIfComposited(); @@ -2992,7 +2992,7 @@ } } -bool WebGL2RenderingContextBase::validateReadPixelsFormatAndType(GLenum format, GLenum type) +bool WebGL2RenderingContextBase::validateReadPixelsFormatAndType(GLenum format, GLenum type, DOMArrayBufferView* buffer) { switch (format) { case GL_RED: @@ -3014,49 +3014,63 @@ switch (type) { case GL_UNSIGNED_BYTE: + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint8) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type UNSIGNED_BYTE but ArrayBufferView not Uint8Array"); + return false; + } + return true; case GL_BYTE: + if (buffer && buffer->type() != DOMArrayBufferView::TypeInt8) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type BYTE but ArrayBufferView not Int8Array"); + return false; + } + return true; case GL_HALF_FLOAT: + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint16) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type HALF_FLOAT but ArrayBufferView not Uint16Array"); + return false; + } + return true; case GL_FLOAT: + if (buffer && buffer->type() != DOMArrayBufferView::TypeFloat32) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type FLOAT but ArrayBufferView not Float32Array"); + return false; + } + return true; case GL_UNSIGNED_SHORT: case GL_UNSIGNED_SHORT_5_6_5: case GL_UNSIGNED_SHORT_4_4_4_4: case GL_UNSIGNED_SHORT_5_5_5_1: + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint16) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type UNSIGNED_SHORT but ArrayBufferView not Uint16Array"); + return false; + } + return true; case GL_SHORT: + if (buffer && buffer->type() != DOMArrayBufferView::TypeInt16) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type SHORT but ArrayBufferView not Int16Array"); + return false; + } + return true; case GL_UNSIGNED_INT: case GL_UNSIGNED_INT_2_10_10_10_REV: case GL_UNSIGNED_INT_10F_11F_11F_REV: case GL_UNSIGNED_INT_5_9_9_9_REV: + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint32) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type UNSIGNED_INT but ArrayBufferView not Uint32Array"); + return false; + } + return true; case GL_INT: - break; + if (buffer && buffer->type() != DOMArrayBufferView::TypeInt32) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type INT but ArrayBufferView not Int32Array"); + return false; + } + return true; default: synthesizeGLError(GL_INVALID_ENUM, "readPixels", "invalid type"); return false; } - - return true; -} - -DOMArrayBufferView::ViewType WebGL2RenderingContextBase::readPixelsExpectedArrayBufferViewType(GLenum type) -{ - switch (type) { - case GL_BYTE: - return DOMArrayBufferView::TypeInt8; - case GL_UNSIGNED_SHORT: - return DOMArrayBufferView::TypeUint16; - case GL_SHORT: - return DOMArrayBufferView::TypeInt16; - case GL_HALF_FLOAT: - return DOMArrayBufferView::TypeUint16; - case GL_UNSIGNED_INT: - case GL_UNSIGNED_INT_2_10_10_10_REV: - case GL_UNSIGNED_INT_10F_11F_11F_REV: - case GL_UNSIGNED_INT_5_9_9_9_REV: - return DOMArrayBufferView::TypeUint32; - case GL_INT: - return DOMArrayBufferView::TypeInt32; - default: - return WebGLRenderingContextBase::readPixelsExpectedArrayBufferViewType(type); - } } WebGLFramebuffer* WebGL2RenderingContextBase::getFramebufferBinding(GLenum target)
diff --git a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h index cffd030..430cb14 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h +++ b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
@@ -224,9 +224,8 @@ bool validateBufferTarget(const char* functionName, GLenum target) override; bool validateAndUpdateBufferBindTarget(const char* functionName, GLenum, WebGLBuffer*) override; bool validateFramebufferTarget(GLenum target) override; - bool validateReadPixelsFormatAndType(GLenum format, GLenum type) override; - DOMArrayBufferView::ViewType readPixelsExpectedArrayBufferViewType(GLenum type) override; + bool validateReadPixelsFormatAndType(GLenum format, GLenum type, DOMArrayBufferView*) override; WebGLFramebuffer* getFramebufferBinding(GLenum target) override; WebGLFramebuffer* getReadFramebufferBinding() override; GLint getMaxTextureLevelForTarget(GLenum target) override;
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp index 7570d6779..b6ccb5d 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp +++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -3469,7 +3469,7 @@ return true; } -bool WebGLRenderingContextBase::validateReadPixelsFormatAndType(GLenum format, GLenum type) +bool WebGLRenderingContextBase::validateReadPixelsFormatAndType(GLenum format, GLenum type, DOMArrayBufferView* buffer) { switch (format) { case GL_ALPHA: @@ -3483,37 +3483,43 @@ switch (type) { case GL_UNSIGNED_BYTE: + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint8) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type UNSIGNED_BYTE but ArrayBufferView not Uint8Array"); + return false; + } + return true; case GL_UNSIGNED_SHORT_5_6_5: case GL_UNSIGNED_SHORT_4_4_4_4: case GL_UNSIGNED_SHORT_5_5_5_1: + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint16) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type UNSIGNED_SHORT but ArrayBufferView not Uint16Array"); + return false; + } + return true; case GL_FLOAT: + if (extensionEnabled(OESTextureFloatName)) { + if (buffer && buffer->type() != DOMArrayBufferView::TypeFloat32) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type FLOAT but ArrayBufferView not Float32Array"); + return false; + } + return true; + } + synthesizeGLError(GL_INVALID_ENUM, "readPixels", "invalid type"); + return false; case GL_HALF_FLOAT_OES: - break; + if (extensionEnabled(OESTextureHalfFloatName)) { + if (buffer && buffer->type() != DOMArrayBufferView::TypeUint16) { + synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "type HALF_FLOAT_OES but ArrayBufferView not Uint16Array"); + return false; + } + return true; + } + synthesizeGLError(GL_INVALID_ENUM, "readPixels", "invalid type"); + return false; default: synthesizeGLError(GL_INVALID_ENUM, "readPixels", "invalid type"); return false; } - - return true; -} - -DOMArrayBufferView::ViewType WebGLRenderingContextBase::readPixelsExpectedArrayBufferViewType(GLenum type) -{ - switch (type) { - case GL_UNSIGNED_BYTE: - return DOMArrayBufferView::TypeUint8; - case GL_UNSIGNED_SHORT_5_6_5: - case GL_UNSIGNED_SHORT_4_4_4_4: - case GL_UNSIGNED_SHORT_5_5_5_1: - return DOMArrayBufferView::TypeUint16; - case GL_FLOAT: - return DOMArrayBufferView::TypeFloat32; - case GL_HALF_FLOAT_OES: - return DOMArrayBufferView::TypeUint16; - default: - ASSERT_NOT_REACHED(); - return DOMArrayBufferView::TypeUint8; - } } WebGLImageConversion::PixelStoreParams WebGLRenderingContextBase::getPackPixelStoreParams() @@ -3530,9 +3536,9 @@ return params; } -bool WebGLRenderingContextBase::validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, long long bufferSize) +bool WebGLRenderingContextBase::validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, DOMArrayBufferView* buffer, long long bufferSize) { - if (!validateReadPixelsFormatAndType(format, type)) + if (!validateReadPixelsFormatAndType(format, type, buffer)) return false; // Calculate array size, taking into consideration of pack parameters. @@ -3567,16 +3573,9 @@ synthesizeGLError(GL_INVALID_FRAMEBUFFER_OPERATION, "readPixels", reason); return; } - if (!validateReadPixelsFuncParameters(width, height, format, type, static_cast<long long>(pixels->byteLength()))) + if (!validateReadPixelsFuncParameters(width, height, format, type, pixels, static_cast<long long>(pixels->byteLength()))) return; - DOMArrayBufferView::ViewType expectedViewType = readPixelsExpectedArrayBufferViewType(type); - // Validate array type against pixel type. - if (pixels->type() != expectedViewType) { - synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "ArrayBufferView was the wrong type for the pixel format"); - return; - } - clearIfComposited(); void* data = pixels->baseAddress();
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h index 3c05bf66..f341d6b 100644 --- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h +++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
@@ -848,16 +848,14 @@ // If not, generates a GL error, returns false. bool validateReadBufferAndGetInfo(const char* functionName, WebGLFramebuffer*& readFramebufferBinding); - // Helper function to check format/type enums for readPixels. + // Helper function to check format/type and ArrayBuffer view type for readPixels. // Generates INVALID_ENUM and returns false if parameters are invalid. - virtual bool validateReadPixelsFormatAndType(GLenum format, GLenum type); - - // Helper function to get expected ArrayBuffer view type for readPixels. - virtual DOMArrayBufferView::ViewType readPixelsExpectedArrayBufferViewType(GLenum type); + // Generates INVALID_OPERATION if ArrayBuffer view type is incompatible with type. + virtual bool validateReadPixelsFormatAndType(GLenum format, GLenum type, DOMArrayBufferView*); // Helper function to check parameters of readPixels. Returns true if all parameters // are valid. Otherwise, generates appropriate error and returns false. - bool validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, long long bufferSize); + bool validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, DOMArrayBufferView*, long long bufferSize); virtual GLint getMaxTextureLevelForTarget(GLenum target);