Disable basic printing on ChromeOS.

Disable basic printing on ChromeOS.
It's actually a printing with Cloud Print Web dialog.

BUG=408684, 158799
Committed: https://crrev.com/e6f4a714b83dc7d8187b3f93d41a9a1d590a42d3
Cr-Commit-Position: refs/heads/master@{#302959}

[Vitaly Buka (NO REVIEWS), 2014-10-30 23:16:25]

vitalybuka@chromium.org changed reviewers:
+ alekseys@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-10-30 23:17:29]

Patchset #2 (id:20001) has been deleted

[Aleksey Shlyapnikov, 2014-10-30 23:25:24]

The CQ bit was checked by alekseys@chromium.org

[Aleksey Shlyapnikov, 2014-10-30 23:25:24]

lgtm

[commit-bot: I haz the power, 2014-10-30 23:26:48]

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

[commit-bot: I haz the power, 2014-10-30 23:26:49]

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

[commit-bot: I haz the power, 2014-10-30 23:26:50]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Vitaly Buka (NO REVIEWS), 2014-10-31 16:34:07]

The CQ bit was checked by vitalybuka@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-10-31 16:34:11]

The CQ bit was unchecked by vitalybuka@chromium.org

[Vitaly Buka (NO REVIEWS), 2014-10-31 17:05:37]

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

[Lei Zhang, 2014-10-31 18:24:38]

lgtm

https://codereview.chromium.org/694633002/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/694633002/diff/40001/build/common.gypi#newcod...
build/common.gypi:2920: 'defines': ['DISABLE_BASIC_PRINTING=1'],
You probably want to add this to the GN version as well.

I didn't foresee the need do disable basic printing, thus we have
ENABLE_FULL_PRINTING + DISABLE_BASIC_PRINTING now. In the long run, we may want
to reorganize our ifdefs into all of the enable form:
- ENABLE_BASIC_PRINTING
- ENABLE_PRINT_PREVIEW
- ENABLE_PRINTING (really just ENABLE_BASIC_PRINTING || ENABLE_PRINT_PREVIEW)

[Vitaly Buka (NO REVIEWS), 2014-10-31 19:09:49]

https://codereview.chromium.org/694633002/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/694633002/diff/40001/build/common.gypi#newcod...
build/common.gypi:2920: 'defines': ['DISABLE_BASIC_PRINTING=1'],
On 2014/10/31 18:24:38, Lei Zhang wrote:
> You probably want to add this to the GN version as well.

Done.

> 
> I didn't foresee the need do disable basic printing, thus we have
> ENABLE_FULL_PRINTING + DISABLE_BASIC_PRINTING now. In the long run, we may
want
> to reorganize our ifdefs into all of the enable form:
> - ENABLE_BASIC_PRINTING
> - ENABLE_PRINT_PREVIEW
> - ENABLE_PRINTING (really just ENABLE_BASIC_PRINTING || ENABLE_PRINT_PREVIEW)

Agree, I also thought about that.

[Vitaly Buka (NO REVIEWS), 2014-10-31 19:09:52]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-10-31 19:11:05]

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

[commit-bot: I haz the power, 2014-10-31 19:17:03]

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

[commit-bot: I haz the power, 2014-10-31 19:17:04]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Vitaly Buka (NO REVIEWS), 2014-10-31 19:29:01]

vitalybuka@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2014-11-04 21:16:54]

I don't think this state of printing defines is reasonable. It was already in a
super confusing state that tripped people up and everybody thought needed
refactoring, and now we're adding even more stuff. I don't think we should make
the state worse and I think we need to make things better.

I think we should rename the existing ones first or at the same time. Really
this is an easy global-search-and-replace, although it will be annoying to land.

enable_printing GYP variable is super confusing since it's a tri-state but named
like a bool. This should be printing_mode like in GN. Then we should do the
defines like Lei said.

[Vitaly Buka (NO REVIEWS), 2014-11-05 11:35:04]

On 2014/11/04 21:16:54, brettw wrote:
> I don't think this state of printing defines is reasonable. It was already in
a
> super confusing state that tripped people up and everybody thought needed
> refactoring, and now we're adding even more stuff. I don't think we should
make
> the state worse and I think we need to make things better.
> 
> I think we should rename the existing ones first or at the same time. Really
> this is an easy global-search-and-replace, although it will be annoying to
land.
> 
> enable_printing GYP variable is super confusing since it's a tri-state but
named
> like a bool. This should be printing_mode like in GN. Then we should do the
> defines like Lei said.

Done.

[brettw, 2014-11-05 21:58:59]

lgtm

[Vitaly Buka (NO REVIEWS), 2014-11-05 23:48:02]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-11-05 23:49:40]

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

[commit-bot: I haz the power, 2014-11-06 01:51:29]

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

[commit-bot: I haz the power, 2014-11-06 01:51:30]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Vitaly Buka (NO REVIEWS), 2014-11-06 02:59:10]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-11-06 03:00:32]

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

[commit-bot: I haz the power, 2014-11-06 04:02:51]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2014-11-06 04:03:21]

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