diff options
author | ihf@chromium.org <ihf@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-10 09:14:47 +0000 |
---|---|---|
committer | ihf@chromium.org <ihf@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-10 09:14:47 +0000 |
commit | 132bca8c3ea90a9b3a19af613339abf459cb8633 (patch) | |
tree | e6d821fc367e2bb5c2ce8c1bbe8cd80f18151937 | |
parent | 6132f069acd71b9275bed974a5e0b8a6ab9d909a (diff) | |
download | chromium_src-132bca8c3ea90a9b3a19af613339abf459cb8633.zip chromium_src-132bca8c3ea90a9b3a19af613339abf459cb8633.tar.gz chromium_src-132bca8c3ea90a9b3a19af613339abf459cb8633.tar.bz2 |
Instrument pepper plugin load failures.
ChromeOS experiences an increased failure to load Pepper Flash plugins.
To help investigate these failures in the field increase logging.
Also disallow npapi plugin generation on ChromeOS as a fallback. We
don't need a zombie process when this failure happens.
BUG=chromium:314301
TEST=kill -stop, reload, kill -9 ppapi process on Pixel.
Review URL: https://codereview.chromium.org/103623003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239740 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | content/browser/plugin_service_impl.cc | 24 | ||||
-rw-r--r-- | content/browser/ppapi_plugin_process_host.cc | 15 | ||||
-rw-r--r-- | content/common/pepper_plugin_list.cc | 6 | ||||
-rw-r--r-- | content/renderer/render_frame_impl.cc | 6 | ||||
-rw-r--r-- | content/zygote/zygote_main_linux.cc | 7 |
5 files changed, 43 insertions, 15 deletions
diff --git a/content/browser/plugin_service_impl.cc b/content/browser/plugin_service_impl.cc index b0153d9..74feede 100644 --- a/content/browser/plugin_service_impl.cc +++ b/content/browser/plugin_service_impl.cc @@ -298,7 +298,12 @@ PluginProcessHost* PluginServiceImpl::FindOrStartNpapiPluginProcess( START_NPAPI_FLASH_AT_LEAST_ONCE, FLASH_USAGE_ENUM_COUNT); } - +#if defined(OS_CHROMEOS) + // TODO(ihf): Move to an earlier place once crbug.com/314301 is fixed. For now + // we still want Plugin.FlashUsage recorded if we end up here. + LOG(WARNING) << "Refusing to start npapi plugin on ChromeOS."; + return NULL; +#endif // This plugin isn't loaded by any plugin process, so create a new process. scoped_ptr<PluginProcessHost> new_host(new PluginProcessHost()); if (!new_host->Init(info)) { @@ -314,8 +319,10 @@ PpapiPluginProcessHost* PluginServiceImpl::FindOrStartPpapiPluginProcess( const base::FilePath& profile_data_directory) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - if (filter_ && !filter_->CanLoadPlugin(render_process_id, plugin_path)) + if (filter_ && !filter_->CanLoadPlugin(render_process_id, plugin_path)) { + VLOG(1) << "Unable to load ppapi plugin: " << plugin_path.MaybeAsASCII(); return NULL; + } PpapiPluginProcessHost* plugin_host = FindPpapiPluginProcess(plugin_path, profile_data_directory); @@ -324,8 +331,11 @@ PpapiPluginProcessHost* PluginServiceImpl::FindOrStartPpapiPluginProcess( // Validate that the plugin is actually registered. PepperPluginInfo* info = GetRegisteredPpapiPluginInfo(plugin_path); - if (!info) + if (!info) { + VLOG(1) << "Unable to find ppapi plugin registration for: " + << plugin_path.MaybeAsASCII(); return NULL; + } // Record when PPAPI Flash process is started for the first time. static bool counted = false; @@ -337,8 +347,14 @@ PpapiPluginProcessHost* PluginServiceImpl::FindOrStartPpapiPluginProcess( } // This plugin isn't loaded by any plugin process, so create a new process. - return PpapiPluginProcessHost::CreatePluginHost( + plugin_host = PpapiPluginProcessHost::CreatePluginHost( *info, profile_data_directory); + if (!plugin_host) { + VLOG(1) << "Unable to create ppapi plugin process for: " + << plugin_path.MaybeAsASCII(); + } + + return plugin_host; } PpapiPluginProcessHost* PluginServiceImpl::FindOrStartPpapiBrokerProcess( diff --git a/content/browser/ppapi_plugin_process_host.cc b/content/browser/ppapi_plugin_process_host.cc index f6c9fd5..cb408af 100644 --- a/content/browser/ppapi_plugin_process_host.cc +++ b/content/browser/ppapi_plugin_process_host.cc @@ -117,6 +117,7 @@ PpapiPluginProcessHost* PpapiPluginProcessHost::CreatePluginHost( const base::FilePath& profile_data_directory) { PpapiPluginProcessHost* plugin_host = new PpapiPluginProcessHost( info, profile_data_directory); + DCHECK(plugin_host); if (plugin_host->Init(info)) return plugin_host; @@ -253,8 +254,10 @@ bool PpapiPluginProcessHost::Init(const PepperPluginInfo& info) { } std::string channel_id = process_->GetHost()->CreateChannel(); - if (channel_id.empty()) + if (channel_id.empty()) { + VLOG(1) << "Could not create pepper host channel."; return false; + } const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess(); CommandLine::StringType plugin_launcher = @@ -267,8 +270,10 @@ bool PpapiPluginProcessHost::Init(const PepperPluginInfo& info) { int flags = ChildProcessHost::CHILD_NORMAL; #endif base::FilePath exe_path = ChildProcessHost::GetChildPath(flags); - if (exe_path.empty()) + if (exe_path.empty()) { + VLOG(1) << "Pepper plugin exe path is empty."; return false; + } CommandLine* cmd_line = new CommandLine(exe_path); cmd_line->AppendSwitchASCII(switches::kProcessType, @@ -357,10 +362,12 @@ void PpapiPluginProcessHost::RequestPluginChannel(Client* client) { } void PpapiPluginProcessHost::OnProcessLaunched() { + VLOG(2) << "ppapi plugin process launched."; host_impl_->set_plugin_process_handle(process_->GetHandle()); } void PpapiPluginProcessHost::OnProcessCrashed(int exit_code) { + VLOG(1) << "ppapi plugin process crashed."; PluginServiceImpl::GetInstance()->RegisterPluginCrash(plugin_path_); } @@ -391,8 +398,8 @@ void PpapiPluginProcessHost::OnChannelConnected(int32 peer_pid) { // Called when the browser <--> plugin channel has an error. This normally // means the plugin has crashed. void PpapiPluginProcessHost::OnChannelError() { - DVLOG(1) << "PpapiPluginProcessHost" << (is_broker_ ? "[broker]" : "") - << "::OnChannelError()"; + VLOG(1) << "PpapiPluginProcessHost" << (is_broker_ ? "[broker]" : "") + << "::OnChannelError()"; // We don't need to notify the renderers that were communicating with the // plugin since they have their own channels which will go into the error // state at the same time. Instead, we just need to notify any renderers diff --git a/content/common/pepper_plugin_list.cc b/content/common/pepper_plugin_list.cc index ddb4af8..ec5f6ae 100644 --- a/content/common/pepper_plugin_list.cc +++ b/content/common/pepper_plugin_list.cc @@ -56,7 +56,7 @@ void ComputePluginsFromCommandLine(std::vector<PepperPluginInfo>* plugins) { size_t plugins_to_register = modules.size(); if (plugins_to_register > kMaxPluginsToRegisterFromCommandLine) { - DLOG(WARNING) << plugins_to_register << " pepper plugins registered from" + VLOG(1) << plugins_to_register << " pepper plugins registered from" << " command line which exceeds the limit (maximum " << kMaxPluginsToRegisterFromCommandLine << " plugins allowed)"; plugins_to_register = kMaxPluginsToRegisterFromCommandLine; @@ -66,7 +66,7 @@ void ComputePluginsFromCommandLine(std::vector<PepperPluginInfo>* plugins) { std::vector<std::string> parts; base::SplitString(modules[i], ';', &parts); if (parts.size() < 2) { - DLOG(ERROR) << "Required mime-type not found"; + VLOG(1) << "Required mime-type not found"; continue; } @@ -89,7 +89,7 @@ void ComputePluginsFromCommandLine(std::vector<PepperPluginInfo>* plugins) { if (base::PathExists(plugin.path)) { skip_file_check_flags |= index_mask; } else { - DLOG(ERROR) << "Plugin doesn't exist:" << plugin.path.MaybeAsASCII(); + VLOG(1) << "Plugin doesn't exist: " << plugin.path.MaybeAsASCII(); continue; } } diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 0357eab..a02e8f5 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -478,10 +478,14 @@ blink::WebPlugin* RenderFrameImpl::CreatePlugin( pepper_module.get(), params, render_view_->AsWeakPtr(), this); } } - +#if defined(OS_CHROMEOS) + LOG(WARNING) << "Pepper module/plugin creation failed."; + return NULL; +#else // TODO(jam): change to take RenderFrame. return new WebPluginImpl(frame, params, info.path, render_view_->AsWeakPtr(), this); +#endif #else return NULL; #endif diff --git a/content/zygote/zygote_main_linux.cc b/content/zygote/zygote_main_linux.cc index 1d32ab0..9189321 100644 --- a/content/zygote/zygote_main_linux.cc +++ b/content/zygote/zygote_main_linux.cc @@ -262,9 +262,10 @@ void PreloadPepperPlugins() { std::string error; base::NativeLibrary library = base::LoadNativeLibrary(plugins[i].path, &error); - DLOG_IF(WARNING, !library) << "Unable to load plugin " - << plugins[i].path.value() << " " - << error; + VLOG_IF(1, !library) << "Unable to load plugin " + << plugins[i].path.value() << " " + << error; + (void)library; // Prevent release-mode warning. } } |