Only build various flash/pnacl/ppapi code when they are enabled.

Only build various flash/pnacl/ppapi code when they are enabled.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291069

[David Yen, 2014-08-15 22:35:43]

I'm not too familiar with the pepper_flash files, I've added Trung as a
reviewer.

[Lei Zhang, 2014-08-19 19:19:54]

+vtl's chromium account instead

[viettrungluu, 2014-08-19 20:33:58]

It's probably okay, but could you explain why we're doing this? Adding more
ifdefs makes me sad.

[Lei Zhang, 2014-08-19 21:19:15]

On 2014/08/19 20:33:58, viettrungluu wrote:
> It's probably okay, but could you explain why we're doing this? Adding more
> ifdefs makes me sad.

We are building code on Android (and probably iOS) that is unused.

[viettrungluu, 2014-08-19 21:36:34]

https://codereview.chromium.org/471263003/diff/40001/chrome/common/chrome_con...
File chrome/common/chrome_content_client.cc (right):

https://codereview.chromium.org/471263003/diff/40001/chrome/common/chrome_con...
chrome/common/chrome_content_client.cc:105: const char kNaClPluginMimeType[] =
"application/x-nacl";
Where did this part of the change come from? (It doesn't seem to exist anywhere
in the "before" version ... and I'm not qualified to judge if this is right.)

[Lei Zhang, 2014-08-19 21:57:44]

On 2014/08/19 21:36:34, viettrungluu wrote:
>
https://codereview.chromium.org/471263003/diff/40001/chrome/common/chrome_con...
> File chrome/common/chrome_content_client.cc (right):
> 
>
https://codereview.chromium.org/471263003/diff/40001/chrome/common/chrome_con...
> chrome/common/chrome_content_client.cc:105: const char kNaClPluginMimeType[] =
> "application/x-nacl";
> Where did this part of the change come from? (It doesn't seem to exist
anywhere
> in the "before" version ... and I'm not qualified to judge if this is right.)

Bad rebase, if you look at the diff in patch set 2, it made sense. Between patch
sets 2 and 3, the code in question got moved to
components/nacl/common/nacl_constants.h. Onwards to patch set 4.

[viettrungluu, 2014-08-20 16:40:25]

lgtm w/nit

https://codereview.chromium.org/471263003/diff/60001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
(right):

https://codereview.chromium.org/471263003/diff/60001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc:250: 
It's not necessarily obvious to me that other (non-PNaCl) components won't be
added here later ... but I guess we can cross that bridge if/when we get there.
<shrug>

https://codereview.chromium.org/471263003/diff/60001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc:258:
#endif
nit: a "// !defined(DISABLE_NACL)" would be nice, since it's more than a few
lines

[Lei Zhang, 2014-08-20 21:24:10]

https://codereview.chromium.org/471263003/diff/60001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc
(right):

https://codereview.chromium.org/471263003/diff/60001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc:258:
#endif
On 2014/08/20 16:40:25, viettrungluu wrote:
> nit: a "// !defined(DISABLE_NACL)" would be nice, since it's more than a few
> lines

Done.

[Lei Zhang, 2014-08-20 22:00:23]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-20 22:03:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/471263003/100001

[commit-bot: I haz the power, 2014-08-21 00:12:41]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-21 00:16:23]

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

[commit-bot: I haz the power, 2014-08-21 00:16:24]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Lei Zhang, 2014-08-21 00:26:57]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-21 00:27:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/471263003/120001

[commit-bot: I haz the power, 2014-08-21 02:05:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-21 02:08:21]

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

[commit-bot: I haz the power, 2014-08-21 02:08:22]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[Lei Zhang, 2014-08-21 08:47:14]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-21 08:48:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/471263003/120001

[commit-bot: I haz the power, 2014-08-21 14:59:47]

Committed patchset #7 (120001) as 291069

[rlarocque, 2014-08-21 16:59:38]

A revert of this CL (patchset #7) has been created in
https://codereview.chromium.org/492323002/ by rlarocque@chromium.org.

The reason for reverting is: Suspected of breaking the disable_nacl build..