Don't add fake flash when command-line flash exists.

chrome/common/chrome_content_client.cc

 // Copyright (c) 2012 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 "chrome/common/chrome_content_client.h"
 
 #include <stdint.h>
 
 #include <map>
 #include <memory>
@@ skipping 262 matching lines @@
                                            content::kFlashPluginSwfDescription);
   plugin.mime_types.push_back(swf_mime_type);
   content::WebPluginMimeType spl_mime_type(content::kFlashPluginSplMimeType,
                                            content::kFlashPluginSplExtension,
                                            content::kFlashPluginSplDescription);
   plugin.mime_types.push_back(spl_mime_type);
 
   return plugin;
 }
 
-void AddPepperFlashFromCommandLine(
-    std::vector<content::PepperPluginInfo>* plugins) {
+bool GetCommandLinePepperFlash(content::PepperPluginInfo* plugin) {
   const base::CommandLine::StringType flash_path =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
           switches::kPpapiFlashPath);
   if (flash_path.empty())
-    return;
+    return false;
 
   // Also get the version from the command-line. Should be something like 11.2
   // or 11.2.123.45.
   std::string flash_version =
       base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kPpapiFlashVersion);
 
-  plugins->push_back(
-      CreatePepperFlashInfo(base::FilePath(flash_path),
-                            flash_version, false, true, false));
+  *plugin = CreatePepperFlashInfo(base::FilePath(flash_path),
+                                  flash_version, false, true, false);
+  return true;
 }
 
 #if defined(OS_LINUX)
 // This method is used on Linux only because of architectural differences in how
 // it loads the component updated flash plugin, and not because the other
 // platforms do not support component updated flash. On other platforms, the
 // component updater sends an IPC message to all threads, at undefined points in
 // time, with the URL of the component updated flash. Because the linux zygote
 // thread has no access to the file system after it warms up, it must preload
 // the component updated flash.
@@ skipping 163 matching lines @@
   }
 
   return plugin_map.rbegin()->second;
 }
 #endif  // defined(ENABLE_PLUGINS)
 
 void ChromeContentClient::AddPepperPlugins(
     std::vector<content::PepperPluginInfo>* plugins) {
 #if defined(ENABLE_PLUGINS)
   ComputeBuiltInPlugins(plugins);
-  AddPepperFlashFromCommandLine(plugins);
 
 #if defined(OS_LINUX)
-  // Depending on the sandbox configurtion, the user data directory
+  // Depending on the sandbox configuration, the user data directory
   // is not always available. If it is not available, do not try and load any
   // flash plugin. The flash player, if any, preloaded before the sandbox
   // initialization will continue to be used.
   if (!sandbox::Credentials::HasFileSystemAccess()) {
     return;
   }
 #endif  // defined(OS_LINUX)
 
   ScopedVector<content::PepperPluginInfo> flash_versions;
+  std::unique_ptr<content::PepperPluginInfo> command_line_flash(

[Nico, 2016/09/27 21:09:14]

why is this on the heap?

Also, why the api changin in GetCommandLinePepperFlash()? Couldn't you just pass
flash_versions instead of plugins to the old GetCommandLinePepperFlash()?

[Greg K, 2016/09/27 21:12:29]

FWIW, since the other functions take a PepperPluginInfo*, I like the continuity
here, unless we have them all take the flash_versions list.

[Nico, 2016/09/27 21:13:40]

I like CLs that do one thing per CL: Either change behavior, or refactor code
and don't change behavior. Since this CL says it's a "change behavior" CL I'd
prefer if it was limited to doing the behavior change it's advertised to do in
the simplest way possible :-)

[waffles, 2016/09/27 21:44:52]

Passing flash_versions was my first thought, but since it's a ScopedVector
instead of a std::vector, I still needed to change the signature of that
function; while I was there it started to bother me that the signature of the
function differed from GetComponentUpdatedPepperFlash and GetSystemPepperFlash,
thus the alteration.

I think it's a small refactoring, but I can undo it or split it into a separate
CL if you insist.

Re: heap, it feels strange to me to use ScopedVector with objects not in the
heap, although I agree that the entire ScopedVector here could be rethought.
That seemed like a much larger refactor, so I landed on this middle ground.

[Nico, 2016/09/27 21:46:24]

Ah, I had missed that flash_versions is a ScopedVector, my bad, sorry :-/ lgtm
then.

+      new content::PepperPluginInfo);
+  if (GetCommandLinePepperFlash(command_line_flash.get()))
+    flash_versions.push_back(command_line_flash.release());
 
 #if defined(OS_LINUX)
   std::unique_ptr<content::PepperPluginInfo> component_flash(
       new content::PepperPluginInfo);
   if (GetComponentUpdatedPepperFlash(component_flash.get()))
     flash_versions.push_back(component_flash.release());
 #endif  // defined(OS_LINUX)
 
   std::unique_ptr<content::PepperPluginInfo> system_flash(
       new content::PepperPluginInfo);
@@ skipping 175 matching lines @@
   if (!origin_trial_policy_)
     origin_trial_policy_ = base::MakeUnique<ChromeOriginTrialPolicy>();
   return origin_trial_policy_.get();
 }
 
 #if defined(OS_ANDROID)
 media::MediaClientAndroid* ChromeContentClient::GetMediaClientAndroid() {
   return new ChromeMediaClientAndroid();
 }
 #endif  // OS_ANDROID