Generate the print messages in components/printing

Generate the print messages in components/printing

Adds a new file to //printing to hold the enums to be included
during the IPC generation. This allows to use them in more than
one generator at the time.

BUG=311308
Committed: https://crrev.com/7cf873680f76a1f6190f64e3819b51dd64e31574
Cr-Commit-Position: refs/heads/master@{#313540}

[dgn, 2015-01-19 18:09:30]

dgn@chromium.org changed reviewers:
+ bauerb@chromium.org, sgurun@chromium.org, vitalybuka@chromium.org

[dgn, 2015-01-19 18:09:31]

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
File components/printing/common/print_messages.h (right):

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
components/printing/common/print_messages.h:24: #undef
PRINTING_PRINTING_PARAM_TRAITS_MACROS_H_
Inspired by: 
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/impo...

[Bernhard Bauer, 2015-01-19 22:34:32]

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
File chrome/common/all_messages.h (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
chrome/common/all_messages.h:19: #include
"components/printing/common/print_messages.h"
I just realized... are you actually going to need these messages outside of
components/printing? Usually we abstract away all of the IPC into the component.
If you only need the messages in the component, we might get away without the
param_traits file?

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
File components/printing/common/printing_message_generator.cc (right):

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
components/printing/common/printing_message_generator.cc:6: #define
IPC_MESSAGE_IMPL
BTW, an alternative to adding printing_message_generator.{cc,h} would be to move
this to print_messages.cc.

[Vitaly Buka (NO REVIEWS), 2015-01-21 00:59:59]

https://codereview.chromium.org/857053002/diff/20001/chrome/common/BUILD.gn
File chrome/common/BUILD.gn (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/BUILD.gn#n...
chrome/common/BUILD.gn:140: "//components/printing/common:printing_common",
I guess printing_common should have printing in deps

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
File chrome/common/all_messages.h (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
chrome/common/all_messages.h:19: #include
"components/printing/common/print_messages.h"
Maybe in future. Now we have code in chrome/ that sends these messages.

On 2015/01/19 22:34:31, Bernhard Bauer wrote:
> I just realized... are you actually going to need these messages outside of
> components/printing? Usually we abstract away all of the IPC into the
component.
> If you only need the messages in the component, we might get away without the
> param_traits file?

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
File components/printing/common/print_messages.h (left):

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
components/printing/common/print_messages.h:89:
IPC_ENUM_TRAITS_MAX_VALUE(printing::MarginType,
do you need to them to separate file?
can we just have one traits here and one in utilities?

[Bernhard Bauer, 2015-01-21 10:13:57]

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
File chrome/common/all_messages.h (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
chrome/common/all_messages.h:19: #include
"components/printing/common/print_messages.h"
On 2015/01/21 00:59:59, Vitaly Buka wrote:
> Maybe in future. Now we have code in chrome/ that sends these messages.

Is that code going to be componentized as well?

[Vitaly Buka (NO REVIEWS), 2015-01-21 16:46:45]

On 2015/01/21 10:13:57, Bernhard Bauer wrote:
>
https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
> File chrome/common/all_messages.h (right):
> 
>
https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
> chrome/common/all_messages.h:19: #include
> "components/printing/common/print_messages.h"
> On 2015/01/21 00:59:59, Vitaly Buka wrote:
> > Maybe in future. Now we have code in chrome/ that sends these messages.
> 
> Is that code going to be componentized as well?

yes

[dgn, 2015-01-21 18:24:24]

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
File components/printing/common/print_messages.h (left):

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
components/printing/common/print_messages.h:89:
IPC_ENUM_TRAITS_MAX_VALUE(printing::MarginType,
On 2015/01/21 00:59:59, Vitaly Buka wrote:
> do you need to them to separate file?
> can we just have one traits here and one in utilities?

Then since they are processed by different generators, the functions are defined
twice.

[Bernhard Bauer, 2015-01-21 18:57:29]

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
File chrome/common/all_messages.h (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
chrome/common/all_messages.h:19: #include
"components/printing/common/print_messages.h"
On 2015/01/21 10:13:57, Bernhard Bauer wrote:
> On 2015/01/21 00:59:59, Vitaly Buka wrote:
> > Maybe in future. Now we have code in chrome/ that sends these messages.
> 
> Is that code going to be componentized as well?

Could we add a TODO then to get rid of the traits file once we only use the
messages in one place?

[dgn, 2015-01-22 00:14:29]

https://codereview.chromium.org/857053002/diff/20001/chrome/common/BUILD.gn
File chrome/common/BUILD.gn (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/BUILD.gn#n...
chrome/common/BUILD.gn:140: "//components/printing/common:printing_common",
On 2015/01/21 00:59:59, Vitaly Buka wrote:
> I guess printing_common should have printing in deps

But there are references to //printing directly, not only through
//components/printing. Isn't it better to make it explicit that we are using it
instead of relying on components to bring it in?

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
File chrome/common/all_messages.h (right):

https://codereview.chromium.org/857053002/diff/20001/chrome/common/all_messag...
chrome/common/all_messages.h:19: #include
"components/printing/common/print_messages.h"
On 2015/01/21 18:57:29, Bernhard Bauer wrote:
> On 2015/01/21 10:13:57, Bernhard Bauer wrote:
> > On 2015/01/21 00:59:59, Vitaly Buka wrote:
> > > Maybe in future. Now we have code in chrome/ that sends these messages.
> > 
> > Is that code going to be componentized as well?
> 
> Could we add a TODO then to get rid of the traits file once we only use the
> messages in one place?

Done.

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
File components/printing/common/printing_message_generator.cc (right):

https://codereview.chromium.org/857053002/diff/20001/components/printing/comm...
components/printing/common/printing_message_generator.cc:6: #define
IPC_MESSAGE_IMPL
On 2015/01/19 22:34:31, Bernhard Bauer wrote:
> BTW, an alternative to adding printing_message_generator.{cc,h} would be to
move
> this to print_messages.cc.

Done.

[Bernhard Bauer, 2015-01-22 09:30:12]

https://codereview.chromium.org/857053002/diff/40001/components/printing/comm...
File components/printing/common/print_messages.cc (right):

https://codereview.chromium.org/857053002/diff/40001/components/printing/comm...
components/printing/common/print_messages.cc:5: //#include
"components/printing/common/print_messages.h"
Nit: remove this comment.

https://codereview.chromium.org/857053002/diff/40001/components/printing/comm...
components/printing/common/print_messages.cc:7: #define IPC_MESSAGE_IMPL
Nit: I think I would move the generators to the bottom of the file, or at least
after the regular includes, to make it closer to the usual style.

[dgn, 2015-01-22 14:27:14]

https://codereview.chromium.org/857053002/diff/40001/components/printing/comm...
File components/printing/common/print_messages.cc (right):

https://codereview.chromium.org/857053002/diff/40001/components/printing/comm...
components/printing/common/print_messages.cc:5: //#include
"components/printing/common/print_messages.h"
On 2015/01/22 09:30:12, Bernhard Bauer wrote:
> Nit: remove this comment.

Done.

https://codereview.chromium.org/857053002/diff/40001/components/printing/comm...
components/printing/common/print_messages.cc:7: #define IPC_MESSAGE_IMPL
On 2015/01/22 09:30:12, Bernhard Bauer wrote:
> Nit: I think I would move the generators to the bottom of the file, or at
least
> after the regular includes, to make it closer to the usual style.

Done. 

Both styles seem to be used
(https://code.google.com/p/chromium/codesearch#search/&q=f:cc%20ipc/param_trai...

They 'look' like includes so it seems to me that they belong with the other
#include statements.

[dgn, 2015-01-23 16:47:56]

dgn@chromium.org changed reviewers:
+ dcheng@chromium.org, thestig@chromium.org

[dgn, 2015-01-23 16:47:56]

dcheng@chromium.org: Please review changes in 
chrome/common/all_messages.h 
chrome/common/chrome_utility_printing_messages.h 
components/printing/common/print_messages.h

thestig@chromium.org: Please review changes in 
chrome/common/BUILD.gn 
chrome/common/common_message_generator.h

[sgurun-gerrit only, 2015-01-23 22:13:40]

On 2015/01/23 16:47:56, dgn wrote:
> mailto:dcheng@chromium.org: Please review changes in 
> chrome/common/all_messages.h 
> chrome/common/chrome_utility_printing_messages.h 
> components/printing/common/print_messages.h
> 
> mailto:thestig@chromium.org: Please review changes in 
> chrome/common/BUILD.gn 
> chrome/common/common_message_generator.h

looks good to me (but not an owner on any file :)

[dcheng, 2015-01-23 23:01:53]

https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
File printing/printing_param_traits_macros.h (right):

https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
printing/printing_param_traits_macros.h:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
Please add an OWNERS file and add set noparent rules for IPC reviewers for this
file.

[Vitaly Buka (NO REVIEWS), 2015-01-23 23:29:36]

lgtm

[dgn, 2015-01-26 10:02:09]

https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
File printing/printing_param_traits_macros.h (right):

https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
printing/printing_param_traits_macros.h:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2015/01/23 23:01:53, dcheng wrote:
> Please add an OWNERS file and add set noparent rules for IPC reviewers for
this
> file.

Done.

[Vitaly Buka (NO REVIEWS), 2015-01-27 01:45:02]

On 2015/01/26 10:02:09, dgn wrote:
>
https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
> File printing/printing_param_traits_macros.h (right):
> 
>
https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
> printing/printing_param_traits_macros.h:1: // Copyright 2015 The Chromium
> Authors. All rights reserved.
> On 2015/01/23 23:01:53, dcheng wrote:
> > Please add an OWNERS file and add set noparent rules for IPC reviewers for
> this
> > file.
> 
> Done.

Better to avoid introducing IPC into printing/

could you please move printing_param_traits_macros.h and printing/OWNERS changes
into components/printing/common?

[dgn, 2015-01-27 16:48:30]

On 2015/01/27 01:45:02, Vitaly Buka wrote:
> On 2015/01/26 10:02:09, dgn wrote:
> >
>
https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
> > File printing/printing_param_traits_macros.h (right):
> > 
> >
>
https://codereview.chromium.org/857053002/diff/60001/printing/printing_param_...
> > printing/printing_param_traits_macros.h:1: // Copyright 2015 The Chromium
> > Authors. All rights reserved.
> > On 2015/01/23 23:01:53, dcheng wrote:
> > > Please add an OWNERS file and add set noparent rules for IPC reviewers for
> > this
> > > file.
> > 
> > Done.
> 
> Better to avoid introducing IPC into printing/
> 
> could you please move printing_param_traits_macros.h and printing/OWNERS
changes
> into components/printing/common?

Done

[Vitaly Buka (NO REVIEWS), 2015-01-27 17:24:19]

lgtm components/printing/

[dgn, 2015-01-27 18:07:51]

Thanks! :)

dcheng@chromium.org: Please review changes in 
chrome/common/all_messages.h 
chrome/common/chrome_utility_printing_messages.h 
components/printing/common/print_messages.h

thestig@chromium.org: Please review changes in 
chrome/common/BUILD.gn 
chrome/common/common_message_generator.h

[Vitaly Buka (NO REVIEWS), 2015-01-27 18:22:28]

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
File components/printing/common/print_messages.h (right):

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
components/printing/common/print_messages.h:13: #include
"ipc/ipc_message_macros.h"
order

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
File components/printing/common/printing_param_traits_macros.h (right):

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
components/printing/common/printing_param_traits_macros.h:12: #include
"ipc/ipc_message_macros.h"
order

[dgn, 2015-01-27 18:28:17]

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
File components/printing/common/print_messages.h (right):

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
components/printing/common/print_messages.h:13: #include
"ipc/ipc_message_macros.h"
On 2015/01/27 18:22:27, Vitaly Buka wrote:
> order

Done.

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
File components/printing/common/printing_param_traits_macros.h (right):

https://codereview.chromium.org/857053002/diff/100001/components/printing/com...
components/printing/common/printing_param_traits_macros.h:12: #include
"ipc/ipc_message_macros.h"
On 2015/01/27 18:22:27, Vitaly Buka wrote:
> order

Done.

[dcheng, 2015-01-27 18:36:58]

https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
File components/printing/common/printing_param_traits_macros.h (right):

https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
components/printing/common/printing_param_traits_macros.h:1: // Copyright 2015
The Chromium Authors. All rights reserved.
Mind updating OWNERS to cover this file as well?

Yes, this is a huge pain =(

[Vitaly Buka (NO REVIEWS), 2015-01-27 18:45:40]

https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
File components/printing/common/printing_param_traits_macros.h (right):

https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
components/printing/common/printing_param_traits_macros.h:1: // Copyright 2015
The Chromium Authors. All rights reserved.
On 2015/01/27 18:36:57, dcheng wrote:
> Mind updating OWNERS to cover this file as well?
> 
> Yes, this is a huge pain =(

it's already there

[dcheng, 2015-01-27 18:46:43]

On 2015/01/27 at 18:45:40, vitalybuka wrote:
>
https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
> File components/printing/common/printing_param_traits_macros.h (right):
> 
>
https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
> components/printing/common/printing_param_traits_macros.h:1: // Copyright 2015
The Chromium Authors. All rights reserved.
> On 2015/01/27 18:36:57, dcheng wrote:
> > Mind updating OWNERS to cover this file as well?
> > 
> > Yes, this is a huge pain =(
> 
> it's already there

Oops, good point. lgtm

[Lei Zhang, 2015-01-27 19:57:18]

On 2015/01/27 18:46:43, dcheng wrote:
> On 2015/01/27 at 18:45:40, vitalybuka wrote:
> >
>
https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
> > File components/printing/common/printing_param_traits_macros.h (right):
> > 
> >
>
https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
> > components/printing/common/printing_param_traits_macros.h:1: // Copyright
2015
> The Chromium Authors. All rights reserved.
> > On 2015/01/27 18:36:57, dcheng wrote:
> > > Mind updating OWNERS to cover this file as well?
> > > 
> > > Yes, this is a huge pain =(
> > 
> > it's already there
> 
> Oops, good point. lgtm

Presumably your l-g cover chrome/common/common_message_generator.h, since
#include "components/printing/common/print_messages.h" is being moved from there
to all_messages.h?

[dcheng, 2015-01-28 04:45:07]

On 2015/01/27 at 19:57:18, thestig wrote:
> On 2015/01/27 18:46:43, dcheng wrote:
> > On 2015/01/27 at 18:45:40, vitalybuka wrote:
> > >
> >
https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
> > > File components/printing/common/printing_param_traits_macros.h (right):
> > > 
> > >
> >
https://codereview.chromium.org/857053002/diff/120001/components/printing/com...
> > > components/printing/common/printing_param_traits_macros.h:1: // Copyright
2015
> > The Chromium Authors. All rights reserved.
> > > On 2015/01/27 18:36:57, dcheng wrote:
> > > > Mind updating OWNERS to cover this file as well?
> > > > 
> > > > Yes, this is a huge pain =(
> > > 
> > > it's already there
> > 
> > Oops, good point. lgtm
> 
> Presumably your l-g cover chrome/common/common_message_generator.h, since
#include "components/printing/common/print_messages.h" is being moved from there
to all_messages.h?

Yes, all the ipc changes are fine since it's just moving stuff around.

[Lei Zhang, 2015-01-28 17:37:17]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2015-01-28 17:37:18]

lgtm

[commit-bot: I haz the power, 2015-01-28 17:38:37]

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

[commit-bot: I haz the power, 2015-01-28 17:44:43]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-28 17:46:27]

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