Plugin Placeholders: Refactor for platforms that don't support plugins

Plugin Placeholders: Refactor for platforms that don't support plugins

This patch removes a lot of plugin placeholder code from builds that don't support plugins (enable_plugins==0).

BUG=

Committed: https://crrev.com/0bd1dade4968461abd05b807baf7c22088b3035a
Cr-Commit-Position: refs/heads/master@{#330444}

[tommycli, 2015-05-15 19:54:43]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-15 19:57:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/1

[commit-bot: I haz the power, 2015-05-15 20:07:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-15 20:07:15]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tommycli, 2015-05-15 20:28:47]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-15 20:29:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/40001

[commit-bot: I haz the power, 2015-05-15 20:48:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-15 20:48:23]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[tommycli, 2015-05-15 22:16:54]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-15 22:19:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/80001

[commit-bot: I haz the power, 2015-05-15 22:39:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-15 22:39:56]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[tommycli, 2015-05-15 22:40:29]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-15 22:43:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/100001

[tommycli, 2015-05-15 23:29:07]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-15 23:29:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/140001

[tommycli, 2015-05-15 23:57:57]

tommycli@chromium.org changed reviewers:
+ bauerb@chromium.org

[tommycli, 2015-05-15 23:57:58]

bauerb: ptal

This is the refactor we discussed a while ago making LoadablePluginPlaceholder
only used in ENABLE_PLUGINS==1 builds.

This reduces the Android apk by 6.5KB, so it's not much, but it's something.

[Bernhard Bauer, 2015-05-16 00:22:21]

