New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 769893 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Consolidate uri parsing in printer_configuration.cc

Project Member Reported by skau@chromium.org, Sep 28 2017

Issue description

We do a lot of uri parsing to breakup and reconstruct printer uris.  We'd like to use GURL but ipp:// is a non-standard scheme.  The next best thing is to consolidate all of this code in one place where it's obviously correct.
 

Comment 1 by skau@chromium.org, Dec 1 2017

Cc: skau@chromium.org
Owner: valleau@chromium.org
Status: Assigned (was: Untriaged)
Refactoring task.  No behavior changes are expected.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01fa272e0a17b14506935fd0cecd208c4a8eed0b

commit 01fa272e0a17b14506935fd0cecd208c4a8eed0b
Author: David Valleau <valleau@chromium.org>
Date: Wed Jan 17 22:36:49 2018

Moving some relevant URI parsing functions into printer_configuration

R=skau@chromium.org

Bug:  769893 
Change-Id: Ie097ae87ebd4da78c95171a79cef24501bd66883
Reviewed-on: https://chromium-review.googlesource.com/861956
Commit-Queue: David Valleau <valleau@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529927}
[modify] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chrome/browser/chromeos/printing/printer_configurer.cc
[modify] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chromeos/BUILD.gn
[modify] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chromeos/printing/printer_configuration.cc
[modify] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chromeos/printing/printer_configuration.h
[modify] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chromeos/printing/printing_constants.h
[add] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chromeos/printing/uri_components.cc
[add] https://crrev.com/01fa272e0a17b14506935fd0cecd208c4a8eed0b/chromeos/printing/uri_components.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68d7f4c648c22483915b993e6bca232fb59463c7

commit 68d7f4c648c22483915b993e6bca232fb59463c7
Author: Sorin Jianu <sorin@chromium.org>
Date: Thu Jan 18 01:31:41 2018

Revert "Moving some relevant URI parsing functions into printer_configuration"

This reverts commit 01fa272e0a17b14506935fd0cecd208c4a8eed0b.

Reason for revert: suspecting build break due to link error:

cups_printers_handler.cc:function chromeos::settings::(anonymous namespace)::QueryAutoconf

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.chromiumos%2Flinux-chromeos-dbg%2F3797%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

Original change's description:
> Moving some relevant URI parsing functions into printer_configuration
> 
> R=​skau@chromium.org
> 
> Bug:  769893 
> Change-Id: Ie097ae87ebd4da78c95171a79cef24501bd66883
> Reviewed-on: https://chromium-review.googlesource.com/861956
> Commit-Queue: David Valleau <valleau@chromium.org>
> Reviewed-by: Hector Carmona <hcarmona@chromium.org>
> Reviewed-by: Sean Kau <skau@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#529927}

TBR=hcarmona@chromium.org,skau@chromium.org,valleau@chromium.org

Change-Id: I57954406b2909923930c9c7b2e28f8d10bf16bd2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  769893 
Reviewed-on: https://chromium-review.googlesource.com/872051
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529998}
[modify] https://crrev.com/68d7f4c648c22483915b993e6bca232fb59463c7/chrome/browser/chromeos/printing/printer_configurer.cc
[modify] https://crrev.com/68d7f4c648c22483915b993e6bca232fb59463c7/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/68d7f4c648c22483915b993e6bca232fb59463c7/chromeos/BUILD.gn
[modify] https://crrev.com/68d7f4c648c22483915b993e6bca232fb59463c7/chromeos/printing/printer_configuration.cc
[modify] https://crrev.com/68d7f4c648c22483915b993e6bca232fb59463c7/chromeos/printing/printer_configuration.h
[modify] https://crrev.com/68d7f4c648c22483915b993e6bca232fb59463c7/chromeos/printing/printing_constants.h
[delete] https://crrev.com/a8095a7eeb8d9d342e4dd452c206146a201778e2/chromeos/printing/uri_components.cc
[delete] https://crrev.com/a8095a7eeb8d9d342e4dd452c206146a201778e2/chromeos/printing/uri_components.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jan 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2

commit 03e6fa6bbf75680658d9c30b9a5e48cf63232cb2
Author: David Valleau <valleau@chromium.org>
Date: Fri Jan 19 02:54:26 2018

Reland "Moving some relevant URI parsing functions..."

https://chromium-review.googlesource.com/c/chromium/src/+/861956

Fixing link error caused by the free ParseUri function in
printer_configuration.h not being exported properly.

This reverts commit 68d7f4c648c22483915b993e6bca232fb59463c7.

Bug:  769893 
Change-Id: I02a482272bf97e7166c221448dd2444bece00d03
Reviewed-on: https://chromium-review.googlesource.com/872265
Commit-Queue: David Valleau <valleau@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530404}
[modify] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chrome/browser/chromeos/printing/printer_configurer.cc
[modify] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc
[modify] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chromeos/BUILD.gn
[modify] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chromeos/printing/printer_configuration.cc
[modify] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chromeos/printing/printer_configuration.h
[modify] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chromeos/printing/printing_constants.h
[add] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chromeos/printing/uri_components.cc
[add] https://crrev.com/03e6fa6bbf75680658d9c30b9a5e48cf63232cb2/chromeos/printing/uri_components.h

Comment 5 by skau@chromium.org, Feb 9 2018

Status: Fixed (was: Assigned)

Sign in to add a comment