Commit 33288b98 authored by Jimmy Gong's avatar Jimmy Gong Committed by Commit Bot

Fix printer URI generation for printer EULA links

- Previously we appended the license to the os-credits page URL.
- This is incorrect as we need to create the URL with the license
  as a reference fragment. eg. chrome://os-credits/#license
- Update tests to reflect this.

Bug: 1044646
Test: unit + browser tests
Change-Id: I0bbb8067cbffdbcd14ab1ff4b37014c8ef7424ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015674
Commit-Queue: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarBailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736208}
parent c7c8237b
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "chrome/browser/component_updater/cros_component_installer_chromeos.h" #include "chrome/browser/component_updater/cros_component_installer_chromeos.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/common/webui_url_constants.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/debug_daemon/debug_daemon_client.h" #include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
#include "chromeos/printing/ppd_provider.h" #include "chromeos/printing/ppd_provider.h"
...@@ -300,6 +301,15 @@ void PrinterConfigurer::SetPrinterConfigurerForTesting( ...@@ -300,6 +301,15 @@ void PrinterConfigurer::SetPrinterConfigurerForTesting(
g_printer_configurer_for_test = printer_configurer.release(); g_printer_configurer_for_test = printer_configurer.release();
} }
// static
GURL PrinterConfigurer::GeneratePrinterEulaUrl(const std::string& license) {
GURL eula_url(chrome::kChromeUIOSCreditsURL);
// Construct the URL with proper reference fragment.
GURL::Replacements replacements;
replacements.SetRefStr(license);
return eula_url.ReplaceComponents(replacements);
}
std::ostream& operator<<(std::ostream& out, const PrinterSetupResult& result) { std::ostream& operator<<(std::ostream& out, const PrinterSetupResult& result) {
switch (result) { switch (result) {
case kFatalError: case kFatalError:
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/macros.h" #include "base/macros.h"
#include "url/gurl.h"
class Profile; class Profile;
...@@ -95,6 +96,10 @@ class PrinterConfigurer { ...@@ -95,6 +96,10 @@ class PrinterConfigurer {
static void SetPrinterConfigurerForTesting( static void SetPrinterConfigurerForTesting(
std::unique_ptr<PrinterConfigurer> printer_configurer); std::unique_ptr<PrinterConfigurer> printer_configurer);
// Returns a generated EULA GURL for the provided |license|. |license| is the
// identifier tag of the printer's license information.
static GURL GeneratePrinterEulaUrl(const std::string& license);
protected: protected:
PrinterConfigurer() = default; PrinterConfigurer() = default;
......
...@@ -26,7 +26,6 @@ ...@@ -26,7 +26,6 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/print_preview/print_preview_utils.h" #include "chrome/browser/ui/webui/print_preview/print_preview_utils.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/common/webui_url_constants.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/debug_daemon/debug_daemon_client.h" #include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
#include "chromeos/printing/printer_configuration.h" #include "chromeos/printing/printer_configuration.h"
...@@ -267,7 +266,7 @@ void LocalPrinterHandlerChromeos::OnResolvedEulaUrl( ...@@ -267,7 +266,7 @@ void LocalPrinterHandlerChromeos::OnResolvedEulaUrl(
return; return;
} }
GURL eula_url(chrome::kChromeUIOSCreditsURL + license); GURL eula_url = chromeos::PrinterConfigurer::GeneratePrinterEulaUrl(license);
std::move(cb).Run(eula_url.spec()); std::move(cb).Run(eula_url.spec());
} }
......
...@@ -346,7 +346,7 @@ TEST_F(LocalPrinterHandlerChromeosTest, StartFetchValidEulaUrl) { ...@@ -346,7 +346,7 @@ TEST_F(LocalPrinterHandlerChromeosTest, StartFetchValidEulaUrl) {
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
EXPECT_EQ(fetched_eula_url, "chrome://os-credits/expected_make_model"); EXPECT_EQ(fetched_eula_url, "chrome://os-credits/#expected_make_model");
} }
// Test that a printer with no PPD license will return an empty string. // Test that a printer with no PPD license will return an empty string.
......
...@@ -39,7 +39,6 @@ ...@@ -39,7 +39,6 @@
#include "chrome/browser/ui/chrome_select_file_policy.h" #include "chrome/browser/ui/chrome_select_file_policy.h"
#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_paths.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "chrome/common/webui_url_constants.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/debug_daemon/debug_daemon_client.h" #include "chromeos/dbus/debug_daemon/debug_daemon_client.h"
#include "chromeos/printing/ppd_line_reader.h" #include "chromeos/printing/ppd_line_reader.h"
...@@ -1240,7 +1239,7 @@ void CupsPrintersHandler::OnGetEulaUrl(const std::string& callback_id, ...@@ -1240,7 +1239,7 @@ void CupsPrintersHandler::OnGetEulaUrl(const std::string& callback_id,
return; return;
} }
GURL eula_url(chrome::kChromeUIOSCreditsURL + license); GURL eula_url = PrinterConfigurer::GeneratePrinterEulaUrl(license);
ResolveJavascriptCallback( ResolveJavascriptCallback(
base::Value(callback_id), base::Value(callback_id),
eula_url.is_valid() ? base::Value(eula_url.spec()) : base::Value()); eula_url.is_valid() ? base::Value(eula_url.spec()) : base::Value());
......
...@@ -382,10 +382,7 @@ suite('CupsAddPrinterDialogTests', function() { ...@@ -382,10 +382,7 @@ suite('CupsAddPrinterDialogTests', function() {
addDialog.$$('.action-button').click(); addDialog.$$('.action-button').click();
Polymer.dom.flush(); Polymer.dom.flush();
const eulaLink = 'google'; const expectedEulaLink = 'chrome://os-credits/#google';
const path = window.location.pathname;
const expectedEulaLink = window.location.origin +
path.slice(0, path.lastIndexOf('/') + 1) + eulaLink;
const expectedManufacturer = 'Google'; const expectedManufacturer = 'Google';
const expectedModel = 'printer'; const expectedModel = 'printer';
const expectedModel2 = 'newPrinter'; const expectedModel2 = 'newPrinter';
...@@ -405,7 +402,7 @@ suite('CupsAddPrinterDialogTests', function() { ...@@ -405,7 +402,7 @@ suite('CupsAddPrinterDialogTests', function() {
// Check that the EULA text is not shown. // Check that the EULA text is not shown.
assertTrue(urlElement.hidden); assertTrue(urlElement.hidden);
cupsPrintersBrowserProxy.setEulaUrl(eulaLink); cupsPrintersBrowserProxy.setEulaUrl(expectedEulaLink);
modelDialog.$$('#manufacturerDropdown').value = expectedManufacturer; modelDialog.$$('#manufacturerDropdown').value = expectedManufacturer;
modelDropdown = modelDialog.$$('#modelDropdown'); modelDropdown = modelDialog.$$('#modelDropdown');
...@@ -429,7 +426,7 @@ suite('CupsAddPrinterDialogTests', function() { ...@@ -429,7 +426,7 @@ suite('CupsAddPrinterDialogTests', function() {
// Check that the EULA text is hidden. // Check that the EULA text is hidden.
assertTrue(urlElement.hidden); assertTrue(urlElement.hidden);
resetGetEulaUrl(cupsPrintersBrowserProxy, eulaLink); resetGetEulaUrl(cupsPrintersBrowserProxy, expectedEulaLink);
// Change ppdModel and expect |getEulaUrl| to be called again. // Change ppdModel and expect |getEulaUrl| to be called again.
modelDropdown.value = expectedModel3; modelDropdown.value = expectedModel3;
......
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