Do not show the EULA if it has already been accepted by the user in another user data dir.

Do not show the EULA if it has already been accepted by the user in another user data dir.

BUG=131033
TEST=install system-level chrome with require_eula in master prefs and eulaaccepted=0 in the ClientState key. nuke all user data dirs (including the metro one). now launch desktop chrome and accept the eula.  make chrome the default browser and launch into metro.  notice that it works.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142576

[grt (UTC plus 2), 2012-06-14 02:09:52]

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
File chrome/browser/first_run/first_run_win.cc (right):

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
chrome/browser/first_run/first_run_win.cc:219: if
(install_prefs->GetBool(installer::master_preferences::kRequireEula,
i'm tempted to get rid of this check for require_eula.  on the other hand, i
want to avoid a regression.  whaddya think, robert?

[robertshield, 2012-06-14 02:50:06]

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
File chrome/browser/first_run/first_run_win.cc (right):

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
chrome/browser/first_run/first_run_win.cc:183: BrowserDistribution* dist = 
BrowserDistribution::GetDistribution();
nit: double spaces

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
chrome/browser/first_run/first_run_win.cc:219: if
(install_prefs->GetBool(installer::master_preferences::kRequireEula,
On 2012/06/14 02:09:52, grt wrote:
> i'm tempted to get rid of this check for require_eula.  on the other hand, i
> want to avoid a regression.  whaddya think, robert?

If require_eula is not present and the eulaaccepted is 0 then Chrome will run
but never update, right? If that's correct we should drop the check since any
case where Chrome never updates is double plus bad.

No likely regressions are coming to mind, though that means little :s

[grt (UTC plus 2), 2012-06-14 03:39:00]

Thanks, PTAL.

Glenn and Somas: this change will make Chrome's first-run flow trigger the EULA
solely on the basis of Google Update's eulaaccepted state in the registry rather
than on require_eula in master preferences.  This seems like a safe change to
Robert and I.  Do you two see any problems with this (is there anyone else who
should be consulted)?  See the related issue http://crbug.com/106554.

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
File chrome/browser/first_run/first_run_win.cc (right):

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
chrome/browser/first_run/first_run_win.cc:183: BrowserDistribution* dist = 
BrowserDistribution::GetDistribution();
On 2012/06/14 02:50:06, robertshield wrote:
> nit: double spaces

Done.

https://chromiumcodereview.appspot.com/10539153/diff/1/chrome/browser/first_r...
chrome/browser/first_run/first_run_win.cc:219: if
(install_prefs->GetBool(installer::master_preferences::kRequireEula,
On 2012/06/14 02:50:06, robertshield wrote:
> On 2012/06/14 02:09:52, grt wrote:
> > i'm tempted to get rid of this check for require_eula.  on the other hand, i
> > want to avoid a regression.  whaddya think, robert?
> 
> If require_eula is not present and the eulaaccepted is 0 then Chrome will run
> but never update, right? If that's correct we should drop the check since any
> case where Chrome never updates is double plus bad.
> 
> No likely regressions are coming to mind, though that means little :s

Done.

[robertshield, 2012-06-14 04:47:45]

lgtm

[commit-bot: I haz the power, 2012-06-14 16:37:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grt@chromium.org/10539153/4002

[commit-bot: I haz the power, 2012-06-14 17:21:34]

Try job failure for 10539153-4002 (retry) on mac_rel for step "runhooks".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-14 17:46:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grt@chromium.org/10539153/4002

[commit-bot: I haz the power, 2012-06-14 19:03:49]

Try job failure for 10539153-4002 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[grt (UTC plus 2), 2012-06-15 21:07:27]

as it turns out, the previous approach was no good since only the first user
would see the EULA.  this new approach will show it once to each user.  PTAL.

[grt (UTC plus 2), 2012-06-16 01:03:04]

Ananta: do you have time to review this?

[ananta, 2012-06-16 01:14:46]

LGTM

http://codereview.chromium.org/10539153/diff/16001/chrome/common/chrome_paths.h
File chrome/common/chrome_paths.h (right):

http://codereview.chromium.org/10539153/diff/16001/chrome/common/chrome_paths...
chrome/common/chrome_paths.h:22: DIR_ALT_USER_DATA,            // Directory of
the desktop or metro user data
Should this name be metroized?

[grt (UTC plus 2), 2012-06-16 02:06:11]

http://codereview.chromium.org/10539153/diff/16001/chrome/common/chrome_paths.h
File chrome/common/chrome_paths.h (right):

http://codereview.chromium.org/10539153/diff/16001/chrome/common/chrome_paths...
chrome/common/chrome_paths.h:22: DIR_ALT_USER_DATA,            // Directory of
the desktop or metro user data
On 2012/06/16 01:14:46, ananta wrote:
> Should this name be metroized?

Could you elaborate?  I don't understand the question.  I did mean to #ifdef
this so it's windows-only, though, which I've just done in the latest patch set.

[commit-bot: I haz the power, 2012-06-16 02:32:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grt@chromium.org/10539153/20002

[commit-bot: I haz the power, 2012-06-16 03:45:25]

Change committed as 142576