Removing use of effective_uri from CUPS printing code

Now that mdns resolutions is supported in CUPS, we don't need to hold
the resolved uri in the printer object anymore.

BUG=758018,936251
TEST=Able to configure and print using mdns resolution in CUPS

Change-Id: I2f149e6558be4689e7b13122a5853feba7ea6b8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1551683
Commit-Queue: Sean Kau <skau@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648366}

chrome/browser/chromeos/printing/cups_printers_manager.cc

@@ -256,7 +256,6 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
     printers_[kConfigured] = synced_printers_manager_->GetConfiguredPrinters();
     RebuildConfiguredPrintersIndex();
     RebuildDetectedLists();
-    UpdateConfiguredPrinterURIs();
     NotifyObservers({kConfigured});
   }
 
@@ -290,13 +289,6 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
         break;
     }
     RebuildDetectedLists();
-
-    // We may have new URI information for a configured printer in the changed
-    // detected list.  If we do, pass the updated information along to
-    // observers.
-    if (UpdateConfiguredPrinterURIs()) {
-      NotifyObservers({kConfigured});
-    }
   }
 
  private:
@@ -328,31 +320,6 @@ class CupsPrintersManagerImpl : public CupsPrintersManager,
     }
   }
 
-  // Cross reference the Configured printers with the raw detected printer
-  // lists.  Returns true if any entries in the configured printers list
-  // changed as a result of this cross referencing, false otherwise.
-  bool UpdateConfiguredPrinterURIs() {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_);
-    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;
-          }
-        }
-      }
-    }
-    return updated;
-  }
-
   // 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.
   const PrinterDetector::DetectedPrinter* FindDetectedPrinter(

chrome/browser/chromeos/printing/printer_configurer.cc

@@ -119,11 +119,6 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
     DCHECK(!printer.uri().empty());
     PRINTER_LOG(USER) << printer.make_and_model() << " Printer setup requested";
 
-    if (!printer.RequiresIpResolution()) {
-      StartConfiguration(printer, std::move(callback));
-      return;
-    }
-
     // Ensure that |address| is non-empty before attempting to resolve it.
     // If the uri in |printer| does not contain both a hostname and a port
     // number then GetHostAndPort() will return an empty string.
@@ -167,7 +162,7 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
                        << " Attempting autoconf setup";
     auto* client = DBusThreadManager::Get()->GetDebugDaemonClient();
     client->CupsAddAutoConfiguredPrinter(
-        printer.id(), printer.UriForCups(),
+        printer.id(), printer.uri(),
         base::BindOnce(&PrinterConfigurerImpl::OnAddedPrinter,
                        weak_factory_.GetWeakPtr(), printer,
                        std::move(callback)));
@@ -193,9 +188,6 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
 
     PRINTER_LOG(EVENT) << printer->make_and_model()
                        << " IP Resolution succeeded";
-    std::string effective_uri = printer->ReplaceHostAndPort(endpoint);
-    printer->set_effective_uri(effective_uri);
-
     StartConfiguration(*printer, std::move(cb));
   }
 
@@ -222,7 +214,7 @@ class PrinterConfigurerImpl : public PrinterConfigurer {
 
     PRINTER_LOG(EVENT) << printer.make_and_model() << " Manual printer setup";
     client->CupsAddManuallyConfiguredPrinter(
-        printer.id(), printer.UriForCups(), ppd_contents,
+        printer.id(), printer.uri(), ppd_contents,
         base::BindOnce(&PrinterConfigurerImpl::OnAddedPrinter,
                        weak_factory_.GetWeakPtr(), printer, std::move(cb)));
   }
@@ -316,7 +308,7 @@ std::string PrinterConfigurer::SetupFingerprint(const Printer& printer) {
   base::MD5Context ctx;
   base::MD5Init(&ctx);
   base::MD5Update(&ctx, printer.id());
-  base::MD5Update(&ctx, printer.UriForCups());
+  base::MD5Update(&ctx, printer.uri());
   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;

chrome/browser/chromeos/printing/synced_printers_manager_unittest.cc

@@ -290,10 +290,6 @@ TEST_F(SyncedPrintersManagerTest, UpdatedPrinterConfiguration) {
   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));

chrome/browser/chromeos/printing/zeroconf_printer_detector.cc

