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

Issue 757887 link

Starred by 2 users
Status: WontFix
Owner:
valleau@chromium.org
Closed: Jan 2018
Cc:
█ justincarlson@chromium.org
skau@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Make all observer callbacks in printing subsystem two-step

Project Member Reported by justincarlson@chromium.org, Aug 22 2017 
WE've run into a large number of initialization-error bugs in the observer subsystems in printing.  Fixing them means having subtle dependency ordering in constructors (e.g. make sure Foo is initialized before Bar is created because Bar may immediately callback into this object and use Foo).

We should convert all Observer usage to be two-step, e.g. don't start issuing observer callbacks on construction, do it after a Start() function is called or similar.

Comment 1 by skau@chromium.org, Sep 19 2017

Cc: justincarlson@chromium.org skau@chromium.org
Owner: valleau@chromium.org
We should probably add tests for this to ensure we don't break anything.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 6 2017 

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

commit 0aa24b56e2aab22f820c2f7d218bc923bfb19381
Author: David Valleau <valleau@chromium.org>
Date: Fri Oct 06 21:58:42 2017

Changed the observer system in CUPS printers manager to 2 step process.

The observes in the Usb and Zeroconf printer detectors have been changed to a
two-step process where observers are first added and then must be started in
order for callback to begin being registered. A new unit test was added to test
this behaviour.

R=dpapad@chromium.org, justincarlson@chromium.org, skau@chromium.org

Bug:  757887 ,  761536 

Change-Id: Ie47876c13609c3528ab23a8fca11057ceb7085e7
Reviewed-on: https://chromium-review.googlesource.com/682958
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: David Valleau <valleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507186}
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/cups_printers_manager.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/cups_printers_manager.h
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/cups_printers_manager_unittest.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/printer_detector.h
[add] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/printer_detector_test_util.h
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/usb_printer_detector.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/usb_printer_detector.h
[add] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/usb_printer_detector_unittest.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/zeroconf_printer_detector.cc
[add] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/chromeos/printing/zeroconf_printer_detector_unittest.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/ui/webui/print_preview/local_printer_handler_chromeos.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/0aa24b56e2aab22f820c2f7d218bc923bfb19381/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.h

Comment 4 by valleau@chromium.org, Jan 24 2018

Status: WontFix (was: Assigned)
The following change removes the need for two-step observer callback setup so I'm marking this bug as obsolete.

https://chromium-review.googlesource.com/c/chromium/src/+/871660
►

Sign in to add a comment