Skip to content

Commit

Permalink
Printing: Fix parameter type for local printer observer
Browse files Browse the repository at this point in the history
Loosen the parameter restrictions for addListenerCallback() so the
DestinationStore can properly only expect LocalDestinationInfo for the
'local-printers-updated' event.

The 'local-printers-updated' event fired from PrintPreviewHandler
should only ever send LocalDestinationInfo.

Bug: b:300136694
Change-Id: Ic52a75572582d7970bfc9043d70138ec4dd35265
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4955615
Commit-Queue: Gavin Williams <gavinwill@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1213072}
  • Loading branch information
Gavin Williams authored and Chromium LUCI CQ committed Oct 21, 2023
1 parent 3d4bfed commit 045b3a9
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 21 deletions.
20 changes: 6 additions & 14 deletions chrome/browser/resources/print_preview/data/destination_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,7 @@ export class DestinationStore extends EventTarget {
*/
constructor(
addListenerCallback:
(eventName: string,
listener:
(t: PrinterType,
p: LocalDestinationInfo[]|
ExtensionDestinationInfo[]) => void) => void) {
(eventName: string, listener: (p1: any, p2?: any) => void) => void) {
super();

this.destinationSearchStatus_ = new Map([
Expand All @@ -262,9 +258,8 @@ export class DestinationStore extends EventTarget {
if (loadTimeData.getBoolean('isLocalPrinterObservingEnabled')) {
addListenerCallback(
'local-printers-updated',
(type: PrinterType,
printers: LocalDestinationInfo[]|ExtensionDestinationInfo[]) =>
this.onLocalPrintersUpdated_(type, printers));
(printers: LocalDestinationInfo[]) =>
this.onLocalPrintersUpdated_(printers));
}
// </if>
}
Expand Down Expand Up @@ -974,24 +969,21 @@ export class DestinationStore extends EventTarget {

this.nativeLayerCros_.observeLocalPrinters().then(
(printers: LocalDestinationInfo[]) =>
this.onLocalPrintersUpdated_(PrinterType.LOCAL_PRINTER, printers));
this.onLocalPrintersUpdated_(printers));
}

/**
* Inserts any new printers retrieved from the 'local-printers-updated' event.
* @param printerType The type of printer(s) added.
* @param printers Information about the printers that have been retrieved.
*/
private onLocalPrintersUpdated_(
printerType: PrinterType,
printers: LocalDestinationInfo[]|ExtensionDestinationInfo[]) {
private onLocalPrintersUpdated_(printers: LocalDestinationInfo[]) {
if (!printers) {
return;
}

this.insertDestinations_(printers.map(
(printer: LocalDestinationInfo|ExtensionDestinationInfo) =>
parseDestination(printerType, printer)));
printer => parseDestination(PrinterType.LOCAL_PRINTER, printer)));
}
// </if>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,6 @@ void PrintPreviewHandlerChromeOS::OnHandleObserveLocalPrinters(
void PrintPreviewHandlerChromeOS::OnLocalPrintersUpdated(
std::vector<crosapi::mojom::LocalDestinationInfoPtr> printers) {
FireWebUIListener("local-printers-updated",
base::Value(static_cast<int>(mojom::PrinterType::kLocal)),
ConvertPrintersToValues(printers));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,9 +496,7 @@ TEST_F(PrintPreviewHandlerChromeOSTest, FireLocalPrintersUpdated) {

const content::TestWebUI::CallData& data = *web_ui()->call_data().back();
AssertWebUIEventFired(data, "local-printers-updated");
EXPECT_EQ(static_cast<int>(mojom::PrinterType::kLocal),
data.arg2()->GetInt());
EXPECT_EQ(printers.size(), data.arg3()->GetList().size());
EXPECT_EQ(printers.size(), data.arg2()->GetList().size());
}

} // namespace printing
Original file line number Diff line number Diff line change
Expand Up @@ -485,9 +485,7 @@ suite('DestinationStoreTest', function() {

// Fire the event and expect the destination store to add the local
// printers.
webUIListenerCallback(
'local-printers-updated', PrinterType.LOCAL_PRINTER,
[printer1, printer2]);
webUIListenerCallback('local-printers-updated', [printer1, printer2]);
assertTrue(!!destinationStore.destinations().find(
destination => destination.id === printer1.printerName));
assertTrue(!!destinationStore.destinations().find(
Expand Down

0 comments on commit 045b3a9

Please sign in to comment.