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

Issue 666740 link

Starred by 1 user
Status: Assigned
Owner:
thestig@chromium.org
Cc:
█ scottbyer@chromium.org
vitalyb...@chromium.org
█ gene@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

enable_service_discovery depends on enable_basic_printing/enable_print_preview

Project Member Reported by raphael....@intel.com, Nov 18 2016 
I'm testing a build with both enable_basic_printing and enable_print_preview set to false, but linking the final chrome binary fails:

obj/chrome/browser/libbrowser.a(cloud_print_printer_list.o):../../chrome/browser/printing/cloud_print/cloud_print_printer_list.cc:function cloud_print::CloudPrintPrinterList::OnGCDApiFlowComplete(base::Diction
aryValue const&): error: undefined reference to 'cloud_print::kPrinterListValue'
obj/chrome/browser/libbrowser.a(cloud_print_printer_list.o):../../chrome/browser/printing/cloud_print/cloud_print_printer_list.cc:function cloud_print::CloudPrintPrinterList::OnGCDApiFlowComplete(base::Dictio$
aryValue const&): error: undefined reference to 'cloud_print::kIdValue'
obj/chrome/browser/libbrowser.a(cloud_print_printer_list.o):../../chrome/browser/printing/cloud_print/cloud_print_printer_list.cc:function cloud_print::CloudPrintPrinterList::OnGCDApiFlowComplete(base::Diction
aryValue const&): error: undefined reference to 'cloud_print::kDisplayNameValue'
obj/chrome/browser/libbrowser.a(cloud_print_printer_list.o):../../chrome/browser/printing/cloud_print/cloud_print_printer_list.cc:function cloud_print::CloudPrintPrinterList::OnGCDApiFlowComplete(base::Diction
aryValue const&): error: undefined reference to 'cloud_print::kPrinterDescValue'
obj/chrome/browser/libbrowser.a(gcd_api_flow.o):../../chrome/browser/printing/cloud_print/gcd_api_flow.cc:function cloud_print::CloudPrintApiFlowRequest::GetExtraRequestHeaders(): error: undefined reference to
 'cloud_print::kChromeCloudPrintProxyHeader'
obj/chrome/browser/libbrowser.a(privet_confirm_api_flow.o):../../chrome/browser/printing/cloud_print/privet_confirm_api_flow.cc:function cloud_print::PrivetConfirmApiCallFlow::OnGCDApiFlowComplete(base::Dictio
naryValue const&): error: undefined reference to 'cloud_print::kSuccessValue'

This error is caused by the enable_service_discovery bits in //chrome/build/BUILD.gn including some files in //chrome/build/printing/cloud_print that depend on //chrome/common/cloud_print/cloud_print_constants.{cc,h}, which are only built if at least one of enable_basic_printing or enable_print_preview is enabled (//chrome/common/BUILD.gn).

While unconditionally building those two files is easy enough, I'm filing the bug to determine if this is a valid solution in the first place; should enable_service_discovery depend on the printing variables (or at least the cloud_print bits) instead, for example?

Comment 1 by thestig@chromium.org, Nov 18 2016 

I've known about this but I didn't know anyone cared. Locally I have this "no printing" Linux build config:

enable_print_preview = false
enable_basic_printing = false
enable_mdns = false

Comment 2 by raphael....@intel.com, Nov 22 2016 

I admit this isn't a popular use case, but I've been experimenting with different options to see how they influence the final binary size, and stumbled upon this issue.

Given Monorail says one of the owners has never logged in and another has been absent for almost 20 days, is there anyone else I should CC here?

Comment 3 by thestig@chromium.org, Nov 22 2016 

I subscribe to printing bug labels, though I don't have time to help. If this is importand and you want to try untangling this, please give it a try. If you are just experimenting with binary sizes, try the comment 1 suggestion?

Comment 4 by raphael....@intel.com, Nov 25 2016 

I'd like to send a CL to get things working as expected, but what "expected" means here is not totally clear to me: should cloud_print_constants.{cc,h} be built regardless of the build flags, should enable_service_discovery depend on enable_print_preview/enable_basic_printing or something else?

Comment 5 by thestig@chromium.org, Mar 9 2017

Status: Available (was: Untriaged)
We may want to move some of cloud_print_constants out into chrome/service, since it's probably the only user of those constants. I'm not 100% certain, but having it in chrome/common means on Windows both chrome.dll and chrome_child.dll has a copy, unless some linker optimization magic can figure out it's unused in one and remove it. If we move it, then it's tied to the conditionals for chrome/service as a whole.

enable_service_discovery probably should depend on enable_print_preview. Without print preview, there's no way to show the UI for cloud print.
Project Member

Comment 6 by sheriffbot@chromium.org, Mar 9 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 7 by thestig@chromium.org, Apr 3 2018

Labels: -Hotlist-Recharge-Cold
Owner: thestig@chromium.org
I think Gene said to just remove enable_service_discovery and make it a runtime flag.

Comment 8 by benhenry@chromium.org, Aug 3

Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".
►

Sign in to add a comment