More cleanup of DefaultApps code

More cleanup of DefaultApps code. This should also fix some issues when apps are installed in webstore-non-supported locales.

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

[Erik does not do reviews, 2011-01-03 22:01:30]

LGTM - nice cleanup

http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
File chrome/browser/extensions/default_apps_unittest.cc (right):

http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
chrome/browser/extensions/default_apps_unittest.cc:28: // The launcher should
not be shown until the default apps have been installed.
nit: 80 cols

http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
chrome/browser/extensions/default_apps_unittest.cc:165: // This should stop the
default apps from trying to be installed. The launcher
Doesn't this put us in a weird state?  They could have some default apps
installed, but no explanation as to where they came from.  Do we need to
uninstall any default apps in this situation?

[Aaron Boodman, 2011-01-04 20:59:35]

http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
File chrome/browser/extensions/default_apps_unittest.cc (right):

http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
chrome/browser/extensions/default_apps_unittest.cc:165: // This should stop the
default apps from trying to be installed. The launcher
On 2011/01/03 22:01:30, Erik Kay wrote:
> Doesn't this put us in a weird state?  They could have some default apps
> installed, but no explanation as to where they came from.  Do we need to
> uninstall any default apps in this situation?

It does.

This code was mainly intended to handle the case where you already have apps
installed before the default app code runs, but it also affects the case you
describe.

The problem with trying to gracefully handle this situation is that we don't
really know if an app was installed because it was a default app or if it was
installed by the user. So the user could conceivably install one of the default
apps manually, then the default app installation runs, and we end up
uninstallling something the user installed.

In practice, the bad situation seems unlikely to occur since it would be weird
for one default app to install but another not to.

Adding more code to work around these edge cases seemed scary, and a little
wasteful since we're going to be reworking the location code, and would
hopefully be able to use that in the future to better track where apps came
from.

What do you think? Can you think of some half measure I could do now to better
handle this issue that would align with future work?

[Erik does not do reviews, 2011-01-04 21:19:40]

On 2011/01/04 20:59:35, Aaron Boodman wrote:
>
http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
> File chrome/browser/extensions/default_apps_unittest.cc (right):
> 
>
http://codereview.chromium.org/6040005/diff/2001/chrome/browser/extensions/de...
> chrome/browser/extensions/default_apps_unittest.cc:165: // This should stop
the
> default apps from trying to be installed. The launcher
> On 2011/01/03 22:01:30, Erik Kay wrote:
> > Doesn't this put us in a weird state?  They could have some default apps
> > installed, but no explanation as to where they came from.  Do we need to
> > uninstall any default apps in this situation?
> 
> It does.
> 
> This code was mainly intended to handle the case where you already have apps
> installed before the default app code runs, but it also affects the case you
> describe.
> 
> The problem with trying to gracefully handle this situation is that we don't
> really know if an app was installed because it was a default app or if it was
> installed by the user. So the user could conceivably install one of the
default
> apps manually, then the default app installation runs, and we end up
> uninstallling something the user installed.
> 
> In practice, the bad situation seems unlikely to occur since it would be weird
> for one default app to install but another not to.
> 
> Adding more code to work around these edge cases seemed scary, and a little
> wasteful since we're going to be reworking the location code, and would
> hopefully be able to use that in the future to better track where apps came
> from.
> 
> What do you think? Can you think of some half measure I could do now to better
> handle this issue that would align with future work?

Yes.  We have another reason we need to know if an app was installed by default
or by user action (I'll drop you an email about it).  It would be great if we
could store something in preferences that indicated this so that we could answer
the question.  Agree that it sucks that this is more mucking about with location
code.

[Aaron Boodman, 2011-01-04 23:41:27]

Ok, I think I will do that as a separate change.