Fix user-to-system-level installation delegation. This looks to have been bro...

Fix user-to-system-level installation delegation. This looks to have been broken since r66096.

BUG=72620
TEST=Install user level Chrome then system level Chrome. Observer that user level Chrome removes itself and delegates to system level.



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

[tommi (sloooow) - chröme, 2011-02-10 20:52:37]

http://codereview.chromium.org/6490008/diff/1/chrome/installer/util/install_u...
File chrome/installer/util/install_util.cc (right):

http://codereview.chromium.org/6490008/diff/1/chrome/installer/util/install_u...
chrome/installer/util/install_util.cc:75: ProductState state;
With this, now Chrome depends on the ProductState and I suspect some other parts
of the installer it didn't previously depend on.

Could we move this function out of the installer altogether?

It looks like GoogleChromeDistribution::InactiveUserToastExperiment is the only
thing in the installer that uses this function.
We could take that function out of the browserdistribution class and move it
into the installer and use ProductState there.  That way we also move the
experiment code out of Chrome.

[tommi (sloooow) - chröme, 2011-02-10 21:03:00]

lgtm as is as discussed

On Thu, Feb 10, 2011 at 3:52 PM, <tommi@chromium.org> wrote:

>
>
>
http://codereview.chromium.org/6490008/diff/1/chrome/installer/util/install_u...
> File chrome/installer/util/install_util.cc (right):
>
>
>
http://codereview.chromium.org/6490008/diff/1/chrome/installer/util/install_u...
> chrome/installer/util/install_util.cc:75: ProductState state;
> With this, now Chrome depends on the ProductState and I suspect some
> other parts of the installer it didn't previously depend on.
>
> Could we move this function out of the installer altogether?
>
> It looks like GoogleChromeDistribution::InactiveUserToastExperiment is
> the only thing in the installer that uses this function.
> We could take that function out of the browserdistribution class and
> move it into the installer and use ProductState there.  That way we also
> move the experiment code out of Chrome.
>
>
> http://codereview.chromium.org/6490008/
>

[robertshield, 2011-02-10 21:09:39]

http://codereview.chromium.org/6490008/diff/1/chrome/installer/util/install_u...
File chrome/installer/util/install_util.cc (right):

http://codereview.chromium.org/6490008/diff/1/chrome/installer/util/install_u...
chrome/installer/util/install_util.cc:75: ProductState state;
On 2011/02/10 20:52:37, tommi wrote:
> With this, now Chrome depends on the ProductState and I suspect some other
parts
> of the installer it didn't previously depend on.
> 
> Could we move this function out of the installer altogether?

We could, but I'm not sure that's the best way to go just yet. The new
machine-state inspection code in Chrome has the advantage of being tested which
the code in Chrome isn't. I'd like to stick with using the one canonical way of
inspecting machine state (i.e. the InstallationState/ProductState code).

> 
> It looks like GoogleChromeDistribution::InactiveUserToastExperiment is the
only
> thing in the installer that uses this function.
> We could take that function out of the browserdistribution class and move it
> into the installer and use ProductState there.  That way we also move the
> experiment code out of Chrome.