From d5e116ae533e10d6528c1a55c4e76f64ce11bd3f Mon Sep 17 00:00:00 2001 From: wfh Date: Mon, 27 Jul 2015 13:47:01 -0700 Subject: [PATCH] One-time migration of NPAPI Flash to PPAPI Flash. Migrate users with PPAPI Flash disabled and NPAPI Flash enabled to PPAPI Flash for M45. Clean up some old migration code that wasn't being used any more. Simplify the system Pepper Flash detection logic. BUG=510114,454131 TEST=See bug for detailed manual tests. Review URL: https://codereview.chromium.org/1255943002 Cr-Commit-Position: refs/heads/master@{#340556} --- chrome/browser/plugins/plugin_prefs.cc | 52 +++++---------------- chrome/browser/plugins/plugin_prefs_factory.cc | 4 +- chrome/browser/ui/webui/plugins_ui.cc | 8 ++-- chrome/common/chrome_content_client.cc | 39 +++------------- chrome/common/chrome_paths.cc | 62 ++++++++++++++------------ chrome/common/chrome_paths.h | 15 ++++--- chrome/common/pref_names.cc | 14 ++---- chrome/common/pref_names.h | 3 +- 8 files changed, 70 insertions(+), 127 deletions(-) diff --git a/chrome/browser/plugins/plugin_prefs.cc b/chrome/browser/plugins/plugin_prefs.cc index fd58cde5acbb..0b9c387b463a 100644 --- a/chrome/browser/plugins/plugin_prefs.cc +++ b/chrome/browser/plugins/plugin_prefs.cc @@ -328,30 +328,17 @@ void PluginPrefs::SetPrefs(PrefService* prefs) { bool migrate_to_pepper_flash = false; #if defined(OS_WIN) || defined(OS_MACOSX) - // If bundled NPAPI Flash is enabled while Pepper Flash is disabled, we - // would like to turn Pepper Flash on. And we only want to do it once. - // TODO(yzshen): Remove all |migrate_to_pepper_flash|-related code after it - // has been run once by most users. (Maybe Chrome 24 or Chrome 25.) - // NOTE(shess): Keep in mind that Mac is on a different schedule. - if (!prefs_->GetBoolean(prefs::kPluginsMigratedToPepperFlash)) { - prefs_->SetBoolean(prefs::kPluginsMigratedToPepperFlash, true); + // If NPAPI Flash is enabled while Pepper Flash is disabled, we would like to + // turn Pepper Flash on. And we want to do it once, when NPAPI is disabled in + // Chrome 45. + // TODO(wfh): Remove this code once it has been run by most users, around + // Chrome 49 or Chrome 50. See crbug.com/514250. + if (!prefs_->GetBoolean(prefs::kNpapiFlashMigratedToPepperFlash)) { + prefs_->SetBoolean(prefs::kNpapiFlashMigratedToPepperFlash, true); migrate_to_pepper_flash = true; } #endif - bool remove_component_pepper_flash_settings = false; - // If component-updated Pepper Flash is disabled, we would like to remove that - // settings item. And we only want to do it once. (Please see the comments of - // kPluginsRemovedOldComponentPepperFlashSettings for why.) - // TODO(yzshen): Remove all |remove_component_pepper_flash_settings|-related - // code after it has been run once by most users. - if (!prefs_->GetBoolean( - prefs::kPluginsRemovedOldComponentPepperFlashSettings)) { - prefs_->SetBoolean(prefs::kPluginsRemovedOldComponentPepperFlashSettings, - true); - remove_component_pepper_flash_settings = true; - } - { // Scoped update of prefs::kPluginsPluginsList. ListPrefUpdate update(prefs_, prefs::kPluginsPluginsList); base::ListValue* saved_plugins_list = update.Get(); @@ -363,14 +350,10 @@ void PluginPrefs::SetPrefs(PrefService* prefs) { base::DictionaryValue* pepper_flash_node = NULL; bool npapi_flash_enabled = false; if (migrate_to_pepper_flash) { - PathService::Get(chrome::FILE_FLASH_PLUGIN, &npapi_flash); + PathService::Get(chrome::FILE_FLASH_SYSTEM_PLUGIN, &npapi_flash); PathService::Get(chrome::FILE_PEPPER_FLASH_PLUGIN, &pepper_flash); } - // Used when |remove_component_pepper_flash_settings| is set to true. - base::ListValue::iterator component_pepper_flash_node = - saved_plugins_list->end(); - for (base::ListValue::iterator it = saved_plugins_list->begin(); it != saved_plugins_list->end(); ++it) { @@ -393,7 +376,7 @@ void PluginPrefs::SetPrefs(PrefService* prefs) { // Files have a path attribute, groups don't. base::FilePath plugin_path(path); - // The path to the intenral plugin directory changes everytime Chrome + // The path to the internal plugin directory changes everytime Chrome // is auto-updated, since it contains the current version number. For // example, it changes from foobar\Chrome\Application\21.0.1180.83 to // foobar\Chrome\Application\21.0.1180.89. @@ -432,22 +415,14 @@ void PluginPrefs::SetPrefs(PrefService* prefs) { } } - if (migrate_to_pepper_flash && - base::FilePath::CompareEqualIgnoreCase( - path, npapi_flash.value())) { + if (migrate_to_pepper_flash && base::FilePath::CompareEqualIgnoreCase( + path, npapi_flash.value())) { npapi_flash_enabled = enabled; } else if (migrate_to_pepper_flash && base::FilePath::CompareEqualIgnoreCase( path, pepper_flash.value())) { if (!enabled) pepper_flash_node = plugin; - } else if (remove_component_pepper_flash_settings && - IsComponentUpdatedPepperFlash(plugin_path)) { - if (!enabled) { - component_pepper_flash_node = it; - // Skip setting |enabled| into |plugin_state_|. - continue; - } } plugin_state_.Set(plugin_path, enabled); @@ -462,11 +437,6 @@ void PluginPrefs::SetPrefs(PrefService* prefs) { pepper_flash_node->SetBoolean("enabled", true); plugin_state_.Set(pepper_flash, true); } - - if (component_pepper_flash_node != saved_plugins_list->end()) { - DCHECK(remove_component_pepper_flash_settings); - saved_plugins_list->Erase(component_pepper_flash_node, NULL); - } } else { // If the saved plugin list is empty, then the call to UpdatePreferences() // below failed in an earlier run, possibly because the user closed the diff --git a/chrome/browser/plugins/plugin_prefs_factory.cc b/chrome/browser/plugins/plugin_prefs_factory.cc index 4bba9ad3239f..35eead514422 100644 --- a/chrome/browser/plugins/plugin_prefs_factory.cc +++ b/chrome/browser/plugins/plugin_prefs_factory.cc @@ -57,9 +57,7 @@ void PluginPrefsFactory::RegisterProfilePrefs( PathService::Get(chrome::DIR_INTERNAL_PLUGINS, &internal_dir); registry->RegisterFilePathPref(prefs::kPluginsLastInternalDirectory, internal_dir); - registry->RegisterBooleanPref(prefs::kPluginsMigratedToPepperFlash, false); - registry->RegisterBooleanPref( - prefs::kPluginsRemovedOldComponentPepperFlashSettings, false); + registry->RegisterBooleanPref(prefs::kNpapiFlashMigratedToPepperFlash, false); registry->RegisterListPref(prefs::kPluginsPluginsList); registry->RegisterListPref(prefs::kPluginsDisabledPlugins); registry->RegisterListPref(prefs::kPluginsDisabledPluginsExceptions); diff --git a/chrome/browser/ui/webui/plugins_ui.cc b/chrome/browser/ui/webui/plugins_ui.cc index 9d5d8241d8bf..6826844584b3 100644 --- a/chrome/browser/ui/webui/plugins_ui.cc +++ b/chrome/browser/ui/webui/plugins_ui.cc @@ -376,9 +376,11 @@ void PluginsDOMHandler::PluginsLoaded( base::string16 desc = group_plugin.desc; if (group_plugin.is_pepper_plugin() && group_plugin.name == base::ASCIIToUTF16(content::kFlashPluginName)) { - base::FilePath system_path; - PathService::Get(chrome::DIR_PEPPER_FLASH_SYSTEM_PLUGIN, &system_path); - if (group_plugin.path.DirName() == system_path) { + base::FilePath system_flash_path; + PathService::Get(chrome::FILE_PEPPER_FLASH_SYSTEM_PLUGIN, + &system_flash_path); + if (base::FilePath::CompareEqualIgnoreCase(group_plugin.path.value(), + system_flash_path.value())) { #if defined(GOOGLE_CHROME_BUILD) // Existing documentation for debugging Flash describe this plugin as // "Debug" so preserve this nomenclature here. diff --git a/chrome/common/chrome_content_client.cc b/chrome/common/chrome_content_client.cc index 5d13f3352bd4..a4e28a1c3d98 100644 --- a/chrome/common/chrome_content_client.cc +++ b/chrome/common/chrome_content_client.cc @@ -308,17 +308,6 @@ bool GetBundledPepperFlash(content::PepperPluginInfo* plugin) { #endif // FLAPPER_AVAILABLE } -#if defined(OS_WIN) -const char kPepperFlashDLLBaseName[] = -#if defined(ARCH_CPU_X86) - "pepflashplayer32_"; -#elif defined(ARCH_CPU_X86_64) - "pepflashplayer64_"; -#else -#error Unsupported Windows CPU architecture. -#endif // defined(ARCH_CPU_X86) -#endif // defined(OS_WIN) - #if defined(FLAPPER_AVAILABLE) bool IsSystemFlashScriptDebuggerPresent() { #if defined(OS_WIN) @@ -356,14 +345,16 @@ bool GetSystemPepperFlash(content::PepperPluginInfo* plugin) { if (command_line->HasSwitch(switches::kPpapiFlashPath)) return false; - base::FilePath flash_path; - if (!PathService::Get(chrome::DIR_PEPPER_FLASH_SYSTEM_PLUGIN, &flash_path)) + base::FilePath flash_filename; + if (!PathService::Get(chrome::FILE_PEPPER_FLASH_SYSTEM_PLUGIN, + &flash_filename)) return false; - if (!base::PathExists(flash_path)) + if (!base::PathExists(flash_filename)) return false; - base::FilePath manifest_path(flash_path.AppendASCII("manifest.json")); + base::FilePath manifest_path( + flash_filename.DirName().AppendASCII("manifest.json")); std::string manifest_data; if (!base::ReadFileToString(manifest_path, &manifest_data)) @@ -380,23 +371,7 @@ bool GetSystemPepperFlash(content::PepperPluginInfo* plugin) { if (!chrome::CheckPepperFlashManifest(*manifest, &version)) return false; -#if defined(OS_WIN) - // PepperFlash DLLs on Windows look like basename_v_x_y_z.dll. - std::string filename(kPepperFlashDLLBaseName); - filename.append(version.GetString()); - base::ReplaceChars(filename, ".", "_", &filename); - filename.append(".dll"); - - base::FilePath path(flash_path.Append(base::ASCIIToUTF16(filename))); -#else - // PepperFlash on OS X is called PepperFlashPlayer.plugin - base::FilePath path(flash_path.Append(chrome::kPepperFlashPluginFilename)); -#endif - - if (!base::PathExists(path)) - return false; - - *plugin = CreatePepperFlashInfo(path, version.GetString()); + *plugin = CreatePepperFlashInfo(flash_filename, version.GetString()); return true; } #endif // defined(ENABLE_PLUGINS) diff --git a/chrome/common/chrome_paths.cc b/chrome/common/chrome_paths.cc index b48092b758a9..ed7cdfb75965 100644 --- a/chrome/common/chrome_paths.cc +++ b/chrome/common/chrome_paths.cc @@ -35,22 +35,6 @@ namespace { -#if defined(OS_WIN) -const wchar_t kFlashRegistryRoot[] = L"SOFTWARE\\Macromedia\\FlashPlayerPepper"; - -const wchar_t kFlashPlayerPathValueName[] = L"PlayerPath"; -#endif - -// File name of the internal Flash plugin on different platforms. -const base::FilePath::CharType kInternalFlashPluginFileName[] = -#if defined(OS_MACOSX) - FILE_PATH_LITERAL("Flash Player Plugin for Chrome.plugin"); -#elif defined(OS_WIN) - FILE_PATH_LITERAL("gcswf32.dll"); -#else // OS_LINUX, etc. - FILE_PATH_LITERAL("libgcflashplayer.so"); -#endif - // The Pepper Flash plugins are in a directory with this name. const base::FilePath::CharType kPepperFlashBaseDirectory[] = FILE_PATH_LITERAL("PepperFlash"); @@ -58,6 +42,10 @@ const base::FilePath::CharType kPepperFlashBaseDirectory[] = #if defined(OS_MACOSX) && !defined(OS_IOS) const base::FilePath::CharType kPepperFlashSystemBaseDirectory[] = FILE_PATH_LITERAL("Internet Plug-Ins/PepperFlashPlayer"); +const base::FilePath::CharType kFlashSystemBaseDirectory[] = + FILE_PATH_LITERAL("Internet Plug-Ins"); +const base::FilePath::CharType kFlashSystemPluginName[] = + FILE_PATH_LITERAL("Flash Player.plugin"); #endif const base::FilePath::CharType kInternalNaClPluginFileName[] = @@ -96,15 +84,23 @@ bool GetInternalPluginsDirectory(base::FilePath* result) { } #if defined(OS_WIN) -// Gets the Flash path if installed on the system. -bool GetSystemFlashDirectory(base::FilePath* out_path) { - base::win::RegKey path_key(HKEY_LOCAL_MACHINE, kFlashRegistryRoot, KEY_READ); +// Gets the Flash path if installed on the system. |is_npapi| determines whether +// to return the NPAPI of the PPAPI version of the system plugin. +bool GetSystemFlashFilename(base::FilePath* out_path, bool is_npapi) { + const wchar_t kNpapiFlashRegistryRoot[] = + L"SOFTWARE\\Macromedia\\FlashPlayerPlugin"; + const wchar_t kPepperFlashRegistryRoot[] = + L"SOFTWARE\\Macromedia\\FlashPlayerPepper"; + const wchar_t kFlashPlayerPathValueName[] = L"PlayerPath"; + + base::win::RegKey path_key( + HKEY_LOCAL_MACHINE, + is_npapi ? kNpapiFlashRegistryRoot : kPepperFlashRegistryRoot, KEY_READ); base::string16 path_str; if (FAILED(path_key.ReadValue(kFlashPlayerPathValueName, &path_str))) return false; - base::FilePath plugin_path = base::FilePath(path_str).DirName(); - *out_path = plugin_path; + *out_path = base::FilePath(path_str); return true; } #endif @@ -270,20 +266,35 @@ bool PathProvider(int key, base::FilePath* result) { return false; cur = cur.Append(kPepperFlashBaseDirectory); break; - case chrome::DIR_PEPPER_FLASH_SYSTEM_PLUGIN: + case chrome::FILE_PEPPER_FLASH_SYSTEM_PLUGIN: #if defined(OS_WIN) - if (!GetSystemFlashDirectory(&cur)) + if (!GetSystemFlashFilename(&cur, false)) return false; #elif defined(OS_MACOSX) && !defined(OS_IOS) if (!GetLocalLibraryDirectory(&cur)) return false; cur = cur.Append(kPepperFlashSystemBaseDirectory); + cur = cur.Append(chrome::kPepperFlashPluginFilename); #else // Chrome on iOS does not supports PPAPI binaries, return false. // TODO(wfh): If Adobe release PPAPI binaries for Linux, add support here. return false; #endif break; + case chrome::FILE_FLASH_SYSTEM_PLUGIN: +#if defined(OS_WIN) + if (!GetSystemFlashFilename(&cur, true)) + return false; +#elif defined(OS_MACOSX) && !defined(OS_IOS) + if (!GetLocalLibraryDirectory(&cur)) + return false; + cur = cur.Append(kFlashSystemBaseDirectory); + cur = cur.Append(kFlashSystemPluginName); +#else + // Chrome on other platforms does not supports system NPAPI binaries. + return false; +#endif + break; case chrome::FILE_LOCAL_STATE: if (!PathService::Get(chrome::DIR_USER_DATA, &cur)) return false; @@ -294,11 +305,6 @@ bool PathProvider(int key, base::FilePath* result) { return false; cur = cur.Append(FILE_PATH_LITERAL("script.log")); break; - case chrome::FILE_FLASH_PLUGIN: - if (!GetInternalPluginsDirectory(&cur)) - return false; - cur = cur.Append(kInternalFlashPluginFileName); - break; case chrome::FILE_PEPPER_FLASH_PLUGIN: if (!PathService::Get(chrome::DIR_PEPPER_FLASH_PLUGIN, &cur)) return false; diff --git a/chrome/common/chrome_paths.h b/chrome/common/chrome_paths.h index b7021dfc2ddd..cd06c931ce92 100644 --- a/chrome/common/chrome_paths.h +++ b/chrome/common/chrome_paths.h @@ -72,9 +72,6 @@ enum { DIR_COMPONENT_UPDATED_PEPPER_FLASH_PLUGIN, // Base directory of the Pepper // Flash plugins downloaded by the // component updater. - DIR_PEPPER_FLASH_SYSTEM_PLUGIN, // Base directory of the system version of - // the Pepper Flash plugin, downloadable - // from Adobe website. FILE_RESOURCE_MODULE, // Full path and filename of the module that // contains embedded resources (version, // strings, images, etc.). @@ -83,12 +80,16 @@ enum { FILE_RECORDED_SCRIPT, // Full path to the script.log file that // contains recorded browser events for // playback. - FILE_FLASH_PLUGIN, // Full path to the internal NPAPI Flash plugin - // file. Querying this path will succeed no - // matter the file exists or not. FILE_PEPPER_FLASH_PLUGIN, // Full path to the bundled Pepper Flash plugin // file. - + FILE_PEPPER_FLASH_SYSTEM_PLUGIN, // Full path to the system version of the + // Pepper Flash plugin, downloadable from + // Adobe website. Querying this path might + // succeed no matter the file exists or not. + FILE_FLASH_SYSTEM_PLUGIN, // Full path to the system version of NPAPI + // Flash plugin, downloadable from Adobe + // website. Querying this path might succeed no + // matter the file exists or not. FILE_NACL_PLUGIN, // Full path to the internal NaCl plugin file. DIR_PNACL_BASE, // Full path to the base dir for PNaCl. DIR_PNACL_COMPONENT, // Full path to the latest PNaCl version diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index f07c87563b16..65b885789c75 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -905,17 +905,9 @@ const char kPluginsEnabledPlugins[] = "plugins.plugins_enabled"; // When bundled NPAPI Flash is removed, if at that point it is enabled while // Pepper Flash is disabled, we would like to turn on Pepper Flash. And we will -// want to do so only once. -const char kPluginsMigratedToPepperFlash[] = "plugins.migrated_to_pepper_flash"; - -// In the early stage of component-updated PPAPI Flash, we did field trials in -// which it was set to disabled by default. The corresponding settings item may -// remain in some users' profiles. Currently it affects both the bundled and -// component-updated PPAPI Flash (since the two share the same enable/disable -// state). We want to remove this item to get those users to use PPAPI Flash. -// We will want to do so only once. -const char kPluginsRemovedOldComponentPepperFlashSettings[] = - "plugins.removed_old_component_pepper_flash_settings"; +// want to do so in M45, once, for realz. +const char kNpapiFlashMigratedToPepperFlash[] = + "plugins.npapi_flash_migrated_to_pepper_flash"; #if !defined(OS_ANDROID) // Whether about:plugins is shown in the details mode or not. diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 68fc5f84bf96..fbf56af894a9 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -311,8 +311,7 @@ extern const char kPluginsPluginsList[]; extern const char kPluginsDisabledPlugins[]; extern const char kPluginsDisabledPluginsExceptions[]; extern const char kPluginsEnabledPlugins[]; -extern const char kPluginsMigratedToPepperFlash[]; -extern const char kPluginsRemovedOldComponentPepperFlashSettings[]; +extern const char kNpapiFlashMigratedToPepperFlash[]; #if !defined(OS_ANDROID) extern const char kPluginsShowDetails[]; #endif -- 2.11.4.GIT