New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 742487 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Refactor CUPS printer management to handle discovery gracefully

Project Member Reported by justincarlson@chromium.org, Jul 13 2017

Issue description

Refactor PrintersManager, detectors, and callsites to accommodate discovery 
 
Blockedon: 744987 744997
Blockedon: 744993 744995 744996
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45339aec0d65279c9be3238138020952a2a1dec3

commit 45339aec0d65279c9be3238138020952a2a1dec3
Author: Justin Carlson <justincarlson@chromium.org>
Date: Mon Jul 17 23:59:10 2017

Rename PrintersManager to SyncedPrintersManager.

Also rename factory class.  This is in preparation for subsuming
some of this functionality into a higher level printers manager.

BUG= 742487 

Change-Id: I4929a4faa6bbdade248c9bdb710a3b53d8ebb262
Reviewed-on: https://chromium-review.googlesource.com/569849
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Anthony Vallee-Dubois <anthonyvd@chromium.org>
Commit-Queue: Justin Carlson <justincarlson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487292}
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/cups_print_job_manager_factory.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/cups_print_job_manager_impl.cc
[delete] https://crrev.com/946720ebebdf76c96cb860c744a69fb12f937eac/chrome/browser/chromeos/printing/printers_manager_factory.h
[rename] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/synced_printers_manager.cc
[rename] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/synced_printers_manager.h
[rename] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/synced_printers_manager_factory.cc
[add] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/synced_printers_manager_factory.h
[rename] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/synced_printers_manager_unittest.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/usb_printer_detector.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/chromeos/printing/usb_printer_detector_factory.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/sync/chrome_sync_client.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/sync/profile_sync_service_factory.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/sync/test/integration/printers_helper.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/sync/test/integration/printers_helper.h
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/ui/webui/print_preview/printer_backend_proxy_chromeos.cc
[modify] https://crrev.com/45339aec0d65279c9be3238138020952a2a1dec3/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df87d8134aaa5b1b19704603f19cbde5b470f051

commit df87d8134aaa5b1b19704603f19cbde5b470f051
Author: Justin Carlson <justincarlson@chromium.org>
Date: Thu Jul 20 17:02:10 2017

Refactor SyncedPrintersManager.

This change brings SyncedPrintersManager closer to PrinterDetector
style interfaces, and aligns function names/styles with the design doc
for CUPS printers management.  It also removes a bunch of unique_ptr
wrapping of printers that are passed around, since these are not at
all performance critical and the unique_ptr stuff hampers readability
(and leads to subtle bugs when unique_ptrs are both used in a bind and
passed as a parameter at the same time).

Also convert it to the factory-function pattern while I'm mucking with
stuff and clean up the header.


Bug:  742487 
Change-Id: Ibc9dd0a2f94dad8cb7a1f4adb1897668800311bf
Reviewed-on: https://chromium-review.googlesource.com/575289
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Commit-Queue: Justin Carlson <justincarlson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488276}
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/chromeos/printing/synced_printers_manager.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/chromeos/printing/synced_printers_manager.h
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/chromeos/printing/synced_printers_manager_factory.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/chromeos/printing/synced_printers_manager_unittest.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/chromeos/printing/usb_printer_detector.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/sync/test/integration/printers_helper.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/sync/test/integration/two_client_printers_sync_test.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/ui/webui/print_preview/printer_backend_proxy_chromeos.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/df87d8134aaa5b1b19704603f19cbde5b470f051/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.h

Comment 6 by skau@chromium.org, Jul 27 2017

Cc: skau@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d9f08573ffcace0f52f3740bdf1dde28b11cccf

commit 4d9f08573ffcace0f52f3740bdf1dde28b11cccf
Author: Sean Kau <skau@chromium.org>
Date: Sat Jul 29 00:29:16 2017

Extend Printing detector interface to pass metadata.

This changes the PrinterDetector interface to pass through additional
metadata about printers in its API.  This will allow us to do automatic
driver searches in users of the Detection results.

Copy from https://chromium-review.googlesource.com/c/580587

Bug:  744996 ,  742487 ,  725739 
Change-Id: I8dc80224d35620c1614a6afb25e187f1ec00489b
Reviewed-on: https://chromium-review.googlesource.com/590586
Commit-Queue: Sean Kau <skau@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490556}
[modify] https://crrev.com/4d9f08573ffcace0f52f3740bdf1dde28b11cccf/chrome/browser/chromeos/printing/combining_printer_detector.cc
[modify] https://crrev.com/4d9f08573ffcace0f52f3740bdf1dde28b11cccf/chrome/browser/chromeos/printing/combining_printer_detector_unittest.cc
[modify] https://crrev.com/4d9f08573ffcace0f52f3740bdf1dde28b11cccf/chrome/browser/chromeos/printing/printer_detector.h
[modify] https://crrev.com/4d9f08573ffcace0f52f3740bdf1dde28b11cccf/chrome/browser/chromeos/printing/usb_printer_detector.cc
[modify] https://crrev.com/4d9f08573ffcace0f52f3740bdf1dde28b11cccf/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/4d9f08573ffcace0f52f3740bdf1dde28b11cccf/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 17 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6152c1f35f944f591fc282e1d27c74d503212a3f

commit 6152c1f35f944f591fc282e1d27c74d503212a3f
Author: Sean Kau <skau@chromium.org>
Date: Thu Aug 17 22:25:33 2017

Extend Printing detector interface to pass metadata.

This changes the PrinterDetector interface to pass through additional
metadata about printers in its API.  This will allow us to do automatic
driver searches in users of the Detection results.

Copy from https://chromium-review.googlesource.com/c/580587

TBR=skau@chromium.org

(cherry picked from commit 4d9f08573ffcace0f52f3740bdf1dde28b11cccf)

Bug:  744996 ,  742487 ,  725739 
Change-Id: I8dc80224d35620c1614a6afb25e187f1ec00489b
Reviewed-on: https://chromium-review.googlesource.com/590586
Commit-Queue: Sean Kau <skau@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490556}
Reviewed-on: https://chromium-review.googlesource.com/619770
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#658}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/6152c1f35f944f591fc282e1d27c74d503212a3f/chrome/browser/chromeos/printing/combining_printer_detector.cc
[modify] https://crrev.com/6152c1f35f944f591fc282e1d27c74d503212a3f/chrome/browser/chromeos/printing/combining_printer_detector_unittest.cc
[modify] https://crrev.com/6152c1f35f944f591fc282e1d27c74d503212a3f/chrome/browser/chromeos/printing/printer_detector.h
[modify] https://crrev.com/6152c1f35f944f591fc282e1d27c74d503212a3f/chrome/browser/chromeos/printing/usb_printer_detector.cc
[modify] https://crrev.com/6152c1f35f944f591fc282e1d27c74d503212a3f/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/6152c1f35f944f591fc282e1d27c74d503212a3f/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.h

Status: Fixed (was: Assigned)

Sign in to add a comment