Added in switches for the Mac Installer to send preferences to Chrome

Added in switches for the Mac Installer to send preferences to Chrome

BUG=

Committed: https://crrev.com/5b0b74aec747196fbcb404e670829f5fa53ac4f2
Cr-Commit-Position: refs/heads/master@{#415525}

[Anna Zeng, 2016-08-23 16:52:11]

Description was changed from

==========
Added in appropriate switches for the Mac Installer to send preferences to
Chrome

BUG=
==========

to

==========
Added in switches for the Mac Installer to send preferences to Chrome

BUG=
==========

[Anna Zeng, 2016-08-23 16:58:34]

zengster@google.com changed reviewers:
+ cpu@chromium.org, ellyjones@chromium.org, grt@chromium.org,
ivanhernandez@google.com, mark@chromium.org

[Anna Zeng, 2016-08-23 21:29:51]

zengster@google.com changed reviewers:
+ sdy@chromium.org

[Elly Fong-Jones, 2016-08-24 16:01:32]

lgtm

How does one test this? Manually do the install flow?

[Anna Zeng, 2016-08-24 18:01:48]

On 2016/08/24 16:01:32, Elly Jones wrote:
> lgtm
> 
> How does one test this? Manually do the install flow?

Currently this is tested manually by going through the install flow -- and
confirming the flags were registered in Chrome after I built it with these
changes. I'm sure progress with testing will come shortly.

[Anna Zeng, 2016-08-24 18:01:58]

The CQ bit was checked by zengster@google.com

[commit-bot: I haz the power, 2016-08-24 18:03:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 18:03:02]

This CL has an open dependency (Issue 2203583002 Patch 140001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[grt (UTC plus 2), 2016-08-24 22:11:55]

rubberstamp lgtm with nits in chrome/app.

fwiw, we do this sort of thing via installerdata JSON on Windows. is it
possible/reasonable to re-use that strategy rather than making something new? if
no (i can think of reasons to not), should the handling of these command-line
arguments be conditional on first-run? hmm. they wouldn't work for over-installs
in that case.

a middle ground would be to re-use the Windows name. kMakeChromeDefaultForUser
is the constant that means "Hey Chrome! During startup, go ahead and makes
yourself the default browser because that's what the user wants!"

https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
chrome/app/chrome_main_delegate.cc:706: if (uma_setting) {
nit: omit braces for single-line conditionals like this

[Anna Zeng, 2016-08-30 20:29:30]

By the above comment, did you mean:
https://cs.chromium.org/chromium/src/chrome/installer/util/master_preferences...
? It seems that the approach being taken here is that a preferences JSON file is
left by the installer somewhere on the computer for the Chrome.

After a discussion with my hosts, we agreed that the best approach to take is to
still use the command line flags -- and that we will keep the projects separate.

https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
File chrome/app/chrome_main_delegate.cc (right):

https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
chrome/app/chrome_main_delegate.cc:706: if (uma_setting) {
On 2016/08/24 22:11:55, grt (UTC plus 2) wrote:
> nit: omit braces for single-line conditionals like this

Done.

[Anna Zeng, 2016-08-30 20:29:39]

The CQ bit was checked by zengster@google.com

[Anna Zeng, 2016-08-30 20:29:42]

The CQ bit was unchecked by zengster@google.com

[Anna Zeng, 2016-08-30 20:54:50]

On 2016/08/30 20:29:30, Anna Zeng wrote:
> By the above comment, did you mean:
>
https://cs.chromium.org/chromium/src/chrome/installer/util/master_preferences...
> ? It seems that the approach being taken here is that a preferences JSON file
is
> left by the installer somewhere on the computer for the Chrome.
> 
> After a discussion with my hosts, we agreed that the best approach to take is
to
> still use the command line flags -- and that we will keep the projects
separate.
> 
>
https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
> File chrome/app/chrome_main_delegate.cc (right):
> 
>
https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
> chrome/app/chrome_main_delegate.cc:706: if (uma_setting) {
> On 2016/08/24 22:11:55, grt (UTC plus 2) wrote:
> > nit: omit braces for single-line conditionals like this
> 
> Done.

After some discussion, we clarified that the name of the switch is all that
needs to change.

[Anna Zeng, 2016-08-30 20:54:52]

On 2016/08/30 20:29:30, Anna Zeng wrote:
> By the above comment, did you mean:
>
https://cs.chromium.org/chromium/src/chrome/installer/util/master_preferences...
> ? It seems that the approach being taken here is that a preferences JSON file
is
> left by the installer somewhere on the computer for the Chrome.
> 
> After a discussion with my hosts, we agreed that the best approach to take is
to
> still use the command line flags -- and that we will keep the projects
separate.
> 
>
https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
> File chrome/app/chrome_main_delegate.cc (right):
> 
>
https://codereview.chromium.org/2270083002/diff/1/chrome/app/chrome_main_dele...
> chrome/app/chrome_main_delegate.cc:706: if (uma_setting) {
> On 2016/08/24 22:11:55, grt (UTC plus 2) wrote:
> > nit: omit braces for single-line conditionals like this
> 
> Done.

After some discussion, we clarified that the name of the switch is all that
needs to change.

[Anna Zeng, 2016-08-30 21:04:43]

The CQ bit was checked by zengster@google.com

[Anna Zeng, 2016-08-30 21:04:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
ellyjones@chromium.org, grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2270083002/#ps20001
(title: "LGTM edits")

[commit-bot: I haz the power, 2016-08-30 21:05:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 21:08:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 21:08:03]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Anna Zeng, 2016-08-30 23:40:41]

The CQ bit was checked by zengster@google.com

[Anna Zeng, 2016-08-30 23:40:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
ellyjones@chromium.org, grt@chromium.org
Link to the patchset: https://codereview.chromium.org/2270083002/#ps40001
(title: "Rebased")

[commit-bot: I haz the power, 2016-08-30 23:41:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 01:13:55]

Description was changed from

==========
Added in switches for the Mac Installer to send preferences to Chrome

BUG=
==========

to

==========
Added in switches for the Mac Installer to send preferences to Chrome

BUG=
==========

[commit-bot: I haz the power, 2016-08-31 01:13:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-31 01:15:43]

Description was changed from

==========
Added in switches for the Mac Installer to send preferences to Chrome

BUG=
==========

to

==========
Added in switches for the Mac Installer to send preferences to Chrome

BUG=

Committed: https://crrev.com/5b0b74aec747196fbcb404e670829f5fa53ac4f2
Cr-Commit-Position: refs/heads/master@{#415525}
==========

[commit-bot: I haz the power, 2016-08-31 01:15:44]

Patchset 3 (id:??) landed as
https://crrev.com/5b0b74aec747196fbcb404e670829f5fa53ac4f2
Cr-Commit-Position: refs/heads/master@{#415525}