@@ -165,13 +165,6 @@ bool ConvertToPrinter(const std::string& service_type,
       "%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()));
-
   // Per the IPP Everywhere Standard 5100.14-2013, section 4.2.1, IPP
   // everywhere-capable printers advertise services prefixed with "_print"
   // (possibly in addition to prefix-free versions).  If we get a printer from a

chrome/browser/chromeos/printing/zeroconf_printer_detector_unittest.cc

@@ -71,9 +71,6 @@ PrinterDetector::DetectedPrinter MakeExpectedPrinter(const std::string& name,
   net::IPAddress ip_address = GetIPAddressFor(name);
   int port = GetPortFor(name);
   bool ssl = flags & kFlagSSL;
-  printer.set_effective_uri(
-      base::StringPrintf("ipp%s://%s:%d/%s_rp", ssl ? "s" : "",
-                         ip_address.ToString().c_str(), port, name.c_str()));
   printer.set_uri(base::StringPrintf("ipp%s://%s.local:%d/%s_rp",
                                      ssl ? "s" : "", name.c_str(), port,
                                      name.c_str()));
@@ -372,7 +369,6 @@ class ZeroconfPrinterDetectorTest : public testing::Test,
 
   void ExpectPrinterEq(const PrinterDetector::DetectedPrinter& expected,
                        const PrinterDetector::DetectedPrinter& actual) {
-    EXPECT_EQ(expected.printer.effective_uri(), actual.printer.effective_uri());
     EXPECT_EQ(expected.printer.uri(), actual.printer.uri());
     // We don't have a good way to directly check for an expected id.
     EXPECT_EQ(expected.printer.uuid(), actual.printer.uuid());

chromeos/printing/printer_configuration.cc

@@ -117,20 +117,6 @@ bool Printer::IsIppEverywhere() const {
   return ppd_reference_.autoconf;
 }
 
-bool Printer::RequiresIpResolution() const {
-  if (effective_uri_.empty() &&
-      base::StartsWith(id_, "zeroconf-", base::CompareCase::SENSITIVE)) {
-    // Check to see if |uri_| is a contains a ".local" hostname. This is to
-    // catch the case where a user edits the address of an existing configured
-    // zeroconf printer to a static IP address.
-    base::Optional<UriComponents> components_optional = ParseUri(uri_);
-    UriComponents uri_components = components_optional.value();
-    return base::EndsWith(uri_components.host(), ".local",
-                          base::CompareCase::SENSITIVE);
-  }
-  return false;
-}
-
 net::HostPortPair Printer::GetHostAndPort() const {
   if (uri_.empty()) {
     return net::HostPortPair();
@@ -199,10 +185,6 @@ bool Printer::HasNetworkProtocol() const {
   }
 }
 
-std::string Printer::UriForCups() const {
-  return effective_uri_.empty() ? uri_ : effective_uri_;
-}
-
 base::Optional<UriComponents> Printer::GetUriComponents() const {
   return chromeos::ParseUri(uri_);
 }

chromeos/printing/printer_configuration.h

@@ -119,11 +119,6 @@ class CHROMEOS_EXPORT Printer {
   const std::string& uri() const { return 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_; }
   PpdReference* mutable_ppd_reference() { return &ppd_reference_; }
 
@@ -140,10 +135,6 @@ class CHROMEOS_EXPORT Printer {
   // |uri_|.
   bool IsIppEverywhere() const;
 
-  // Returns true if |effective_uri_| needs to be computed before the printer
-  // can be installed.
-  bool RequiresIpResolution() const;
-
   // Returns the hostname and port for |uri_|.  Assumes that the uri is
   // well formed.  Returns an empty string if |uri_| is not set.
   net::HostPortPair GetHostAndPort() const;
@@ -163,9 +154,6 @@ class CHROMEOS_EXPORT Printer {
   Source source() const { return source_; }
   void set_source(const Source source) { source_ = source; }
 
-  // Get the URI that we want for talking to cups.
-  std::string UriForCups() const;
-
   // Parses the printers's uri into its components and returns an optional
   // containing a UriComponents object depending on whether or not the uri was
   // successfully parsed.

chromeos/printing/printer_configuration_unittest.cc

@@ -84,42 +84,4 @@ TEST(PrinterConfigurationTest, ParseUriSubdomainQueueAndPort) {
   EXPECT_EQ(result->path(), "/ipp/print");
 }
 
-TEST(PrinterConfigurationTest, RequiresIpResolution) {
-  chromeos::Printer printer;
-  printer.set_effective_uri("");
-  printer.set_id("zeroconf-testid");
-  printer.set_uri("ipp://test.local:631/ipp/print");
-  EXPECT_TRUE(printer.RequiresIpResolution());
-}
-
-// In this case we expect RequiresIpResolution() to return false since
-// |effective_uri_| has already been computed.
-TEST(PrinterConfigurationTest, RequiresIpResolutionNonEmptyEffectiveUri) {
-  chromeos::Printer printer;
-  printer.set_effective_uri("test.local:631");
-  printer.set_id("zeroconf-testid");
-  printer.set_uri("ipp://test.local:631/ipp/print");
-  EXPECT_FALSE(printer.RequiresIpResolution());
-}
-
-// In this case we expect RequiresIpResolution() to return false since |id_|
-// does not have the "zeroconf-" prefix.
-TEST(PrinterConfigurationTest, RequiresIpResolutionNotZeroconf) {
-  chromeos::Printer printer;
-  printer.set_effective_uri("");
-  printer.set_id("testid");
-  printer.set_uri("ipp://test.local:631/ipp/print");
-  EXPECT_FALSE(printer.RequiresIpResolution());
-}
-
-// In this case we expect RequiresIpResolution() to return false since the
-// hostname in |uri_| does not end with ".local".
-TEST(PrinterConfigurationTest, RequiresIpResolutionNoLocal) {
-  chromeos::Printer printer;
-  printer.set_effective_uri("");
-  printer.set_id("zeroconf-testid");
-  printer.set_uri("ipp://localhost:631/ipp/print");
-  EXPECT_FALSE(printer.RequiresIpResolution());
-}
-
 }  // namespace