Always use ENABLE_PLUGINS to indicate plugins support.

Always use ENABLE_PLUGINS to indicate plugins support.

We should rely on GYP_DEFINE "enable_plugins" in gyp files and C++ macro
"ENABLE_PLUGINS" in source code to indicate plugins support. This removes
hard-coded assumptions that certain OS does (or does not) support plugins.

Also add GYP_DEFINE "enable_pdf" and macro "ENABLE_PDF" to indicate
built-in PDF viewer plugin support.

BUG=490433
TEST=trybot
TBR=sievers@chromium.org

Committed: https://crrev.com/11e9711134d075bef1580c56f189ae5e1ab3c034
Cr-Commit-Position: refs/heads/master@{#346284}

[Bernhard Bauer, 2015-08-28 09:08:57]

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

[Bernhard Bauer, 2015-08-28 09:08:57]

Nice, thanks! LGTM with a possible additional simplification:

https://codereview.chromium.org/1318143002/diff/1/content/content_browsertest...
File content/content_browsertests.isolate (right):

https://codereview.chromium.org/1318143002/diff/1/content/content_browsertest...
content/content_browsertests.isolate:80: ['OS=="android" or OS=="linux" or
OS=="mac" or OS=="win"', {
Could this simply be "enable_plugins==1"?

[hshi1, 2015-08-28 16:32:31]

https://codereview.chromium.org/1318143002/diff/1/content/content_browsertest...
File content/content_browsertests.isolate (right):

https://codereview.chromium.org/1318143002/diff/1/content/content_browsertest...
content/content_browsertests.isolate:80: ['OS=="android" or OS=="linux" or
OS=="mac" or OS=="win"', {
On 2015/08/28 09:08:57, Bernhard Bauer wrote:
> Could this simply be "enable_plugins==1"?

Done.

[hshi1, 2015-08-28 16:36:17]

hshi@chromium.org changed reviewers:
+ sievers@chromium.org, thakis@chromium.org

[hshi1, 2015-08-28 16:36:18]

Adding more OWNERS for review. Thanks!

+thakis: OWNER of
chrome/app/chrome_main_delegate.cc
chrome/common/chrome_content_client.cc

+sievers: OWNER of
content/browser/child_process_launcher.cc
content/content_browsertests.isolate

[Nico, 2015-08-28 19:25:13]

https://codereview.chromium.org/1318143002/diff/20001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/1318143002/diff/20001/chrome/common/chrome_co...
chrome/common/chrome_content_client.cc:100: #if !defined(OS_ANDROID)
Should this be some ENABLE_ define too, based on the argument in the cl
description?

[hshi1, 2015-08-28 20:33:55]

https://codereview.chromium.org/1318143002/diff/20001/chrome/common/chrome_co...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/1318143002/diff/20001/chrome/common/chrome_co...
chrome/common/chrome_content_client.cc:100: #if !defined(OS_ANDROID)
On 2015/08/28 19:25:13, Nico wrote:
> Should this be some ENABLE_ define too, based on the argument in the cl
> description?

In offline discussion bauberb@ suggested to add an ENABLE_PDF macro, so PTAL.

[Nico, 2015-08-28 22:19:33]

lgtm, thanks

[hshi1, 2015-08-28 23:01:40]

The CQ bit was checked by hshi@chromium.org

[hshi1, 2015-08-28 23:01:40]

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/1318143002/#ps60001
(title: "Fix tryjob failures: forgot to set/check ENABLE_PDF in a couple
places.")

[commit-bot: I haz the power, 2015-08-28 23:03:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1318143002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1318143002/60001

[commit-bot: I haz the power, 2015-08-29 00:14:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-08-29 00:15:49]

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