Update PPDFile to PpdReference. Change the fields to a sufficient set to implement QuirksServer lo…

Update PPDFile to PpdReference.

Change the fields to a sufficient set to implement QuirksServer lookup
and user-supplied ppd storage, based on discussions with skau@

BUG=617254
Committed: https://crrev.com/fe693f0b9f01d34449e700222d9258cba4546d77
Cr-Commit-Position: refs/heads/master@{#424618}

[Carlson, 2016-10-11 21:28:12]

justincarlson@chromium.org changed reviewers:
+ skau@chromium.org

[skau, 2016-10-11 21:53:57]

lgtm

[skau, 2016-10-11 21:54:05]

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

[commit-bot: I haz the power, 2016-10-11 21:54:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 22:24:11]

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

[commit-bot: I haz the power, 2016-10-11 22:24:13]

Dry run: This issue passed the CQ dry run.

[Carlson, 2016-10-11 22:57:14]

The CQ bit was checked by justincarlson@chromium.org

[commit-bot: I haz the power, 2016-10-11 22:57:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 22:57:39]

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

[commit-bot: I haz the power, 2016-10-11 22:57:39]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M 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.

[justincarlson, 2016-10-11 23:02:52]

justincarlson@google.com changed reviewers:
+ justincarlson@google.com, thestig@chromium.org

[Lei Zhang, 2016-10-11 23:05:51]

Description was changed from

==========
Update PPDFile to PpdReference.  Change the fields to a sufficient set to
implement QuirksServer lookup and user-supplied ppd storage, based on
discussions with skau@

BUG=617254
==========

to

==========
Update PPDFile to PpdReference.

Change the fields to a sufficient set to implement QuirksServer lookup
and user-supplied ppd storage, based on discussions with skau@

BUG=617254
==========

[Lei Zhang, 2016-10-11 23:13:28]

lgtm

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
File chromeos/printing/printer_configuration.h (right):

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:20: // If non-empty, this is the url
of a specific ppd the user has specified
nit: Can we consistently refer to "PPD" as "PPD" ?

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:70: const PpdReference&
ppd_reference() const { return ppd_reference_; }
Some places do:
const Foo* foo() const;
Foo* foo();

And the compiler will pick the right one when you call foo().

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:72: void set_ppd_reference(const
PpdReference& ppd_reference) {
If you already have mutable_ppd_reference(), can't you just get rid of this and
write the following instead?

PpdReference* p_pointer = foo.mutable_ppd_reference();
*p_pointer = new_p;

[Carlson, 2016-10-11 23:28:02]

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
File chromeos/printing/printer_configuration.h (right):

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:20: // If non-empty, this is the url
of a specific ppd the user has specified
On 2016/10/11 23:13:28, Lei Zhang wrote:
> nit: Can we consistently refer to "PPD" as "PPD" ?

Done.  (I think I got them all?  There are a few instances in other files in
this directory that I didn't touch.)

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:70: const PpdReference&
ppd_reference() const { return ppd_reference_; }
On 2016/10/11 23:13:28, Lei Zhang wrote:
> Some places do:
> const Foo* foo() const;
> Foo* foo();
> 
> And the compiler will pick the right one when you call foo().

I understand the appeal of that approach, but I prefer it this way because

a)  That pattern seems to be a bit questionable w.r.t. C++ style guide function
overloading rules on being able to immediately tell which variant is being
called

b) It's a small thing, but we're going to be passing PpdReferences around in the
code soon, almost certainly as const references, so I'd rather be able to write
 
DoStuff(printer.ppd_reference()) 

instead of

DoStuff(*printer.ppd_reference()); 

Will change it if you feel strongly.

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:72: void set_ppd_reference(const
PpdReference& ppd_reference) {
On 2016/10/11 23:13:28, Lei Zhang wrote:
> If you already have mutable_ppd_reference(), can't you just get rid of this
and
> write the following instead?
> 
> PpdReference* p_pointer = foo.mutable_ppd_reference();
> *p_pointer = new_p;

Done.

[Lei Zhang, 2016-10-11 23:36:24]

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
File chromeos/printing/printer_configuration.h (right):

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:20: // If non-empty, this is the url
of a specific ppd the user has specified
On 2016/10/11 23:28:02, Carlson wrote:
> On 2016/10/11 23:13:28, Lei Zhang wrote:
> > nit: Can we consistently refer to "PPD" as "PPD" ?
> 
> Done.  (I think I got them all?  There are a few instances in other files in
> this directory that I didn't touch.)

No worries if you didn't.

https://codereview.chromium.org/2408293002/diff/1/chromeos/printing/printer_c...
chromeos/printing/printer_configuration.h:70: const PpdReference&
ppd_reference() const { return ppd_reference_; }
On 2016/10/11 23:28:02, Carlson wrote:
> On 2016/10/11 23:13:28, Lei Zhang wrote:
> > Some places do:
> > const Foo* foo() const;
> > Foo* foo();
> > 
> > And the compiler will pick the right one when you call foo().
> 
> I understand the appeal of that approach, but I prefer it this way because

Alrighty.

> a)  That pattern seems to be a bit questionable w.r.t. C++ style guide
function
> overloading rules on being able to immediately tell which variant is being
> called

I don't think its a style guide problem, at least not in Chromium land. It's
used in many places.

[justincarlson, 2016-10-11 23:39:30]

The CQ bit was checked by justincarlson@google.com

[justincarlson, 2016-10-11 23:39:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, skau@chromium.org
Link to the patchset: https://codereview.chromium.org/2408293002/#ps20001
(title: "Update PPDFile to PpdReference.  Change the fields to a sufficient set
to implement QuirksServer lo…")

[commit-bot: I haz the power, 2016-10-11 23:40:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 00:46:06]

Description was changed from

==========
Update PPDFile to PpdReference.

Change the fields to a sufficient set to implement QuirksServer lookup
and user-supplied ppd storage, based on discussions with skau@

BUG=617254
==========

to

==========
Update PPDFile to PpdReference.

Change the fields to a sufficient set to implement QuirksServer lookup
and user-supplied ppd storage, based on discussions with skau@

BUG=617254
==========

[commit-bot: I haz the power, 2016-10-12 00:46:07]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-12 00:48:03]

Description was changed from

==========
Update PPDFile to PpdReference.

Change the fields to a sufficient set to implement QuirksServer lookup
and user-supplied ppd storage, based on discussions with skau@

BUG=617254
==========

to

==========
Update PPDFile to PpdReference.

Change the fields to a sufficient set to implement QuirksServer lookup
and user-supplied ppd storage, based on discussions with skau@

BUG=617254

Committed: https://crrev.com/fe693f0b9f01d34449e700222d9258cba4546d77
Cr-Commit-Position: refs/heads/master@{#424618}
==========

[commit-bot: I haz the power, 2016-10-12 00:48:04]

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