Skip to content

Commit

Permalink
Print Preview: Remove cloud printers from sticky settings
Browse files Browse the repository at this point in the history
- Remove cloud printers in sticky settings
- Mark cloud printer type as deprecated
- Remove some handling for cloud printers, since there should now be no
  way for them to end up in the destinations.
- Remove tests that assume cloud printers can be in the destinations,
  and replace these tests with a test verifying unsupported printers
  get removed from sticky settings.

Bug: 1162164
Change-Id: I49a578cc069470cf6822444508288cc360c83647
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564669
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990090}
  • Loading branch information
Rebekah Potter authored and Chromium LUCI CQ committed Apr 7, 2022
1 parent 721981e commit 069a882
Show file tree
Hide file tree
Showing 27 changed files with 134 additions and 319 deletions.
2 changes: 0 additions & 2 deletions chrome/browser/ash/crosapi/local_printer_ash.cc
Expand Up @@ -497,8 +497,6 @@ void LocalPrinterAsh::GetPrinterTypeDenyList(
printer_type = printing::mojom::PrinterType::kPdf;
else if (deny_list_str == "local")
printer_type = printing::mojom::PrinterType::kLocal;
else if (deny_list_str == "cloud")
printer_type = printing::mojom::PrinterType::kCloud;
else
continue;

Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/printing/print_test_utils.cc
Expand Up @@ -53,9 +53,7 @@ base::Value GetPrintTicket(mojom::PrinterType type) {
ticket.SetIntKey(kSettingPageHeight, 279400);
ticket.SetBoolKey(kSettingShowSystemDialog, false);

if (type == mojom::PrinterType::kCloud) {
ticket.SetStringKey(kSettingCloudPrintId, kDummyPrinterName);
} else if (type == mojom::PrinterType::kExtension) {
if (type == mojom::PrinterType::kExtension) {
base::Value capabilities(base::Value::Type::DICTIONARY);
capabilities.SetBoolKey("duplex", true); // non-empty
std::string caps_string;
Expand Down
10 changes: 0 additions & 10 deletions chrome/browser/resources/print_preview/data/destination.ts
Expand Up @@ -45,16 +45,6 @@ export enum DestinationOrigin {
CROS = 'chrome_os',
}

/**
* Cloud Print origins.
*/
export const CloudOrigins: DestinationOrigin[] = [
DestinationOrigin.COOKIES,
// <if expr="chromeos_ash or chromeos_lacros">
DestinationOrigin.DEVICE,
// </if>
];

/**
* Enumeration of the connection statuses of printer destinations.
*/
Expand Down
11 changes: 4 additions & 7 deletions chrome/browser/resources/print_preview/data/destination_match.ts
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

import {assert} from 'chrome://resources/js/assert_ts.js';
import {CloudOrigins, Destination, DestinationOrigin, GooglePromotedDestinationId, RecentDestination} from './destination.js';
import {Destination, DestinationOrigin, GooglePromotedDestinationId, RecentDestination} from './destination.js';

/**
* Printer types for capabilities and printer list requests.
Expand All @@ -14,18 +14,15 @@ export enum PrinterType {
EXTENSION_PRINTER = 1,
PDF_PRINTER = 2,
LOCAL_PRINTER = 3,
CLOUD_PRINTER = 4
CLOUD_PRINTER_DEPRECATED = 4
}

export function originToType(origin: DestinationOrigin): PrinterType {
if (origin === DestinationOrigin.LOCAL || origin === DestinationOrigin.CROS) {
return PrinterType.LOCAL_PRINTER;
}
if (origin === DestinationOrigin.EXTENSION) {
return PrinterType.EXTENSION_PRINTER;
}
assert(CloudOrigins.includes(origin));
return PrinterType.CLOUD_PRINTER;
assert(origin === DestinationOrigin.EXTENSION);
return PrinterType.EXTENSION_PRINTER;
}

export function getPrinterTypeForDestination(
Expand Down
18 changes: 1 addition & 17 deletions chrome/browser/resources/print_preview/data/destination_store.ts
Expand Up @@ -6,7 +6,7 @@ import {assert} from 'chrome://resources/js/assert_ts.js';
import {EventTracker} from 'chrome://resources/js/event_tracker.m.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';

import {DestinationSearchBucket, MetricsContext, PrintPreviewInitializationEvents} from '../metrics.js';
import {MetricsContext, PrintPreviewInitializationEvents} from '../metrics.js';
import {CapabilitiesResponse, NativeLayer, NativeLayerImpl} from '../native_layer.js';
// <if expr="chromeos_ash or chromeos_lacros">
import {NativeLayerCros, NativeLayerCrosImpl, PrinterSetupResponse} from '../native_layer_cros.js';
Expand Down Expand Up @@ -377,14 +377,6 @@ export class DestinationStore extends EventTarget {
return;
}

// Check for Cloud Print printers and remove them. Cloud Print is no longer
// supported.
// TODO(rbpotter): Remove cloud printers from the sticky settings/auto
// select, similar to current handling for privet, and remove this.
if (this.typesToSearch_.has(PrinterType.CLOUD_PRINTER)) {
this.typesToSearch_.delete(PrinterType.CLOUD_PRINTER);
}

for (const printerType of this.typesToSearch_) {
this.startLoadDestinations_(printerType);
}
Expand Down Expand Up @@ -628,14 +620,6 @@ export class DestinationStore extends EventTarget {

// Update and persist selected destination.
this.selectedDestination_ = destination;
// Adjust metrics.
if (destination.cloudID &&
this.destinations_.some(function(otherDestination) {
return otherDestination.cloudID === destination.cloudID &&
otherDestination !== destination;
})) {
this.metrics_.record(DestinationSearchBucket.CLOUD_DUPLICATE_SELECTED);
}
// Notify about selected destination change.
this.dispatchEvent(
new CustomEvent(DestinationStoreEventType.DESTINATION_SELECT));
Expand Down
19 changes: 9 additions & 10 deletions chrome/browser/resources/print_preview/data/model.ts
Expand Up @@ -1066,19 +1066,18 @@ export class PrintPreviewModelElement extends PolymerElement {
recentDestinations = [recentDestinations];
}

// Remove unsupported privet printers from the sticky settings,
// Remove unsupported privet and cloud printers from the sticky settings,
// to free up these spots for supported printers.
const unsupportedOrigins: DestinationOrigin[] = [
DestinationOrigin.COOKIES,
// <if expr="chromeos_ash or chromeos_lacros">
DestinationOrigin.DEVICE,
// </if>
DestinationOrigin.PRIVET,
];
recentDestinations = recentDestinations.filter((d: RecentDestination) => {
return d.origin !== DestinationOrigin.PRIVET;
});

// <if expr="chromeos_ash or chromeos_lacros">
// Remove Cloud Print Drive destination. The Chrome OS version will always
// be shown in the dropdown and is still supported.
recentDestinations = recentDestinations.filter((d: RecentDestination) => {
return d.id !== GooglePromotedDestinationId.DOCS;
return !unsupportedOrigins.includes(d.origin);
});
// </if>

// Initialize recent destinations early so that the destination store can
// start trying to fetch them.
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/resources/print_preview/metrics.ts
Expand Up @@ -168,9 +168,6 @@ export class MetricsContext {
case (PrinterType.LOCAL_PRINTER):
histogram = 'PrintPreview.Initialization.GetPrinters.Local';
break;
case (PrinterType.CLOUD_PRINTER):
histogram = 'PrintPreview.Initialization.GetPrinters.Cloud';
break;
default:
assertNotReached('unknown type = ' + type);
}
Expand Down
Expand Up @@ -18,7 +18,7 @@ import '../strings.m.js';
import {I18nMixin} from 'chrome://resources/js/i18n_mixin.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {CloudOrigins, Destination, DestinationOrigin, GooglePromotedDestinationId, PDF_DESTINATION_KEY} from '../data/destination.js';
import {Destination, DestinationOrigin, GooglePromotedDestinationId, PDF_DESTINATION_KEY} from '../data/destination.js';
import {ERROR_STRING_KEY_MAP, getPrinterStatusIcon, PrinterStatusReason} from '../data/printer_status_cros.js';

import {PrintPreviewDestinationDropdownCrosElement} from './destination_dropdown_cros.js';
Expand Down Expand Up @@ -238,16 +238,6 @@ export class PrintPreviewDestinationSelectCrosElement extends
return '';
}

// Cloudprint destinations contain their own status text.
if (CloudOrigins.some(origin => origin === this.destination.origin)) {
if (this.destination.shouldShowInvalidCertificateError) {
return this.i18n('noLongerSupportedFragment');
}
if (this.destination.connectionStatusText) {
return this.destination.connectionStatusText;
}
}

// Non-local printers do not show an error status.
if (this.destination.origin !== DestinationOrigin.CROS) {
return '';
Expand Down
57 changes: 1 addition & 56 deletions chrome/browser/ui/webui/print_preview/print_preview_handler.cc
Expand Up @@ -12,7 +12,6 @@
#include <utility>
#include <vector>

#include "base/base64.h"
#include "base/bind.h"
#include "base/callback_helpers.h"
#include "base/check.h"
Expand All @@ -21,7 +20,6 @@
#include "base/i18n/number_formatting.h"
#include "base/json/json_reader.h"
#include "base/lazy_instance.h"
#include "base/memory/ref_counted_memory.h"
#include "base/values.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
Expand Down Expand Up @@ -100,11 +98,6 @@ namespace printing {

namespace {

// Max size for PDFs sent to Cloud Print. Server side limit is currently 80MB
// but PDF will double in size when sent to JS. See crbug.com/793506 and
// crbug.com/372240.
constexpr size_t kMaxCloudPrintPdfDataSizeInBytes = 80 * 1024 * 1024 / 2;

mojom::PrinterType GetPrinterTypeForUserAction(UserActionBuckets user_action) {
switch (user_action) {
case UserActionBuckets::kPrintWithExtension:
Expand Down Expand Up @@ -188,8 +181,6 @@ const char kPdfPrinterDisabled[] = "pdfPrinterDisabled";
// Name of a dictionary field indicating whether the destinations are managed by
// the PrinterTypeDenyList enterprise policy.
const char kDestinationsManaged[] = "destinationsManaged";
// Name of a dictionary field holding the cloud print URL.
const char kCloudPrintURL[] = "cloudPrintURL";
#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
// Name of a dictionary field indicating whether the user's Drive directory is
// mounted.
Expand Down Expand Up @@ -235,13 +226,6 @@ UserActionBuckets DetermineUserAction(const base::Value& settings) {
}
#endif

// This needs to be checked before checking for a cloud print ID, since a
// print ticket for printing to Drive will also contain a cloud print ID.
if (settings.FindBoolKey(kSettingPrintToGoogleDrive).value_or(false))
return UserActionBuckets::kPrintToGoogleDrive;
if (settings.FindKey(kSettingCloudPrintId))
return UserActionBuckets::kPrintWithCloudPrint;

mojom::PrinterType type = static_cast<mojom::PrinterType>(
settings.FindIntKey(kSettingPrinterType).value());
switch (type) {
Expand Down Expand Up @@ -570,8 +554,6 @@ void PrintPreviewHandler::ReadPrinterTypeDenyListFromPrefs() {
printer_type = mojom::PrinterType::kPdf;
else if (deny_list_str == "local")
printer_type = mojom::PrinterType::kLocal;
else if (deny_list_str == "cloud")
printer_type = mojom::PrinterType::kCloud;
else
continue;

Expand Down Expand Up @@ -705,7 +687,7 @@ void PrintPreviewHandler::HandleGetPreview(const base::Value::List& args) {
CHECK_GT(request_id, -1);
mojom::PrinterType printer_type = static_cast<mojom::PrinterType>(
settings.FindIntKey(kSettingPrinterType).value());
CHECK(printer_type != mojom::PrinterType::kCloud || IsCloudPrintEnabled());
CHECK_NE(printer_type, mojom::PrinterType::kCloudDeprecated);

CHECK(!base::Contains(preview_callbacks_, request_id));
preview_callbacks_[request_id] = callback_id;
Expand Down Expand Up @@ -808,14 +790,6 @@ void PrintPreviewHandler::HandlePrint(const base::Value::List& args) {
}
ReportUserActionHistogram(user_action);

if (user_action == UserActionBuckets::kPrintWithCloudPrint ||
user_action == UserActionBuckets::kPrintToGoogleDrive) {
// Does not send the title like the other printer handler types below,
// because JS already has the document title from the initial settings.
SendCloudPrintJob(callback_id, data.get());
return;
}

PrinterHandler* handler =
GetPrinterHandler(GetPrinterTypeForUserAction(user_action));
handler->StartPrint(print_preview_ui()->initiator_title(),
Expand Down Expand Up @@ -998,11 +972,6 @@ void PrintPreviewHandler::SendInitialSettings(
if (policies.is_dict() && !policies.DictEmpty())
initial_settings.SetKey(kPolicies, std::move(policies));

if (IsCloudPrintEnabled()) {
initial_settings.SetStringKey(
kCloudPrintURL, GURL(cloud_devices::GetCloudPrintURL()).spec());
}

initial_settings.SetBoolKey(
kPdfPrinterDisabled,
base::Contains(printer_type_deny_list_, mojom::PrinterType::kPdf));
Expand Down Expand Up @@ -1074,25 +1043,6 @@ void PrintPreviewHandler::SendPrinterCapabilities(
RejectJavascriptCallback(base::Value(callback_id), base::Value());
}

void PrintPreviewHandler::SendCloudPrintJob(
const std::string& callback_id,
const base::RefCountedMemory* data) {
// Crash if a cloud print job is requested and cloud print is not enabled.
CHECK(IsCloudPrintEnabled());

// BASE64 encode the job data.
const base::StringPiece raw_data(data->front_as<char>(), data->size());
std::string base64_data;
base::Base64Encode(raw_data, &base64_data);

if (base64_data.size() >= kMaxCloudPrintPdfDataSizeInBytes) {
RejectJavascriptCallback(base::Value(callback_id),
base::Value("OVERSIZED_PDF"));
return;
}
ResolveJavascriptCallback(base::Value(callback_id), base::Value(base64_data));
}

WebContents* PrintPreviewHandler::GetInitiator() {
PrintPreviewDialogController* dialog_controller =
PrintPreviewDialogController::GetInstance();
Expand Down Expand Up @@ -1285,11 +1235,6 @@ void PrintPreviewHandler::OnPrintResult(const std::string& callback_id,
}
}

bool PrintPreviewHandler::IsCloudPrintEnabled() {
return !base::Contains(printer_type_deny_list_, mojom::PrinterType::kCloud) &&
GetPrefs()->GetBoolean(prefs::kCloudPrintSubmitEnabled);
}

void PrintPreviewHandler::BadMessageReceived() {
bad_message::ReceivedBadMessage(
GetInitiator()->GetMainFrame()->GetProcess(),
Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/ui/webui/print_preview/print_preview_handler.h
Expand Up @@ -38,7 +38,6 @@ class LocalPrinter;

namespace base {
class DictionaryValue;
class RefCountedMemory;
}

namespace content {
Expand Down Expand Up @@ -114,9 +113,6 @@ class PrintPreviewHandler : public content::WebUIMessageHandler {
virtual PrinterHandler* GetPrinterHandler(mojom::PrinterType printer_type);

protected:
// Protected so unit tests can override.
virtual bool IsCloudPrintEnabled();

// Shuts down the initiator renderer. Called when a bad IPC message is
// received.
virtual void BadMessageReceived();
Expand Down Expand Up @@ -237,11 +233,6 @@ class PrintPreviewHandler : public content::WebUIMessageHandler {
void SendPrinterCapabilities(const std::string& callback_id,
base::Value settings_info);

// Send the PDF data to Print Preview so that it can be sent to the cloud
// print server to print.
void SendCloudPrintJob(const std::string& callback_id,
const base::RefCountedMemory* data);

// Closes the preview dialog.
void ClosePreviewDialog();

Expand Down

0 comments on commit 069a882

Please sign in to comment.