Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
288003004:
provide a build time flag to enable connecting to production wallet servers by default (Closed)
|
Created:
6 years, 7 months ago by Evan Stade Modified:
6 years, 7 months ago Reviewers:
Ilya Sherman CC:
chromium-reviews, benquan, jam, browser-components-watch_chromium.org, darin-cc_chromium.org, Dane Wallinga, dyu1, estade+watch_chromium.org, rouslan+autofillwatch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Descriptionprovide a build time flag to enable connecting to production wallet servers by default
This flag defaults to true for official builds and false for unofficial builds. If the flag is false, Chromium will connect to sandbox wallet servers.
Net effect: instead of being controller by Chrome vs. Chromium, the default is controlled by Official vs. Unofficial buildtype.
BUG=334088
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272348
Patch Set 1 #Patch Set 2 : hinge on buildtype instead of branding #
Messages
Total messages: 18 (0 generated)
Have we actually heard from Linux distros as to whether they want to have Wallet enabled vs. disabled? > Ideally there should be a way for Chromium distros that don't want > Wallet to remove all traces of Wallet instead of just setting the URL to > a sandbox server, but that would require a bigger change. How much bigger? Surely there are only a few places where we'd need to short-circuit the code. Like, if we don't try to fetch Wallet items when the dialog is opened, default to Autofill being selected, and don't draw the "sign in to wallet" link, does that fully disable Wallet?
On 2014/05/15 00:31:27, Ilya Sherman wrote: > Have we actually heard from Linux distros as to whether they want to have Wallet > enabled vs. disabled? > > > Ideally there should be a way for Chromium distros that don't want > > Wallet to remove all traces of Wallet instead of just setting the URL to > > a sandbox server, but that would require a bigger change. > > How much bigger? Surely there are only a few places where we'd need to > short-circuit the code. Like, if we don't try to fetch Wallet items when the > dialog is opened, default to Autofill being selected, and don't draw the "sign > in to wallet" link, does that fully disable Wallet? I guess I don't know the exact answer to that question until I write the patch. But actually, I take the "ideally" statement back. We don't provide an easy mechanism for removing Google services from Chromium. For example, there's no build-time flag to remove (the option to use) server spelling suggestions.
On 2014/05/15 00:31:27, Ilya Sherman wrote: > Have we actually heard from Linux distros as to whether they want to have Wallet > enabled vs. disabled? > and to answer this question: we never asked. But the policy outlined by Pawel and Joi seems to be "assume Chromium distros want the Google service".
On 2014/05/15 00:40:05, Evan Stade wrote: > On 2014/05/15 00:31:27, Ilya Sherman wrote: > > Have we actually heard from Linux distros as to whether they want to have > Wallet > > enabled vs. disabled? > > > > and to answer this question: we never asked. But the policy outlined by Pawel > and Joi seems to be "assume Chromium distros want the Google service". In that case, why default them to hit sandbox? Do we need this CL at all?
On 2014/05/15 22:06:37, Ilya Sherman wrote: > On 2014/05/15 00:40:05, Evan Stade wrote: > > On 2014/05/15 00:31:27, Ilya Sherman wrote: > > > Have we actually heard from Linux distros as to whether they want to have > > Wallet > > > enabled vs. disabled? > > > > > > > and to answer this question: we never asked. But the policy outlined by Pawel > > and Joi seems to be "assume Chromium distros want the Google service". > > In that case, why default them to hit sandbox? Do we need this CL at all? We want to default unofficial builds (developer builds) to sandbox. The latest version of this cl should default official chromium builds to prod.
On 2014/05/15 23:01:13, Evan Stade wrote: > On 2014/05/15 22:06:37, Ilya Sherman wrote: > > On 2014/05/15 00:40:05, Evan Stade wrote: > > > On 2014/05/15 00:31:27, Ilya Sherman wrote: > > > > Have we actually heard from Linux distros as to whether they want to have > > > Wallet > > > > enabled vs. disabled? > > > > > > > > > > and to answer this question: we never asked. But the policy outlined by > Pawel > > > and Joi seems to be "assume Chromium distros want the Google service". > > > > In that case, why default them to hit sandbox? Do we need this CL at all? > > We want to default unofficial builds (developer builds) to sandbox. The latest > version of this cl should default official chromium builds to prod. What is the motivation for defaulting developer builds to sandbox? The linked bug talks about Linux distros, but it sounds like you think Linux distros (which ship "unofficial" builds) want production servers.
On 2014/05/15 23:50:09, Ilya Sherman wrote:
> On 2014/05/15 23:01:13, Evan Stade wrote:
> > On 2014/05/15 22:06:37, Ilya Sherman wrote:
> > > On 2014/05/15 00:40:05, Evan Stade wrote:
> > > > On 2014/05/15 00:31:27, Ilya Sherman wrote:
> > > > > Have we actually heard from Linux distros as to whether they want to
> have
> > > > Wallet
> > > > > enabled vs. disabled?
> > > > >
> > > >
> > > > and to answer this question: we never asked. But the policy outlined by
> > Pawel
> > > > and Joi seems to be "assume Chromium distros want the Google service".
> > >
> > > In that case, why default them to hit sandbox? Do we need this CL at all?
> >
> > We want to default unofficial builds (developer builds) to sandbox. The
latest
> > version of this cl should default official chromium builds to prod.
>
> What is the motivation for defaulting developer builds to sandbox?
Wallet team requested it.
> The linked
> bug talks about Linux distros, but it sounds like you think Linux distros
(which
> ship "unofficial" builds) want production servers.
Do Linux distros really ship buildtype=Dev builds? That seems wrong, they should
probably fix it. Here's the common.gypi docs:
# Override buildtype to select the desired build flavor.
# Dev - everyday build for development/testing
# Official - release build (generally implies additional processing)
# TODO(mmoss) Once 'buildtype' is fully supported (e.g. Windows gyp
# conversion is done), some of the things which are now controlled by
# 'branding', such as symbol generation, will need to be refactored
# based on 'buildtype' (i.e. we don't care about saving symbols for
# non-Official # builds).
'buildtype%': 'Dev',
LGTM. I guess if anyone complains we can re-evaluate if need be.
The CQ bit was checked by estade@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/estade@chromium.org/288003004/20001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
The CQ bit was checked by estade@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/estade@chromium.org/288003004/20001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
Message was sent while issue was closed.
Change committed as 272348 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
