Commit ec27b0ad authored by rbpotter's avatar rbpotter Committed by Commit Bot

Print Preview Refresh: Fix Save as PDF bug

Print Preview's destination and capabilities update process works as
follows:
(1) The destination associated with the destination-list-item selected
by the user is sent to the destination store. Call this
item-destination.
(2) destination store sets |selectedDestination_| to the destination
in its destinations map that has a matching key to item-destination,
and requests capabilities for item-destination.
(3) When capabilities are returned from the handler, the store updates
|selectedDestination_| if item-destination, the destination for which
they were retrieved, matches |selectedDestination_|.
(4) This update transitions the UI to a ready state.

When the user signs in successfully, the destinations map is completely
reset. The Save as PDF destination is recreated just after reset and
the recreated Save as PDF printer is added to the map. However, this
was not triggering an update to the destination associated with the
destination-list-item displaying the Save as PDF printer, because the
new Save as PDF destination has all the same properties as the old one.
As a result, if this destination was selected, the capabilities were
retrieved for the old Save as PDF destination, while the
|selectedDestination_| was the new destination. This mismatch caused
the destination update event to never be fired, so the UI was stuck
in a non-ready state.

This CL forces the destination list used by the destination-list-items
to update when the destination store updates its map, which ensures
the list item for the Save as PDF printer gets the updated Save as PDF
destination from the destination store's map.

Bug: 895769
Change-Id: I1052609eddb2c240ece4e3132b829fb6b10ef35c
Reviewed-on: https://chromium-review.googlesource.com/c/1284708
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602053}
parent 64baffef
......@@ -1129,6 +1129,8 @@ cr.define('print_preview', function() {
this.autoSelectTimeout_ = setTimeout(
this.selectDefaultDestination_.bind(this),
DestinationStore.AUTO_SELECT_TIMEOUT_);
cr.dispatchSimpleEvent(
this, DestinationStore.EventType.DESTINATIONS_RESET);
}
......@@ -1354,6 +1356,7 @@ cr.define('print_preview', function() {
DESTINATION_SEARCH_STARTED:
'print_preview.DestinationStore.DESTINATION_SEARCH_STARTED',
DESTINATION_SELECT: 'print_preview.DestinationStore.DESTINATION_SELECT',
DESTINATIONS_RESET: 'print_preview.DestinationStore.DESTINATIONS_RESET',
DESTINATIONS_INSERTED:
'print_preview.DestinationStore.DESTINATIONS_INSERTED',
PROVISIONAL_DESTINATION_RESOLVED:
......
......@@ -129,6 +129,10 @@ Polymer({
destinationStore,
print_preview.DestinationStore.EventType.DESTINATIONS_INSERTED,
this.updateDestinations_.bind(this));
this.tracker_.add(
destinationStore,
print_preview.DestinationStore.EventType.DESTINATIONS_RESET,
() => this.destinations_ = []);
this.tracker_.add(
destinationStore,
print_preview.DestinationStore.EventType.DESTINATION_SEARCH_DONE,
......
......@@ -81,7 +81,8 @@
<div class="destination-throbber-name"></div>
</div>
<div class="destination-settings-box" hidden="[[loadingDestination_]]">
<iron-icon icon$="[[destination.icon]]"></iron-icon>
<iron-icon icon$="[[destination.icon]]" hidden="[[!destination]]">
</iron-icon>
<div class="destination-info-wrapper">
<div class="destination-name">[[destination.displayName]]</div>
<div class="destination-location">[[destination.hint]]</div>
......
......@@ -65,8 +65,7 @@ Polymer({
/** @private */
onDestinationSet_: function() {
if (this.destination && this.destination.id)
this.loadingDestination_ = false;
this.loadingDestination_ = !this.destination || !this.destination.id;
},
/**
......
......@@ -7,6 +7,7 @@ cr.define('destination_dialog_test', function() {
const TestNames = {
PrinterList: 'PrinterList',
ShowProvisionalDialog: 'ShowProvisionalDialog',
ReloadPrinterList: 'ReloadPrinterList',
};
const suiteName = 'DestinationDialogTest';
......@@ -168,6 +169,41 @@ cr.define('destination_dialog_test', function() {
assertTrue(dialog.$.dialog.open);
});
});
// Test that destinations are correctly cleared when the destination store
// reloads the printer list.
test(assert(TestNames.ReloadPrinterList), function() {
const lists =
dialog.shadowRoot.querySelectorAll('print-preview-destination-list');
assertEquals(2, lists.length);
const recentItems = lists[0].shadowRoot.querySelectorAll(
'print-preview-destination-list-item');
const printerItems = lists[1].shadowRoot.querySelectorAll(
'print-preview-destination-list-item');
assertEquals(1, recentItems.length);
assertEquals(6, printerItems.length);
const oldPdfDestination = printerItems[0].destination;
assertEquals(
print_preview.Destination.GooglePromotedId.SAVE_AS_PDF,
oldPdfDestination.id);
const whenReset = test_util.eventToPromise(
print_preview.DestinationStore.EventType.DESTINATIONS_RESET,
destinationStore);
cr.webUIListenerCallback('reload-printer-list');
return whenReset.then(() => {
Polymer.dom.flush();
const printerItems = dialog.$.printList.shadowRoot.querySelectorAll(
'print-preview-destination-list-item');
assertEquals(6, printerItems.length);
const newPdfDestination = printerItems[0].destination;
assertEquals(
print_preview.Destination.GooglePromotedId.SAVE_AS_PDF,
newPdfDestination.id);
assertNotEquals(oldPdfDestination, newPdfDestination);
});
});
});
return {
......
......@@ -694,6 +694,10 @@ TEST_F(
destination_dialog_test.TestNames.ShowProvisionalDialog);
});
TEST_F('PrintPreviewDestinationDialogTest', 'ReloadPrinterList', function() {
this.runMochaTest(destination_dialog_test.TestNames.ReloadPrinterList);
});
PrintPreviewAdvancedDialogTest = class extends NewPrintPreviewTest {
/** @override */
get browsePreload() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment