Skip to content

Commit

Permalink
Record metrics for abandoned printer setups.
Browse files Browse the repository at this point in the history 
To determine what printers users are having difficulty setting up,
record the printer if setup is abandoned from the PPD selection screen.
This will tell us where our PPD coverage is lacking.

[email protected]

(cherry picked from commit c397a59)

Bug: 756576
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Id1e0913a3096d8a674a14cf230f6b97af456b330
Reviewed-on: https://chromium-review.googlesource.com/602696
Commit-Queue: Sean Kau <[email protected]>
Reviewed-by: Demetrios Papadopoulos <[email protected]>
Reviewed-by: Xiaoqian Dai <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#509654}
Reviewed-on: https://chromium-review.googlesource.com/728348
Reviewed-by: Sean Kau <[email protected]>
Cr-Commit-Position: refs/branch-heads/3239@{#72}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
  • Loading branch information
Sean Kau committed Oct 19, 2017
1 parent dba861a commit 5c56ee9
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 12 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/resources/settings/printing_page/cups_add_printer_dialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ Polymer({
/** @private */
onCancelTap_: function() {
this.close();
settings.CupsPrintersBrowserProxyImpl.getInstance().cancelPrinterSetUp(
this.activePrinter);
},

/** @private */
Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/resources/settings/printing_page/cups_printers_browser_proxy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ cr.define('settings', function() {
* @param{string} printerId
*/
addDiscoveredPrinter(printerId) {}

/**
* Report to the handler that setup was cancelled.
* @param {!CupsPrinterInfo} newPrinter
*/
cancelPrinterSetUp(newPrinter) {}
}

/**
Expand Down Expand Up @@ -216,6 +222,11 @@ cr.define('settings', function() {
addDiscoveredPrinter(printerId) {
chrome.send('addDiscoveredPrinter', [printerId]);
}

/** @override */
cancelPrinterSetUp(newPrinter) {
chrome.send('cancelPrinterSetUp', [newPrinter]);
}
}

cr.addSingletonGetter(CupsPrintersBrowserProxyImpl);
Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@ void CupsPrintersHandler::RegisterMessages() {
"addDiscoveredPrinter",
base::Bind(&CupsPrintersHandler::HandleAddDiscoveredPrinter,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"cancelPrinterSetUp", base::Bind(&CupsPrintersHandler::HandleSetUpCancel,
base::Unretained(this)));
}

void CupsPrintersHandler::OnJavascriptAllowed() {
Expand Down Expand Up @@ -766,6 +769,14 @@ void CupsPrintersHandler::HandleStopDiscovery(const base::ListValue* args) {
discovery_active_ = false;
}

void CupsPrintersHandler::HandleSetUpCancel(const base::ListValue* args) {
const base::DictionaryValue* printer_dict;
CHECK(args->GetDictionary(0, &printer_dict));

const Printer printer = DictToPrinter(*printer_dict);
printers_manager_->RecordSetupAbandoned(printer);
}

void CupsPrintersHandler::OnPrintersChanged(
CupsPrintersManager::PrinterClass printer_class,
const std::vector<Printer>& printers) {
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ class CupsPrintersHandler : public ::settings::SettingsPageUIHandler,
void HandleStartDiscovery(const base::ListValue* args);
void HandleStopDiscovery(const base::ListValue* args);

// Logs printer set ups that are abandoned.
void HandleSetUpCancel(const base::ListValue* args);

// Given a printer id, find the corresponding ppdManufacturer and ppdModel.
void HandleGetPrinterPpdManufacturerAndModel(const base::ListValue* args);
void OnGetPrinterPpdManufacturerAndModel(
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/webui/settings/cr_settings_browsertest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@ CrSettingsPrintingPageTest.prototype = {
__proto__: CrSettingsBrowserTest.prototype,

/** @override */
browsePreload: 'chrome://settings/printing_page/cups_add_printer_dialog.html',
browsePreload: 'chrome://settings/printing_page/cups_printers.html',

/** @override */
extraLibraries: CrSettingsBrowserTest.prototype.extraLibraries.concat([
Expand Down
180 changes: 169 additions & 11 deletions chrome/test/data/webui/settings/cups_printer_page_tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
class TestCupsPrintersBrowserProxy extends TestBrowserProxy {
constructor() {
super([
'addDiscoveredPrinter',
'getCupsPrintersList',
'getCupsPrinterManufacturersList',
'getCupsPrinterModelsList',
'getPrinterInfo',
'startDiscoveringPrinters',
'stopDiscoveringPrinters',
'cancelPrinterSetUp',
]);

this.printerList = [];
Expand All @@ -20,6 +22,11 @@ class TestCupsPrintersBrowserProxy extends TestBrowserProxy {
this.printerInfo = {};
}

/** @override */
addDiscoveredPrinter(printerId) {
this.methodCalled('addDiscoveredPrinter', printerId);
}

/** @override */
getCupsPrintersList() {
this.methodCalled('getCupsPrintersList');
Expand All @@ -41,8 +48,7 @@ class TestCupsPrintersBrowserProxy extends TestBrowserProxy {
/** @override */
getPrinterInfo(newPrinter) {
this.methodCalled('getPrinterInfo', newPrinter);
// Reject all calls for now.
return Promise.reject();
return Promise.resolve(this.printerInfo);
}

/** @override */
Expand All @@ -54,6 +60,11 @@ class TestCupsPrintersBrowserProxy extends TestBrowserProxy {
stopDiscoveringPrinters() {
this.methodCalled('stopDiscoveringPrinters');
}

/** @override */
cancelPrinterSetUp(newPrinter) {
this.methodCalled('cancelPrinterSetUp', newPrinter);
}
}

suite('CupsAddPrinterDialogTests', function() {
Expand All @@ -68,6 +79,20 @@ suite('CupsAddPrinterDialogTests', function() {
address.value = '127.0.0.1';
}

function clickAddButton(dialog) {
assertTrue(!!dialog, 'Dialog is null for add');
var addButton = dialog.$$('.action-button');
assertTrue(!!addButton, 'Button is null');
MockInteractions.tap(addButton);
}

function clickCancelButton(dialog) {
assertTrue(!!dialog, 'Dialog is null for cancel');
var cancelButton = dialog.$$('.cancel-button');
assertTrue(!!cancelButton, 'Button is null');
MockInteractions.tap(cancelButton);
}

var page = null;
var dialog = null;

Expand All @@ -78,19 +103,19 @@ suite('CupsAddPrinterDialogTests', function() {
cupsPrintersBrowserProxy = new TestCupsPrintersBrowserProxy();
settings.CupsPrintersBrowserProxyImpl.instance_ = cupsPrintersBrowserProxy;

cupsPrintersBrowserProxy.reset();

PolymerTest.clearBody();
page = document.createElement('settings-cups-printers');
dialog = document.createElement('settings-cups-add-printer-dialog');
document.body.appendChild(page);
page.appendChild(dialog);
assertTrue(!!page);
dialog = page.$$('settings-cups-add-printer-dialog');
assertTrue(!!dialog);

dialog.open();
Polymer.dom.flush();
});

teardown(function() {
cupsPrintersBrowserProxy.reset();
page.remove();
dialog = null;
page = null;
Expand Down Expand Up @@ -167,19 +192,152 @@ suite('CupsAddPrinterDialogTests', function() {
assertTrue(!!addDialog);
fillAddManuallyDialog(addDialog);

// Verify that getCupsPrinterModelList is not called.
cupsPrintersBrowserProxy.whenCalled('getCupsPrinterModelsList')
.then(function(manufacturer) { assertGT(0, manufacturer.length); });
.then(function(manufacturer) {
assertNotReached(
'No manufacturer was selected. Unexpected model request.');
});

cupsPrintersBrowserProxy.manufacturers =
['ManufacturerA', 'ManufacturerB', 'Chromites'];
MockInteractions.tap(addDialog.$$('.action-button'));
Polymer.dom.flush();

return cupsPrintersBrowserProxy.
whenCalled('getCupsPrinterManufacturersList').
then(function() {
var modelDialog = dialog.$$('add-printer-manufacturer-model-dialog');
return cupsPrintersBrowserProxy
.whenCalled('getCupsPrinterManufacturersList')
.then(function() {
let modelDialog = dialog.$$('add-printer-manufacturer-model-dialog');
assertTrue(!!modelDialog);
// Manufacturer dialog has been rendered and the model list was not
// requested. We're done.
});
});

/**
* Test that dialog cancellation is logged from the manufacturer screen for
* IPP printers.
*/
test('LogDialogCancelledIpp', function() {
var makeAndModel = 'Printer Make And Model';
// Start on add manually.
dialog.fire('open-manually-add-printer-dialog');
Polymer.dom.flush();

// Populate the printer object.
dialog.newPrinter = {
ppdManufacturer: '',
ppdModel: '',
printerAddress: '192.168.1.13',
printerAutoconf: false,
printerDescription: '',
printerId: '',
printerManufacturer: '',
printerModel: '',
printerMakeAndModel: '',
printerName: 'Test Printer',
printerPPDPath: '',
printerProtocol: 'ipps',
printerQueue: 'moreinfohere',
printerStatus: '',
};

// Seed the getPrinterInfo response. We detect the make and model but it is
// not an autoconf printer.
cupsPrintersBrowserProxy.printerInfo = {
autoconf: false,
manufacturer: 'MFG',
model: 'MDL',
makeAndModel: makeAndModel,
};

// Press the add button to advance dialog.
var addDialog = dialog.$$('add-printer-manually-dialog');
assertTrue(!!addDialog);
clickAddButton(addDialog);

// Click cancel on the manufacturer dialog when it shows up then verify
// cancel was called with the appropriate parameters.
return cupsPrintersBrowserProxy
.whenCalled('getCupsPrinterManufacturersList')
.then(function() {
Polymer.dom.flush();
// Cancel setup with the cancel button.
clickCancelButton(dialog.$$('add-printer-manufacturer-model-dialog'));
return cupsPrintersBrowserProxy.whenCalled('cancelPrinterSetUp');
})
.then(function(printer) {
assertTrue(!!printer, 'New printer is null');
assertEquals(makeAndModel, printer.printerMakeAndModel);
});
});

/**
* Test that dialog cancellation is logged from the manufacturer screen for
* USB printers.
*/
test('LogDialogCancelledUSB', function() {
var vendorId = 0x1234;
var modelId = 0xDEAD;
var manufacturer = 'PrinterMFG';
var model = 'Printy Printerson';

var usbInfo = {
usbVendorId: vendorId,
usbProductId: modelId,
usbVendorName: manufacturer,
usbProductName: model,
};

var expectedPrinter = 'PICK_ME!';

var newPrinter = {
ppdManufacturer: '',
ppdModel: '',
printerAddress: 'EEAADDAA',
printerAutoconf: false,
printerDescription: '',
printerId: expectedPrinter,
printerManufacturer: '',
printerModel: '',
printerMakeAndModel: '',
printerName: '',
printerPPDPath: '',
printerProtocol: 'usb',
printerQueue: 'moreinfohere',
printerStatus: '',
printerUsbInfo: usbInfo,
};

dialog.fire('open-discovery-printers-dialog');

return cupsPrintersBrowserProxy.whenCalled('startDiscoveringPrinters')
.then(function() {
// Select the printer.
// TODO(skau): Figure out how to select in a dom-repeat.
let discoveryDialog = dialog.$$('add-printer-discovery-dialog');
assertTrue(!!discoveryDialog, 'Cannot find discovery dialog');
discoveryDialog.selectedPrinter = newPrinter;
// Run printer setup.
clickAddButton(discoveryDialog);
return cupsPrintersBrowserProxy.whenCalled('addDiscoveredPrinter');
})
.then(function(printerId) {
assertEquals(expectedPrinter, printerId);

cr.webUIListenerCallback(
'on-manually-add-discovered-printer', newPrinter);
return cupsPrintersBrowserProxy.whenCalled(
'getCupsPrinterManufacturersList');
})
.then(function() {
// Cancel setup with the cancel button.
clickCancelButton(dialog.$$('add-printer-manufacturer-model-dialog'));
return cupsPrintersBrowserProxy.whenCalled('cancelPrinterSetUp');
})
.then(function(printer) {
assertEquals(expectedPrinter, printer.printerId);
assertDeepEquals(usbInfo, printer.printerUsbInfo);
});
});
});

0 comments on commit 5c56ee9

Please sign in to comment.