Commit c14400a9 authored by Justin Carlson's avatar Justin Carlson Committed by Commit Bot

Resolve mDNS URIs before handing them to CUPS.

Implementing this also obsoleted the updated time mechanism
for tracking whether or not our configuration is up-to-date
in CUPS.  This involved a lot of fixups to remove the last_updated
field from the Printer object.

Bug: 757595,
Change-Id: I7c75f9ce6a3d16e8bba2a9e6cf0c2e7cf7a09358
Reviewed-on: https://chromium-review.googlesource.com/627342Reviewed-by: default avatarSean Kau <skau@chromium.org>
Commit-Queue: Justin Carlson <justincarlson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496533}
parent 21a88f9a
...@@ -56,16 +56,6 @@ class PrinterDetectorObserverProxy : public PrinterDetector::Observer { ...@@ -56,16 +56,6 @@ class PrinterDetectorObserverProxy : public PrinterDetector::Observer {
ScopedObserver<PrinterDetector, PrinterDetector::Observer> observer_; ScopedObserver<PrinterDetector, PrinterDetector::Observer> observer_;
}; };
// Return the set of ids for the given list of printers.
std::unordered_set<std::string> GetIdsSet(
const std::vector<Printer>& printers) {
std::unordered_set<std::string> ret;
for (const Printer& printer : printers) {
ret.insert(printer.id());
}
return ret;
}
// This is akin to python's filter() builtin, but with reverse polarity on the // This is akin to python's filter() builtin, but with reverse polarity on the
// test function -- *remove* all entries in printers for which test_fn returns // test function -- *remove* all entries in printers for which test_fn returns
// true, discard the rest. // true, discard the rest.
...@@ -105,6 +95,7 @@ class CupsPrintersManagerImpl : public CupsPrintersManager, ...@@ -105,6 +95,7 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
// Prime the printer cache with the configured and enterprise printers. // Prime the printer cache with the configured and enterprise printers.
printers_[kConfigured] = synced_printers_manager_->GetConfiguredPrinters(); printers_[kConfigured] = synced_printers_manager_->GetConfiguredPrinters();
RebuildConfiguredPrintersIndex();
printers_[kEnterprise] = synced_printers_manager_->GetEnterprisePrinters(); printers_[kEnterprise] = synced_printers_manager_->GetEnterprisePrinters();
synced_printers_manager_observer_.Add(synced_printers_manager_); synced_printers_manager_observer_.Add(synced_printers_manager_);
...@@ -192,7 +183,8 @@ class CupsPrintersManagerImpl : public CupsPrintersManager, ...@@ -192,7 +183,8 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
void OnConfiguredPrintersChanged( void OnConfiguredPrintersChanged(
const std::vector<Printer>& printers) override { const std::vector<Printer>& printers) override {
printers_[kConfigured] = printers; printers_[kConfigured] = printers;
configured_printer_ids_ = GetIdsSet(printers); RebuildConfiguredPrintersIndex();
UpdateConfiguredPrinterURIs();
for (auto& observer : observer_list_) { for (auto& observer : observer_list_) {
observer.OnPrintersChanged(kConfigured, printers_[kConfigured]); observer.OnPrintersChanged(kConfigured, printers_[kConfigured]);
} }
...@@ -225,6 +217,41 @@ class CupsPrintersManagerImpl : public CupsPrintersManager, ...@@ -225,6 +217,41 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
} }
private: private:
// Rebuild the index from printer id to index for configured printers.
void RebuildConfiguredPrintersIndex() {
configured_printers_index_.clear();
for (size_t i = 0; i < printers_[kConfigured].size(); ++i) {
configured_printers_index_[printers_[kConfigured][i].id()] = i;
}
}
// Cross reference the Configured printers with the raw detected printer
// lists.
void UpdateConfiguredPrinterURIs() {
bool updated = false;
for (const auto* printer_list : {&usb_detections_, &zeroconf_detections_}) {
for (const auto& detected : *printer_list) {
auto configured =
configured_printers_index_.find(detected.printer.id());
if (configured != configured_printers_index_.end()) {
Printer* configured_printer =
&printers_[kConfigured][configured->second];
if (configured_printer->effective_uri() !=
detected.printer.effective_uri()) {
configured_printer->set_effective_uri(
detected.printer.effective_uri());
updated = true;
}
}
}
}
if (updated) {
for (auto& observer : observer_list_) {
observer.OnPrintersChanged(kAutomatic, printers_[kConfigured]);
}
}
}
// Look through all sources for the detected printer with the given id. // Look through all sources for the detected printer with the given id.
// Return a pointer to the printer on found, null if no entry is found. // Return a pointer to the printer on found, null if no entry is found.
const PrinterDetector::DetectedPrinter* FindDetectedPrinter( const PrinterDetector::DetectedPrinter* FindDetectedPrinter(
...@@ -301,9 +328,10 @@ class CupsPrintersManagerImpl : public CupsPrintersManager, ...@@ -301,9 +328,10 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
void AddDetectedList( void AddDetectedList(
const std::vector<PrinterDetector::DetectedPrinter>& detected_list) { const std::vector<PrinterDetector::DetectedPrinter>& detected_list) {
for (const PrinterDetector::DetectedPrinter& detected : detected_list) { for (const PrinterDetector::DetectedPrinter& detected : detected_list) {
if (base::ContainsKey(configured_printer_ids_, detected.printer.id())) { if (base::ContainsKey(configured_printers_index_,
// It's already in the configured classes, so neither automatic nor detected.printer.id())) {
// discovered is appropriate. Skip it. // It's already in the configured class, don't need to do anything
// else here.
continue; continue;
} }
auto it = detected_printer_ppd_references_.find(detected.printer.id()); auto it = detected_printer_ppd_references_.find(detected.printer.id());
...@@ -379,6 +407,7 @@ class CupsPrintersManagerImpl : public CupsPrintersManager, ...@@ -379,6 +407,7 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
value.reset(new Printer::PpdReference(ref)); value.reset(new Printer::PpdReference(ref));
} }
RebuildDetectedLists(); RebuildDetectedLists();
UpdateConfiguredPrinterURIs();
} }
// Source lists for detected printers. // Source lists for detected printers.
...@@ -421,8 +450,9 @@ class CupsPrintersManagerImpl : public CupsPrintersManager, ...@@ -421,8 +450,9 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
// reference, but have not yet gotten a response. // reference, but have not yet gotten a response.
std::unordered_set<std::string> inflight_ppd_reference_resolutions_; std::unordered_set<std::string> inflight_ppd_reference_resolutions_;
// Ids of all printers in the configured class. // Map from printer id to printers_[kConfigured] index for configured
std::unordered_set<std::string> configured_printer_ids_; // printers.
std::unordered_map<std::string, int> configured_printers_index_;
base::ObserverList<CupsPrintersManager::Observer> observer_list_; base::ObserverList<CupsPrintersManager::Observer> observer_list_;
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/md5.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "chrome/browser/chromeos/printing/ppd_provider_factory.h" #include "chrome/browser/chromeos/printing/ppd_provider_factory.h"
...@@ -43,6 +44,15 @@ namespace chromeos { ...@@ -43,6 +44,15 @@ namespace chromeos {
namespace { namespace {
// Get the URI that we want for talking to cups.
std::string URIForCups(const Printer& printer) {
if (!printer.effective_uri().empty()) {
return printer.effective_uri();
} else {
return printer.uri();
}
}
// Configures printers by downloading PPDs then adding them to CUPS through // Configures printers by downloading PPDs then adding them to CUPS through
// debugd. This class must be used on the UI thread. // debugd. This class must be used on the UI thread.
class PrinterConfigurerImpl : public PrinterConfigurer { class PrinterConfigurerImpl : public PrinterConfigurer {
...@@ -70,9 +80,8 @@ class PrinterConfigurerImpl : public PrinterConfigurer { ...@@ -70,9 +80,8 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
} }
auto* client = DBusThreadManager::Get()->GetDebugDaemonClient(); auto* client = DBusThreadManager::Get()->GetDebugDaemonClient();
client->CupsAddAutoConfiguredPrinter( client->CupsAddAutoConfiguredPrinter(
printer.id(), printer.uri(), printer.id(), URIForCups(printer),
base::Bind(&PrinterConfigurerImpl::OnAddedPrinter, base::Bind(&PrinterConfigurerImpl::OnAddedPrinter,
weak_factory_.GetWeakPtr(), printer, callback), weak_factory_.GetWeakPtr(), printer, callback),
base::Bind(&PrinterConfigurerImpl::OnDbusError, base::Bind(&PrinterConfigurerImpl::OnDbusError,
...@@ -127,7 +136,7 @@ class PrinterConfigurerImpl : public PrinterConfigurer { ...@@ -127,7 +136,7 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
auto* client = DBusThreadManager::Get()->GetDebugDaemonClient(); auto* client = DBusThreadManager::Get()->GetDebugDaemonClient();
client->CupsAddManuallyConfiguredPrinter( client->CupsAddManuallyConfiguredPrinter(
printer.id(), printer.uri(), ppd_contents, printer.id(), URIForCups(printer), ppd_contents,
base::Bind(&PrinterConfigurerImpl::OnAddedPrinter, base::Bind(&PrinterConfigurerImpl::OnAddedPrinter,
weak_factory_.GetWeakPtr(), printer, cb), weak_factory_.GetWeakPtr(), printer, cb),
base::Bind(&PrinterConfigurerImpl::OnDbusError, base::Bind(&PrinterConfigurerImpl::OnDbusError,
...@@ -214,6 +223,21 @@ class PrinterConfigurerImpl : public PrinterConfigurer { ...@@ -214,6 +223,21 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
} // namespace } // namespace
// static
std::string PrinterConfigurer::SetupFingerprint(const Printer& printer) {
base::MD5Context ctx;
base::MD5Init(&ctx);
base::MD5Update(&ctx, printer.id());
base::MD5Update(&ctx, URIForCups(printer));
base::MD5Update(&ctx, printer.ppd_reference().user_supplied_ppd_url);
base::MD5Update(&ctx, printer.ppd_reference().effective_make_and_model);
char autoconf = printer.ppd_reference().autoconf ? 1 : 0;
base::MD5Update(&ctx, std::string(&autoconf, sizeof(autoconf)));
base::MD5Digest digest;
base::MD5Final(&digest, &ctx);
return std::string(reinterpret_cast<char*>(&digest.a[0]), sizeof(digest.a));
}
// static // static
std::unique_ptr<PrinterConfigurer> PrinterConfigurer::Create(Profile* profile) { std::unique_ptr<PrinterConfigurer> PrinterConfigurer::Create(Profile* profile) {
return base::MakeUnique<PrinterConfigurerImpl>(profile); return base::MakeUnique<PrinterConfigurerImpl>(profile);
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#define CHROME_BROWSER_CHROMEOS_PRINTING_PRINTER_CONFIGURER_H_ #define CHROME_BROWSER_CHROMEOS_PRINTING_PRINTER_CONFIGURER_H_
#include <memory> #include <memory>
#include <string>
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "chromeos/printing/printer_configuration.h" #include "chromeos/printing/printer_configuration.h"
...@@ -51,6 +52,12 @@ class PrinterConfigurer { ...@@ -51,6 +52,12 @@ class PrinterConfigurer {
virtual void SetUpPrinter(const Printer& printer, virtual void SetUpPrinter(const Printer& printer,
const PrinterSetupCallback& callback) = 0; const PrinterSetupCallback& callback) = 0;
// Return an opaque fingerprint of the fields used to set up a printer with
// CUPS. The idea here is that if this fingerprint changes for a printer, we
// need to reconfigure CUPS. This fingerprint is not guaranteed to be stable
// across reboots.
static std::string SetupFingerprint(const Printer& printer);
protected: protected:
PrinterConfigurer() = default; PrinterConfigurer() = default;
}; };
......
...@@ -64,8 +64,7 @@ std::unique_ptr<Printer> SpecificsToPrinter( ...@@ -64,8 +64,7 @@ std::unique_ptr<Printer> SpecificsToPrinter(
const sync_pb::PrinterSpecifics& specifics) { const sync_pb::PrinterSpecifics& specifics) {
DCHECK(!specifics.id().empty()); DCHECK(!specifics.id().empty());
auto printer = base::MakeUnique<Printer>( auto printer = base::MakeUnique<Printer>(specifics.id());
specifics.id(), base::Time::FromJavaTime(specifics.updated_timestamp()));
printer->set_display_name(specifics.display_name()); printer->set_display_name(specifics.display_name());
printer->set_description(specifics.description()); printer->set_description(specifics.description());
printer->set_manufacturer(specifics.manufacturer()); printer->set_manufacturer(specifics.manufacturer());
......
...@@ -52,7 +52,6 @@ TEST(SpecificsTranslationTest, SpecificsToPrinter) { ...@@ -52,7 +52,6 @@ TEST(SpecificsTranslationTest, SpecificsToPrinter) {
EXPECT_EQ(kMakeAndModel, result->make_and_model()); EXPECT_EQ(kMakeAndModel, result->make_and_model());
EXPECT_EQ(kUri, result->uri()); EXPECT_EQ(kUri, result->uri());
EXPECT_EQ(kUuid, result->uuid()); EXPECT_EQ(kUuid, result->uuid());
EXPECT_EQ(kUpdateTime, result->last_updated());
EXPECT_EQ(kEffectiveMakeAndModel, EXPECT_EQ(kEffectiveMakeAndModel,
result->ppd_reference().effective_make_and_model); result->ppd_reference().effective_make_and_model);
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/chromeos/printing/printer_configurer.h"
#include "chrome/browser/chromeos/printing/printers_sync_bridge.h" #include "chrome/browser/chromeos/printing/printers_sync_bridge.h"
#include "chrome/browser/chromeos/printing/specifics_translation.h" #include "chrome/browser/chromeos/printing/specifics_translation.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -140,8 +141,8 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager { ...@@ -140,8 +141,8 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager {
} }
void PrinterInstalled(const Printer& printer) override { void PrinterInstalled(const Printer& printer) override {
DCHECK(!printer.last_updated().is_null()); installed_printer_fingerprints_[printer.id()] =
installed_printer_timestamps_[printer.id()] = printer.last_updated(); PrinterConfigurer::SetupFingerprint(printer);
// Register this printer if it's the first time we're using it. // Register this printer if it's the first time we're using it.
if (GetPrinter(printer.id()) == nullptr) { if (GetPrinter(printer.id()) == nullptr) {
...@@ -150,11 +151,11 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager { ...@@ -150,11 +151,11 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager {
} }
bool IsConfigurationCurrent(const Printer& printer) const override { bool IsConfigurationCurrent(const Printer& printer) const override {
auto found = installed_printer_timestamps_.find(printer.id()); auto found = installed_printer_fingerprints_.find(printer.id());
if (found == installed_printer_timestamps_.end()) if (found == installed_printer_fingerprints_.end())
return false; return false;
return found->second == printer.last_updated(); return found->second == PrinterConfigurer::SetupFingerprint(printer);
} }
PrintersSyncBridge* GetSyncBridge() override { return sync_bridge_.get(); } PrintersSyncBridge* GetSyncBridge() override { return sync_bridge_.get(); }
...@@ -167,7 +168,6 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager { ...@@ -167,7 +168,6 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager {
const base::ListValue* values = const base::ListValue* values =
prefs->GetList(prefs::kRecommendedNativePrinters); prefs->GetList(prefs::kRecommendedNativePrinters);
const base::Time timestamp = base::Time::Now();
// Parse the policy JSON into new structures. // Parse the policy JSON into new structures.
std::vector<std::string> new_ids; std::vector<std::string> new_ids;
...@@ -207,8 +207,7 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager { ...@@ -207,8 +207,7 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager {
if (old != enterprise_printers_.end()) { if (old != enterprise_printers_.end()) {
new_printers[id] = std::move(old->second); new_printers[id] = std::move(old->second);
} else { } else {
auto printer = auto printer = RecommendedPrinterToPrinter(*printer_dictionary);
RecommendedPrinterToPrinter(*printer_dictionary, timestamp);
printer->set_source(Printer::SRC_POLICY); printer->set_source(Printer::SRC_POLICY);
new_printers[id] = std::move(printer); new_printers[id] = std::move(printer);
...@@ -249,8 +248,9 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager { ...@@ -249,8 +248,9 @@ class SyncedPrintersManagerImpl : public SyncedPrintersManager {
std::vector<std::string> enterprise_printer_ids_; std::vector<std::string> enterprise_printer_ids_;
std::map<std::string, std::unique_ptr<Printer>> enterprise_printers_; std::map<std::string, std::unique_ptr<Printer>> enterprise_printers_;
// Map of printer ids to installation timestamps. // Map of printer ids to PrinterConfigurer setup fingerprints at the time
std::map<std::string, base::Time> installed_printer_timestamps_; // the printers was last installed with CUPS.
std::map<std::string, std::string> installed_printer_fingerprints_;
base::ObserverList<Observer> observers_; base::ObserverList<Observer> observers_;
}; };
......
...@@ -220,12 +220,12 @@ TEST_F(SyncedPrintersManagerTest, GetEnterprisePrinter) { ...@@ -220,12 +220,12 @@ TEST_F(SyncedPrintersManagerTest, GetEnterprisePrinter) {
} }
TEST_F(SyncedPrintersManagerTest, PrinterNotInstalled) { TEST_F(SyncedPrintersManagerTest, PrinterNotInstalled) {
Printer printer(kTestPrinterId, base::Time::FromInternalValue(1000)); Printer printer(kTestPrinterId);
EXPECT_FALSE(manager_->IsConfigurationCurrent(printer)); EXPECT_FALSE(manager_->IsConfigurationCurrent(printer));
} }
TEST_F(SyncedPrintersManagerTest, PrinterIsInstalled) { TEST_F(SyncedPrintersManagerTest, PrinterIsInstalled) {
Printer printer(kTestPrinterId, base::Time::FromInternalValue(1000)); Printer printer(kTestPrinterId);
manager_->PrinterInstalled(printer); manager_->PrinterInstalled(printer);
EXPECT_TRUE(manager_->IsConfigurationCurrent(printer)); EXPECT_TRUE(manager_->IsConfigurationCurrent(printer));
} }
...@@ -244,7 +244,7 @@ TEST_F(SyncedPrintersManagerTest, PrinterInstalledConfiguresPrinter) { ...@@ -244,7 +244,7 @@ TEST_F(SyncedPrintersManagerTest, PrinterInstalledConfiguresPrinter) {
// Figure out the id of the enterprise printer that was just installed. // Figure out the id of the enterprise printer that was just installed.
std::string enterprise_id = manager_->GetEnterprisePrinters().at(0).id(); std::string enterprise_id = manager_->GetEnterprisePrinters().at(0).id();
Printer configured(kTestPrinterId, base::Time::Now()); Printer configured(kTestPrinterId);
// Install a Configured printer // Install a Configured printer
manager_->UpdateConfiguredPrinter(configured); manager_->UpdateConfiguredPrinter(configured);
...@@ -256,21 +256,48 @@ TEST_F(SyncedPrintersManagerTest, PrinterInstalledConfiguresPrinter) { ...@@ -256,21 +256,48 @@ TEST_F(SyncedPrintersManagerTest, PrinterInstalledConfiguresPrinter) {
// Installing the enterprise printer should *not* generate a configuration // Installing the enterprise printer should *not* generate a configuration
// update. // update.
manager_->PrinterInstalled(Printer(enterprise_id, base::Time::Now())); manager_->PrinterInstalled(Printer(enterprise_id));
EXPECT_EQ(1U, manager_->GetConfiguredPrinters().size()); EXPECT_EQ(1U, manager_->GetConfiguredPrinters().size());
// Installing a printer we don't know about *should* generate a configuration // Installing a printer we don't know about *should* generate a configuration
// update. // update.
manager_->PrinterInstalled(Printer(kTestPrinterId2, base::Time::Now())); manager_->PrinterInstalled(Printer(kTestPrinterId2));
EXPECT_EQ(2U, manager_->GetConfiguredPrinters().size()); EXPECT_EQ(2U, manager_->GetConfiguredPrinters().size());
} }
// Test that we detect that the configuration is stale when any of the relevant
// fields change.
TEST_F(SyncedPrintersManagerTest, UpdatedPrinterConfiguration) { TEST_F(SyncedPrintersManagerTest, UpdatedPrinterConfiguration) {
Printer printer(kTestPrinterId, base::Time::FromInternalValue(1000)); Printer printer(kTestPrinterId);
manager_->PrinterInstalled(printer); manager_->PrinterInstalled(printer);
Printer updated_printer(kTestPrinterId, base::Time::FromInternalValue(2000)); Printer updated(printer);
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated_printer)); updated.set_uri("different value");
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated));
updated = printer;
updated.set_effective_uri("different value");
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated));
updated = printer;
updated.mutable_ppd_reference()->autoconf = true;
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated));
updated = printer;
updated.mutable_ppd_reference()->user_supplied_ppd_url = "different value";
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated));
updated = printer;
updated.mutable_ppd_reference()->effective_make_and_model = "different value";
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated));
updated = printer;
updated.mutable_ppd_reference()->autoconf = true;
EXPECT_FALSE(manager_->IsConfigurationCurrent(updated));
// Sanity check, configuration for the original printers should still be
// current.
EXPECT_TRUE(manager_->IsConfigurationCurrent(printer));
} }
} // namespace } // namespace
......
...@@ -121,11 +121,12 @@ std::string ZeroconfPrinterId(const ServiceDescription& service, ...@@ -121,11 +121,12 @@ std::string ZeroconfPrinterId(const ServiceDescription& service,
bool ConvertToPrinter(const ServiceDescription& service_description, bool ConvertToPrinter(const ServiceDescription& service_description,
const ParsedMetadata& metadata, const ParsedMetadata& metadata,
PrinterDetector::DetectedPrinter* detected_printer) { PrinterDetector::DetectedPrinter* detected_printer) {
// We can at least try to set up a printer if all we have is a service name // If we don't have the minimum information needed to attempt a setup, fail.
// and a protocol, but if we don't have a service name, just fail. Also fail // Also fail
// on a port of 0, as this is used to indicate that the service doesn't // on a port of 0, as this is used to indicate that the service doesn't
// *actually* exist, the device just wants to guard the name. // *actually* exist, the device just wants to guard the name.
if (service_description.service_name.empty() || if (service_description.service_name.empty() ||
service_description.ip_address.empty() ||
(service_description.address.port() == 0)) { (service_description.address.port() == 0)) {
return false; return false;
} }
...@@ -136,23 +137,31 @@ bool ConvertToPrinter(const ServiceDescription& service_description, ...@@ -136,23 +137,31 @@ bool ConvertToPrinter(const ServiceDescription& service_description,
printer.set_display_name(service_description.service_name); printer.set_display_name(service_description.service_name);
printer.set_description(metadata.note); printer.set_description(metadata.note);
printer.set_make_and_model(metadata.product); printer.set_make_and_model(metadata.product);
const char* uri_protocol;
if (service_description.service_type() == if (service_description.service_type() ==
base::StringPiece(kIppServiceName)) { base::StringPiece(kIppServiceName)) {
printer.set_uri(base::StringPrintf( uri_protocol = "ipp";
"ipp://%s/%s", service_description.address.ToString().c_str(),
metadata.rp.c_str()));
} else if (service_description.service_type() == } else if (service_description.service_type() ==
base::StringPiece(kIppsServiceName)) { base::StringPiece(kIppsServiceName)) {
printer.set_uri(base::StringPrintf( uri_protocol = "ipps";
"ipps://%s/%s", service_description.address.ToString().c_str(),
metadata.rp.c_str()));
} else { } else {
// Since we only register for these services, we should never get back // Since we only register for these services, we should never get back
// a service other than the ones above. // a service other than the ones above.
LOG(ERROR) << "Zeroconf printer with unknown service type" NOTREACHED() << "Zeroconf printer with unknown service type"
<< service_description.service_type(); << service_description.service_type();
return false; return false;
} }
printer.set_uri(base::StringPrintf(
"%s://%s/%s", uri_protocol,
service_description.address.ToString().c_str(), metadata.rp.c_str()));
// Use an effective URI with a pre-resolved ip address and port, since CUPS
// can't resolve these addresses in ChromeOS (crbug/626377).
printer.set_effective_uri(base::StringPrintf(
"%s://%s:%d/%s", uri_protocol,
service_description.ip_address.ToString().c_str(),
service_description.address.port(), metadata.rp.c_str()));
// gather ppd identification candidates. // gather ppd identification candidates.
if (!metadata.ty.empty()) { if (!metadata.ty.empty()) {
detected_printer->ppd_search_data.make_and_model.push_back(metadata.ty); detected_printer->ppd_search_data.make_and_model.push_back(metadata.ty);
......
...@@ -15,8 +15,7 @@ Printer::Printer() : source_(SRC_USER_PREFS) { ...@@ -15,8 +15,7 @@ Printer::Printer() : source_(SRC_USER_PREFS) {
id_ = base::GenerateGUID(); id_ = base::GenerateGUID();
} }
Printer::Printer(const std::string& id, const base::Time& last_updated) Printer::Printer(const std::string& id) : id_(id), source_(SRC_USER_PREFS) {
: id_(id), source_(SRC_USER_PREFS), last_updated_(last_updated) {
if (id_.empty()) if (id_.empty())
id_ = base::GenerateGUID(); id_ = base::GenerateGUID();
} }
......
...@@ -68,8 +68,8 @@ class CHROMEOS_EXPORT Printer { ...@@ -68,8 +68,8 @@ class CHROMEOS_EXPORT Printer {
// Constructs a printer object that is completely empty. // Constructs a printer object that is completely empty.
Printer(); Printer();
// Constructs a printer object with an |id| and a |last_updated| timestamp. // Constructs a printer object with the given |id|.
explicit Printer(const std::string& id, const base::Time& last_updated = {}); explicit Printer(const std::string& id);
// Copy constructor and assignment. // Copy constructor and assignment.
Printer(const Printer& printer); Printer(const Printer& printer);
...@@ -110,6 +110,11 @@ class CHROMEOS_EXPORT Printer { ...@@ -110,6 +110,11 @@ class CHROMEOS_EXPORT Printer {
const std::string& uri() const { return uri_; } const std::string& uri() const { return uri_; }
void set_uri(const std::string& uri) { uri_ = uri; } void set_uri(const std::string& uri) { uri_ = uri; }
const std::string& effective_uri() const { return effective_uri_; }
void set_effective_uri(const std::string& effective_uri) {
effective_uri_ = effective_uri;
}
const PpdReference& ppd_reference() const { return ppd_reference_; } const PpdReference& ppd_reference() const { return ppd_reference_; }
PpdReference* mutable_ppd_reference() { return &ppd_reference_; } PpdReference* mutable_ppd_reference() { return &ppd_reference_; }
...@@ -127,10 +132,6 @@ class CHROMEOS_EXPORT Printer { ...@@ -127,10 +132,6 @@ class CHROMEOS_EXPORT Printer {
Source source() const { return source_; } Source source() const { return source_; }
void set_source(const Source source) { source_ = source; } void set_source(const Source source) { source_ = source; }
// Returns the timestamp for the most recent update. Returns 0 if the
// printer was not created with a valid timestamp.
base::Time last_updated() const { return last_updated_; }
private: private:
// Globally unique identifier. Empty indicates a new printer. // Globally unique identifier. Empty indicates a new printer.
std::string id_; std::string id_;
...@@ -160,6 +161,13 @@ class CHROMEOS_EXPORT Printer { ...@@ -160,6 +161,13 @@ class CHROMEOS_EXPORT Printer {
// Contains protocol, hostname, port, and queue. // Contains protocol, hostname, port, and queue.
std::string uri_; std::string uri_;
// When non-empty, the uri to use with cups instead of uri_. This field
// is ephemeral, and not saved to sync service. This allows us to do
// on the fly rewrites of uris to work around limitations in the OS such
// as CUPS not being able to directly resolve mDNS addresses, see crbug/626377
// for details.
std::string effective_uri_;
// How to find the associated postscript printer description. // How to find the associated postscript printer description.
PpdReference ppd_reference_; PpdReference ppd_reference_;
...@@ -168,9 +176,6 @@ class CHROMEOS_EXPORT Printer { ...@@ -168,9 +176,6 @@ class CHROMEOS_EXPORT Printer {
// The datastore which holds this printer. // The datastore which holds this printer.
Source source_; Source source_;
// Timestamp of most recent change.
base::Time last_updated_;
}; };
} // namespace chromeos } // namespace chromeos
......
...@@ -83,17 +83,14 @@ bool DictionaryToPrinter(const DictionaryValue& value, Printer* printer) { ...@@ -83,17 +83,14 @@ bool DictionaryToPrinter(const DictionaryValue& value, Printer* printer) {
const char kPrinterId[] = "id"; const char kPrinterId[] = "id";
std::unique_ptr<Printer> RecommendedPrinterToPrinter( std::unique_ptr<Printer> RecommendedPrinterToPrinter(
const base::DictionaryValue& pref, const base::DictionaryValue& pref) {
const base::Time& timestamp) {
DCHECK(!timestamp.is_null());
std::string id; std::string id;
if (!pref.GetString(kPrinterId, &id)) { if (!pref.GetString(kPrinterId, &id)) {
LOG(WARNING) << "Record id required"; LOG(WARNING) << "Record id required";
return nullptr; return nullptr;
} }
std::unique_ptr<Printer> printer = base::MakeUnique<Printer>(id, timestamp); std::unique_ptr<Printer> printer = base::MakeUnique<Printer>(id);
if (!DictionaryToPrinter(pref, printer.get())) { if (!DictionaryToPrinter(pref, printer.get())) {
LOG(WARNING) << "Failed to parse policy printer."; LOG(WARNING) << "Failed to parse policy printer.";
return nullptr; return nullptr;
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
namespace base { namespace base {
class DictionaryValue; class DictionaryValue;
class Time;
} }
namespace chromeos { namespace chromeos {
...@@ -22,8 +21,7 @@ CHROMEOS_EXPORT extern const char kPrinterId[]; ...@@ -22,8 +21,7 @@ CHROMEOS_EXPORT extern const char kPrinterId[];
// Returns a new printer populated with the fields from |pref|. Processes // Returns a new printer populated with the fields from |pref|. Processes
// dictionaries from policy i.e. cPanel. // dictionaries from policy i.e. cPanel.
CHROMEOS_EXPORT std::unique_ptr<Printer> RecommendedPrinterToPrinter( CHROMEOS_EXPORT std::unique_ptr<Printer> RecommendedPrinterToPrinter(
const base::DictionaryValue& pref, const base::DictionaryValue& pref);
const base::Time& timestamp);
} // namespace chromeos } // namespace chromeos
......
...@@ -25,15 +25,12 @@ const char kMake[] = "Chrome"; ...@@ -25,15 +25,12 @@ const char kMake[] = "Chrome";
const char kModel[] = "Inktastic Laser Magic"; const char kModel[] = "Inktastic Laser Magic";
const char kMakeAndModel[] = "Chrome Inktastic Laser Magic"; const char kMakeAndModel[] = "Chrome Inktastic Laser Magic";
const base::Time kTimestamp = base::Time::FromInternalValue(445566);
// PpdReference test data // PpdReference test data
const char kEffectiveMakeAndModel[] = "PrintBlaster LazerInker 2000"; const char kEffectiveMakeAndModel[] = "PrintBlaster LazerInker 2000";
TEST(PrinterTranslatorTest, RecommendedPrinterToPrinterMissingId) { TEST(PrinterTranslatorTest, RecommendedPrinterToPrinterMissingId) {
base::DictionaryValue value; base::DictionaryValue value;
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(value);
RecommendedPrinterToPrinter(value, kTimestamp);
EXPECT_FALSE(printer); EXPECT_FALSE(printer);
} }
...@@ -45,8 +42,7 @@ TEST(PrinterTranslatorTest, MissingDisplayNameFails) { ...@@ -45,8 +42,7 @@ TEST(PrinterTranslatorTest, MissingDisplayNameFails) {
preference.SetString("uri", kUri); preference.SetString("uri", kUri);
preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel); preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel);
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_FALSE(printer); EXPECT_FALSE(printer);
} }
...@@ -57,8 +53,7 @@ TEST(PrinterTranslatorTest, MissingUriFails) { ...@@ -57,8 +53,7 @@ TEST(PrinterTranslatorTest, MissingUriFails) {
// uri omitted // uri omitted
preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel); preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel);
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_FALSE(printer); EXPECT_FALSE(printer);
} }
...@@ -69,8 +64,7 @@ TEST(PrinterTranslatorTest, MissingPpdResourceFails) { ...@@ -69,8 +64,7 @@ TEST(PrinterTranslatorTest, MissingPpdResourceFails) {
preference.SetString("uri", kUri); preference.SetString("uri", kUri);
// ppd resource omitted // ppd resource omitted
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_FALSE(printer); EXPECT_FALSE(printer);
} }
...@@ -81,8 +75,7 @@ TEST(PrinterTranslatorTest, MissingEffectiveMakeModelFails) { ...@@ -81,8 +75,7 @@ TEST(PrinterTranslatorTest, MissingEffectiveMakeModelFails) {
preference.SetString("uri", kUri); preference.SetString("uri", kUri);
preference.SetString("ppd_resource.foobarwrongfield", "gibberish"); preference.SetString("ppd_resource.foobarwrongfield", "gibberish");
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_FALSE(printer); EXPECT_FALSE(printer);
} }
...@@ -93,8 +86,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterMinimalSetup) { ...@@ -93,8 +86,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterMinimalSetup) {
preference.SetString("uri", kUri); preference.SetString("uri", kUri);
preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel); preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel);
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_TRUE(printer); EXPECT_TRUE(printer);
} }
...@@ -110,8 +102,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinter) { ...@@ -110,8 +102,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinter) {
preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel); preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel);
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_TRUE(printer); EXPECT_TRUE(printer);
EXPECT_EQ(kHash, printer->id()); EXPECT_EQ(kHash, printer->id());
...@@ -122,7 +113,6 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinter) { ...@@ -122,7 +113,6 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinter) {
EXPECT_EQ(kMakeAndModel, printer->make_and_model()); EXPECT_EQ(kMakeAndModel, printer->make_and_model());
EXPECT_EQ(kUri, printer->uri()); EXPECT_EQ(kUri, printer->uri());
EXPECT_EQ(kUUID, printer->uuid()); EXPECT_EQ(kUUID, printer->uuid());
EXPECT_EQ(kTimestamp, printer->last_updated());
EXPECT_EQ(kEffectiveMakeAndModel, EXPECT_EQ(kEffectiveMakeAndModel,
printer->ppd_reference().effective_make_and_model); printer->ppd_reference().effective_make_and_model);
...@@ -136,8 +126,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinterBlankManufacturer) { ...@@ -136,8 +126,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinterBlankManufacturer) {
preference.SetString("uri", kUri); preference.SetString("uri", kUri);
preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel); preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel);
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_TRUE(printer); EXPECT_TRUE(printer);
EXPECT_EQ(kModel, printer->model()); EXPECT_EQ(kModel, printer->model());
...@@ -152,8 +141,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinterBlankModel) { ...@@ -152,8 +141,7 @@ TEST(PrinterTranslatorTest, RecommendedPrinterToPrinterBlankModel) {
preference.SetString("uri", kUri); preference.SetString("uri", kUri);
preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel); preference.SetString("ppd_resource.effective_model", kEffectiveMakeAndModel);
std::unique_ptr<Printer> printer = std::unique_ptr<Printer> printer = RecommendedPrinterToPrinter(preference);
RecommendedPrinterToPrinter(preference, kTimestamp);
EXPECT_TRUE(printer); EXPECT_TRUE(printer);
EXPECT_EQ(kMake, printer->manufacturer()); EXPECT_EQ(kMake, printer->manufacturer());
......
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