Nice! The CL description is a bit unclear though.

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
File chrome/renderer/plugins/non_loadable_plugin_placeholder.cc (right):

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.cc:19:
plugins::PluginPlaceholder* CreateMissingPluginPlaceholder(
Would it make sense to name this method CreateNotSupported(PluginPlaceholder)
and the other one just CreateMissingPlugin? "Missing" would mean that the
content could be shown if the right plugin was installed, and "not supported"
would mean that there is no way this content will be shown.

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.cc:52: new
plugins::PluginPlaceholder(render_frame, NULL, params, html_data);
Use nullptr instead of NULL.

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
File chrome/renderer/plugins/non_loadable_plugin_placeholder.h (right):

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.h:26:
plugins::PluginPlaceholder* CreateMissingPluginPlaceholder(
Put this into a (static) class like NonLoadablePluginPlaceholder, to make it
more similar in usage to ChromePluginPlaceholder? It would also mean we didn't
clutter the global namespace.

https://codereview.chromium.org/1126073003/diff/140001/components/plugins/ren...
File components/plugins/renderer/plugin_placeholder.cc (right):

https://codereview.chromium.org/1126073003/diff/140001/components/plugins/ren...
components/plugins/renderer/plugin_placeholder.cc:17: namespace {
Nit: The anonymous namespace isn't necessary here, as const variables
automatically get internal linkage.

[commit-bot: I haz the power, 2015-05-16 00:54:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-05-16 00:54:23]

Dry run: This issue passed the CQ dry run.

[tommycli, 2015-05-18 20:45:12]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-05-18 20:47:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/160001

[tommycli, 2015-05-18 21:12:42]

bauerb: thanks!

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
File chrome/renderer/plugins/non_loadable_plugin_placeholder.cc (right):

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.cc:19:
plugins::PluginPlaceholder* CreateMissingPluginPlaceholder(
On 2015/05/16 00:22:21, Bernhard Bauer wrote:
> Would it make sense to name this method CreateNotSupported(PluginPlaceholder)
> and the other one just CreateMissingPlugin? "Missing" would mean that the
> content could be shown if the right plugin was installed, and "not supported"
> would mean that there is no way this content will be shown.

Done.

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.cc:52: new
plugins::PluginPlaceholder(render_frame, NULL, params, html_data);
On 2015/05/16 00:22:21, Bernhard Bauer wrote:
> Use nullptr instead of NULL.

Done.

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
File chrome/renderer/plugins/non_loadable_plugin_placeholder.h (right):

https://codereview.chromium.org/1126073003/diff/140001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.h:26:
plugins::PluginPlaceholder* CreateMissingPluginPlaceholder(
On 2015/05/16 00:22:21, Bernhard Bauer wrote:
> Put this into a (static) class like NonLoadablePluginPlaceholder, to make it
> more similar in usage to ChromePluginPlaceholder? It would also mean we didn't
> clutter the global namespace.

Done.

https://codereview.chromium.org/1126073003/diff/140001/components/plugins/ren...
File components/plugins/renderer/plugin_placeholder.cc (right):

https://codereview.chromium.org/1126073003/diff/140001/components/plugins/ren...
components/plugins/renderer/plugin_placeholder.cc:17: namespace {
On 2015/05/16 00:22:21, Bernhard Bauer wrote:
> Nit: The anonymous namespace isn't necessary here, as const variables
> automatically get internal linkage.

Done.

[tommycli, 2015-05-18 21:13:24]

tommycli@chromium.org changed reviewers:
+ thestig@chromium.org

[tommycli, 2015-05-18 21:13:26]

thestig: PTAL
    chrome/renderer/chrome_content_renderer_client.cc
    chrome/renderer/chrome_content_renderer_client.h
    chrome/renderer/resources/blocked_plugin.html

Thanks!

Tommy

[Bernhard Bauer, 2015-05-18 21:29:12]

lgtm

https://codereview.chromium.org/1126073003/diff/160001/chrome/renderer/plugin...
File chrome/renderer/plugins/non_loadable_plugin_placeholder.h (right):

https://codereview.chromium.org/1126073003/diff/160001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.h:35: const
base::FilePath& file_path);
Add DISALLOW_IMPLICIT_CONSTRUCTORS(NonLoadablePluginPlaceholder) to prevent
instantiation.

[tommycli, 2015-05-18 21:42:14]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[tommycli, 2015-05-18 21:42:14]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1126073003/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-05-18 21:42:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/180001

[tommycli, 2015-05-18 21:46:29]

The CQ bit was checked by tommycli@chromium.org to run a CQ dry run

[tommycli, 2015-05-18 21:46:29]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1126073003/#ps200001
(title: " ")

[commit-bot: I haz the power, 2015-05-18 21:47:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/200001

[tommycli, 2015-05-18 22:09:57]

bauerb: thanks!

https://codereview.chromium.org/1126073003/diff/160001/chrome/renderer/plugin...
File chrome/renderer/plugins/non_loadable_plugin_placeholder.h (right):

https://codereview.chromium.org/1126073003/diff/160001/chrome/renderer/plugin...
chrome/renderer/plugins/non_loadable_plugin_placeholder.h:35: const
base::FilePath& file_path);
On 2015/05/18 21:29:12, Bernhard Bauer wrote:
> Add DISALLOW_IMPLICIT_CONSTRUCTORS(NonLoadablePluginPlaceholder) to prevent
> instantiation.

Done.

[Lei Zhang, 2015-05-18 22:29:14]

On 2015/05/18 21:13:26, tommycli wrote:
> thestig: PTAL
>     chrome/renderer/chrome_content_renderer_client.cc
>     chrome/renderer/chrome_content_renderer_client.h
>     chrome/renderer/resources/blocked_plugin.html

lgtm

[tommycli, 2015-05-18 22:29:39]

thestig: thank you!

[tommycli, 2015-05-18 22:29:46]

The CQ bit was unchecked by tommycli@chromium.org

[tommycli, 2015-05-18 22:29:46]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2015-05-18 22:29:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1126073003/200001

[commit-bot: I haz the power, 2015-05-18 22:58:22]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-05-18 22:59:14]

Patchset 11 (id:??) landed as
https://crrev.com/0bd1dade4968461abd05b807baf7c22088b3035a
Cr-Commit-Position: refs/heads/master@{#330444}

[brucedawson, 2015-05-19 00:42:58]

There is a failed Mac ASan 64 Tests build that FindIt suspects is related to
this change. I'm taking a look but I thought I'd pass that along in parallel:

https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests...