Track printer installations for each configuration.

Track printer installations for each configuration.

Track which printers have been configured in the system by tracking
the timestamp for their most recently installed configuration.  This
prevents reinstalling printers which can cause print queues to restart.

BUG=713916
Review-Url: https://codereview.chromium.org/2858353004
Cr-Commit-Position: refs/heads/master@{#470686}
Committed: https://chromium.googlesource.com/chromium/src/+/d571f3812f6e6031ce0da38c6a667cbabfd31e36

[skau, 2017-05-05 23:47:31]

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

[Carlson, 2017-05-05 23:59:16]

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.cc (right):

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.cc:203:
recommended_printer_ids_.push_back(id);
Does this mean we're happy to push the same printer into
recommended_printer_ids_ multiple times?

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:100: // Map of printer ids
to installation timestpamps.
timestamps

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:101: std::map<std::string,
int64_t> installed_printer_timestamps_;
why int64_t instead of base::Time?

https://codereview.chromium.org/2858353004/diff/20001/chromeos/printing/print...
File chromeos/printing/printer_translator.cc (right):

https://codereview.chromium.org/2858353004/diff/20001/chromeos/printing/print...
chromeos/printing/printer_translator.cc:37: bool DictionaryToPrinter(const
DictionaryValue& value, Printer* printer) {
I don't really understand why you wanted to make this change, but I'm not
opposed to it.

https://codereview.chromium.org/2858353004/diff/20001/chromeos/printing/print...
chromeos/printing/printer_translator.cc:72: return printer;
Change this to return true?  Seems to be more in the spirit of the rest of the
change.

[skau, 2017-05-08 18:38:02]

Comments addressed.  PTAL.

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.cc (right):

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.cc:203:
recommended_printer_ids_.push_back(id);
On 2017/05/05 23:59:16, Carlson wrote:
> Does this mean we're happy to push the same printer into
> recommended_printer_ids_ multiple times?

It's cleared on line 179.  We save the ids to keep the order stable.

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:100: // Map of printer ids
to installation timestpamps.
On 2017/05/05 23:59:16, Carlson wrote:
> timestamps

Done.

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:101: std::map<std::string,
int64_t> installed_printer_timestamps_;
On 2017/05/05 23:59:16, Carlson wrote:
> why int64_t instead of base::Time?

I didn't think about it. It's time all the way down now.

https://codereview.chromium.org/2858353004/diff/20001/chromeos/printing/print...
File chromeos/printing/printer_translator.cc (right):

https://codereview.chromium.org/2858353004/diff/20001/chromeos/printing/print...
chromeos/printing/printer_translator.cc:37: bool DictionaryToPrinter(const
DictionaryValue& value, Printer* printer) {
On 2017/05/05 23:59:16, Carlson wrote:
> I don't really understand why you wanted to make this change, but I'm not
> opposed to it.

I thought it was clearer to instantiate the printer with the timestamp instead
of passing it as an argument.

https://codereview.chromium.org/2858353004/diff/20001/chromeos/printing/print...
chromeos/printing/printer_translator.cc:72: return printer;
On 2017/05/05 23:59:16, Carlson wrote:
> Change this to return true?  Seems to be more in the spirit of the rest of the
> change.

Done.

[skau, 2017-05-08 18:38:36]

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

[commit-bot: I haz the power, 2017-05-08 18:39:31]

Dry run: CQ is trying da patch.

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

[skau, 2017-05-08 18:45:47]

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

[skau, 2017-05-08 18:45:48]

dpapad@: OWNER of chrome/browser/ui/webui/*

xdai@: fyi

[Carlson, 2017-05-08 18:49:26]

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.cc (right):

https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.cc:203:
recommended_printer_ids_.push_back(id);
On 2017/05/08 18:38:02, skau wrote:
> On 2017/05/05 23:59:16, Carlson wrote:
> > Does this mean we're happy to push the same printer into
> > recommended_printer_ids_ multiple times?
> 
> It's cleared on line 179.  We save the ids to keep the order stable.

I'm still a little confused by this, sorry.

In this function, we *clear* the existing recommended_printer_ids_, but for
recommended_printers_ we selectively add the printers we haven't seen before.

I don't understand why the two structures are treated differently in this way. 
The contract in printers_manager.h says recommended_printer_ids_ contains the
keys for all recommended_printers_, and either that's not guaranteed with this
code, or   I don't understand something about how it works.

[commit-bot: I haz the power, 2017-05-08 19:08:34]

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

[commit-bot: I haz the power, 2017-05-08 19:08:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skau, 2017-05-08 20:11:30]

On 2017/05/08 18:49:26, Carlson wrote:
>
https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
> File chrome/browser/chromeos/printing/printers_manager.cc (right):
> 
>
https://codereview.chromium.org/2858353004/diff/20001/chrome/browser/chromeos...
> chrome/browser/chromeos/printing/printers_manager.cc:203:
> recommended_printer_ids_.push_back(id);
> On 2017/05/08 18:38:02, skau wrote:
> > On 2017/05/05 23:59:16, Carlson wrote:
> > > Does this mean we're happy to push the same printer into
> > > recommended_printer_ids_ multiple times?
> > 
> > It's cleared on line 179.  We save the ids to keep the order stable.
> 
> I'm still a little confused by this, sorry.
> 
> In this function, we *clear* the existing recommended_printer_ids_, but for
> recommended_printers_ we selectively add the printers we haven't seen before.
> 
> I don't understand why the two structures are treated differently in this way.

> The contract in printers_manager.h says recommended_printer_ids_ contains the
> keys for all recommended_printers_, and either that's not guaranteed with this
> code, or   I don't understand something about how it works.

Thanks.  There's actually a bug.  We don't want to touch printers that haven't
changed but the code currently leaves printers that have been removed.  So,
there could be more printers in the map than in the vector.  I've fixed this by
parsing into a new map then swapping the contents after the fact.

[dpapad, 2017-05-08 20:24:07]

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

[dpapad, 2017-05-08 20:24:08]

@thestig: Could you take a look at the changes in
chrome/browser/ui/webui/print_preview/printer_backend_proxy_chromeos.cc?

[Carlson, 2017-05-08 21:01:17]

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.cc:208: if (old !=
recommended_printers_.end()) {
Generally looks fine, but back to my original comment, I think this behaves
badly if we have multiple entries for the same printer.  We'll end up moving the
existing entry into the new map twice, which will cause the entry in
new_printers to be null.

You could just erase the entry from the existing map after you move it to avoid
this, or do something else, but I think it's worth taking care of this.

[skau, 2017-05-08 21:19:25]

Added a guard for duplicated printer ids.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.cc:208: if (old !=
recommended_printers_.end()) {
On 2017/05/08 21:01:17, Carlson wrote:
> Generally looks fine, but back to my original comment, I think this behaves
> badly if we have multiple entries for the same printer.  We'll end up moving
the
> existing entry into the new map twice, which will cause the entry in
> new_printers to be null.
> 
> You could just erase the entry from the existing map after you move it to
avoid
> this, or do something else, but I think it's worth taking care of this.
We expect all printer entries to be unique within the collection.  But I'll skip
duplicates to avoid this scenario.

[Carlson, 2017-05-08 21:31:00]

lgtm

[skau, 2017-05-08 21:37:34]

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

[commit-bot: I haz the power, 2017-05-08 21:38:14]

Dry run: CQ is trying da patch.

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

[Lei Zhang, 2017-05-08 21:42:48]

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:58: // Adds or updates a
printer. Printers are identified by the id field.  Use an
BTW, this comment looks out of date.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:79: // Registers that the
printer was installed in CUPS.
One of the PrinterInstalled() callers is as follows:

manager->PrinterInstalled(*printer);
manager->RegisterPrinter(std::move(printer));

Can you better explain the difference between PrinterInstalled(), which
"registers the printer" in some way, vs RegisterPrinter()?

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:83: bool
IsConfigurationCurrent(const Printer& printer);
const method?

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_configuration.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_configuration.cc:13: Printer::Printer() :
source_(SRC_USER_PREFS), last_updated_() {
No need to initialize |last_updated_| here.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_configuration.h (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_configuration.h:51: explicit Printer(const
std::string& id, const base::Time& last_updated = {});
No longer explict.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_configuration.h:51: explicit Printer(const
std::string& id, const base::Time& last_updated = {});
Does the second parameter have to be optional? I see many of the calls to this
constructor have been updated to pass in the second argument. Are there any
callers left that don't? The DCHECK in RecommendedPrinterToPrinter() makes me
wonder.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_translator.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_translator.cc:35: // Converts |value| into a Printer
object for the fields that are shared
Can you adjust the comment, since this function is no longer creating the
Printer object?

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_translator_unittest.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_translator_unittest.cc:27: const base::Time kTimestamp
= base::Time::FromJavaTime(445566);
Can we leave FromJavaTime() for when you actually are interacting with Java? If
the values are being (de)serialized, {From,To}InternalValue() are the preferred
way of doing that.

I realized this is because PrintersSyncBridge::AddPrinter() from an earlier CL
is use ToJavaTime(). Is it to late to change this?

[commit-bot: I haz the power, 2017-05-08 22:28:41]

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

[commit-bot: I haz the power, 2017-05-08 22:28:42]

Dry run: This issue passed the CQ dry run.

[skau, 2017-05-09 00:52:01]

Addressed comments.  Some replies, some code changes.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:58: // Adds or updates a
printer. Printers are identified by the id field.  Use an
On 2017/05/08 21:42:48, Lei Zhang wrote:
> BTW, this comment looks out of date.

Done.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:79: // Registers that the
printer was installed in CUPS.
On 2017/05/08 21:42:48, Lei Zhang wrote:
> One of the PrinterInstalled() callers is as follows:
> 
> manager->PrinterInstalled(*printer);
> manager->RegisterPrinter(std::move(printer));
> 
> Can you better explain the difference between PrinterInstalled(), which
> "registers the printer" in some way, vs RegisterPrinter()?

I've tried to clarify the comment on RegisterPrinter to distinguish between the
two methods.  Let me know if you think it can be clearer.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:83: bool
IsConfigurationCurrent(const Printer& printer);
On 2017/05/08 21:42:48, Lei Zhang wrote:
> const method?

Done.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_configuration.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_configuration.cc:13: Printer::Printer() :
source_(SRC_USER_PREFS), last_updated_() {
On 2017/05/08 21:42:48, Lei Zhang wrote:
> No need to initialize |last_updated_| here.

Done.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_configuration.h (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_configuration.h:51: explicit Printer(const
std::string& id, const base::Time& last_updated = {});
On 2017/05/08 21:42:48, Lei Zhang wrote:
> Does the second parameter have to be optional? I see many of the calls to this
> constructor have been updated to pass in the second argument. Are there any
> callers left that don't? The DCHECK in RecommendedPrinterToPrinter() makes me
> wonder.

There are a number of instances where we don't provide the last_updated time as
we want that to be determined by the PrintersManager so it reflects sync data
accurately.

Since there is only one required argument, the linter recommends that the
constructor is still explicit.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_translator.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_translator.cc:35: // Converts |value| into a Printer
object for the fields that are shared
On 2017/05/08 21:42:48, Lei Zhang wrote:
> Can you adjust the comment, since this function is no longer creating the
> Printer object?

Done.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_translator_unittest.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_translator_unittest.cc:27: const base::Time kTimestamp
= base::Time::FromJavaTime(445566);
On 2017/05/08 21:42:48, Lei Zhang wrote:
> Can we leave FromJavaTime() for when you actually are interacting with Java?
If
> the values are being (de)serialized, {From,To}InternalValue() are the
preferred
> way of doing that.
> 
> I realized this is because PrintersSyncBridge::AddPrinter() from an earlier CL
> is use ToJavaTime(). Is it to late to change this?

FromInternalValue generates a TimeDelta.  FromJavaTime is anchored at the Unix
epoch.  Is there a different method that's typically used to convert between
Time and int64_t?

[Lei Zhang, 2017-05-09 01:14:40]

Just want to iron out the Java time thing, otherwise looks good.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:58: // Adds or updates a
printer. Printers are identified by the id field.  Use an
On 2017/05/09 00:52:01, skau wrote:
> On 2017/05/08 21:42:48, Lei Zhang wrote:
> > BTW, this comment looks out of date.
> 
> Done.

Err, what I mean was this felt out of date because there was no "id" parameter
to RegisterPrinter(). Maybe this can be:

The |printer| is identified by its id field. Those with an empty id are treated
as new printers.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:79: // Registers that the
printer was installed in CUPS.
On 2017/05/09 00:52:01, skau wrote:
> On 2017/05/08 21:42:48, Lei Zhang wrote:
> > One of the PrinterInstalled() callers is as follows:
> > 
> > manager->PrinterInstalled(*printer);
> > manager->RegisterPrinter(std::move(printer));
> > 
> > Can you better explain the difference between PrinterInstalled(), which
> > "registers the printer" in some way, vs RegisterPrinter()?
> 
> I've tried to clarify the comment on RegisterPrinter to distinguish between
the
> two methods.  Let me know if you think it can be clearer.

Yes, definitely clearer.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_translator_unittest.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_translator_unittest.cc:27: const base::Time kTimestamp
= base::Time::FromJavaTime(445566);
On 2017/05/09 00:52:01, skau wrote:
> On 2017/05/08 21:42:48, Lei Zhang wrote:
> > Can we leave FromJavaTime() for when you actually are interacting with Java?
> If
> > the values are being (de)serialized, {From,To}InternalValue() are the
> preferred
> > way of doing that.
> > 
> > I realized this is because PrintersSyncBridge::AddPrinter() from an earlier
CL
> > is use ToJavaTime(). Is it to late to change this?
> 
> FromInternalValue generates a TimeDelta.  FromJavaTime is anchored at the Unix
> epoch.  Is there a different method that's typically used to convert between
> Time and int64_t?

I think you are looking at base::TimeDelta::FromInternalValue(). There's also
base::TimeBase::FromInternalValue(), which base::Time derives from.

https://codereview.chromium.org/2858353004/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/printing/printers_manager.h:58: // Add or update a
printer in profile preferences.  Printers are identified by
BTW, most of the other comments say "Returns" so keep this as Adds?

https://codereview.chromium.org/2858353004/diff/120001/chromeos/printing/prin...
File chromeos/printing/printer_translator.cc (right):

https://codereview.chromium.org/2858353004/diff/120001/chromeos/printing/prin...
chromeos/printing/printer_translator.cc:35: // Populates the |printer| object
with cooresponding fields from |value|.
corresponding

[skau, 2017-05-09 19:39:27]

Comments addressed.  IRT java time, I'm going to change the representation in
the proto in a different CL as I need to verify that it won't break anything.

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/printing/printers_manager.h:58: // Adds or updates a
printer. Printers are identified by the id field.  Use an
On 2017/05/09 01:14:39, Lei Zhang wrote:
> On 2017/05/09 00:52:01, skau wrote:
> > On 2017/05/08 21:42:48, Lei Zhang wrote:
> > > BTW, this comment looks out of date.
> > 
> > Done.
> 
> Err, what I mean was this felt out of date because there was no "id" parameter
> to RegisterPrinter(). Maybe this can be:
> 
> The |printer| is identified by its id field. Those with an empty id are
treated
> as new printers.

Thanks.  That phrasing is clearer.

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
File chromeos/printing/printer_translator_unittest.cc (right):

https://codereview.chromium.org/2858353004/diff/80001/chromeos/printing/print...
chromeos/printing/printer_translator_unittest.cc:27: const base::Time kTimestamp
= base::Time::FromJavaTime(445566);
On 2017/05/09 01:14:39, Lei Zhang wrote:
> On 2017/05/09 00:52:01, skau wrote:
> > On 2017/05/08 21:42:48, Lei Zhang wrote:
> > > Can we leave FromJavaTime() for when you actually are interacting with
Java?
> > If
> > > the values are being (de)serialized, {From,To}InternalValue() are the
> > preferred
> > > way of doing that.
> > > 
> > > I realized this is because PrintersSyncBridge::AddPrinter() from an
earlier
> CL
> > > is use ToJavaTime(). Is it to late to change this?
> > 
> > FromInternalValue generates a TimeDelta.  FromJavaTime is anchored at the
Unix
> > epoch.  Is there a different method that's typically used to convert between
> > Time and int64_t?
> 
> I think you are looking at base::TimeDelta::FromInternalValue(). There's also
> base::TimeBase::FromInternalValue(), which base::Time derives from.

Thanks.  I couldn't find it in the header.  It looks like I can migrate from
java time to internal time since internal time is larger.  However, I'm going to
do it in a separate CL.  This will be okay since we only deal with the integer
when converting to/from the proto.

https://codereview.chromium.org/2858353004/diff/120001/chrome/browser/chromeo...
File chrome/browser/chromeos/printing/printers_manager.h (right):

https://codereview.chromium.org/2858353004/diff/120001/chrome/browser/chromeo...
chrome/browser/chromeos/printing/printers_manager.h:58: // Add or update a
printer in profile preferences.  Printers are identified by
On 2017/05/09 01:14:39, Lei Zhang wrote:
> BTW, most of the other comments say "Returns" so keep this as Adds?

Done.

https://codereview.chromium.org/2858353004/diff/120001/chromeos/printing/prin...
File chromeos/printing/printer_translator.cc (right):

https://codereview.chromium.org/2858353004/diff/120001/chromeos/printing/prin...
chromeos/printing/printer_translator.cc:35: // Populates the |printer| object
with cooresponding fields from |value|.
On 2017/05/09 01:14:39, Lei Zhang wrote:
> corresponding

Done.

[Lei Zhang, 2017-05-09 20:23:44]

lgtm

[skau, 2017-05-09 20:24:58]

The CQ bit was checked by skau@chromium.org

[skau, 2017-05-09 20:24:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
justincarlson@chromium.org
Link to the patchset: https://codereview.chromium.org/2858353004/#ps160001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-09 20:25:35]

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, 2017-05-09 22:35:43]

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

[commit-bot: I haz the power, 2017-05-09 22:35:44]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[skau, 2017-05-09 22:48:56]

The CQ bit was checked by skau@chromium.org

[commit-bot: I haz the power, 2017-05-09 22:50:27]

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, 2017-05-10 00:53:10]

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

[commit-bot: I haz the power, 2017-05-10 00:53:13]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does
not exist (misspelled?))

[dpapad, 2017-05-10 17:50:55]

dpapad@chromium.org changed reviewers:
- dpapad@chromium.org

[skau, 2017-05-10 18:42:27]

The CQ bit was checked by skau@chromium.org

[commit-bot: I haz the power, 2017-05-10 18:43:46]

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, 2017-05-10 20:27:54]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1494441747294580,
"parent_rev": "648090c24cfe95b072fe2099ffd4af9bd04aa724", "commit_rev":
"d571f3812f6e6031ce0da38c6a667cbabfd31e36"}

[commit-bot: I haz the power, 2017-05-10 20:28:07]

Description was changed from

==========
Track printer installations for each configuration.

Track which printers have been configured in the system by tracking
the timestamp for their most recently installed configuration.  This
prevents reinstalling printers which can cause print queues to restart.

BUG=713916
==========

to

==========
Track printer installations for each configuration.

Track which printers have been configured in the system by tracking
the timestamp for their most recently installed configuration.  This
prevents reinstalling printers which can cause print queues to restart.

BUG=713916

Review-Url: https://codereview.chromium.org/2858353004
Cr-Commit-Position: refs/heads/master@{#470686}
Committed:
https://chromium.googlesource.com/chromium/src/+/d571f3812f6e6031ce0da38c6a66...
==========

[commit-bot: I haz the power, 2017-05-10 20:28:09]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/d571f3812f6e6031ce0da38c6a66...