Make ThemeService not disable uninstalled themes

Make ThemeService not disable uninstalled themes

If an extension is disabled after it is uninstalled, the extension is not enabled
when it is reinstalled. ThemeService was disabling theme extensions after they
were uninstalled if the extension was uninstalled as a result of the
NOTIFICATION_BROWSER_THEME_CHANGED notification. This bug was causing installing
a theme via the Chrome Web Store to sometimes not work.

BUG=468280
TEST=ThemeServiceTest.UninstallThemeOnThemeChangeNotification

Committed: https://crrev.com/315d1a31021a6d9e9b75ab206a0dd4df389e90f1
Cr-Commit-Position: refs/heads/master@{#348780}

[pkotwicz, 2015-09-10 22:41:33]

pkotwicz@chromium.org changed reviewers:
+ asargent@chromium.org

[pkotwicz, 2015-09-10 22:41:33]

asargent@ can you please take a look?

I did some digging and the current behavior w.r.t to disabling uninstalled
extensions was introduced in https://codereview.chromium.org/6903127

[asargent_no_longer_on_chrome, 2015-09-11 19:47:11]

I'll need to dig into the theme code a little more to know for sure, but my
first instinct is that this doesn't seem quite right. I'd argue we want to aim
for the following property:

-Only 1 theme may be in the 'enabled' state at a time, and that is the currently
active theme

As for when disabled themes get uninstalled, I think the current logic mostly
aims to have an undo buffer of length 1, and uninstall get triggered once the
infobar with the "undo" button has been dismissed. 

A long time ago I did some work to try and make the system work this way for
crbug.com/69963, and I still have an old unsubmitted patch here :
https://codereview.chromium.org/6546072/. I can't quite remember why I never put
it up for review. But I think the TEST line there articulates how we'd want
things to work.

[pkotwicz, 2015-09-11 20:17:23]

The situation is better than what you describe. With the code the way it is now,
there should be just one theme in the enabled state at a time. (With the
exception of very very short periods of time)

When a new theme is installed we do the following:
Step 1 - Install the new theme
Step 2 - Send the NOTIFICATION_BROWSER_THEME_CHANGED notification
Step 3 - Disable the previous theme

The bug is that we sometimes disable the theme AFTER uninstalling it (disabling
the theme before uninstalling it is ok and is expected). Disabling a theme after
uninstalling it is bad, because it prevents the user from ever reinstalling the
theme which was uninstalled. (ThemeService::OnExtensionLoaded() is not called if
a disabled extension is installed) Without this CL, we disable the theme after
uninstalling it if a NOTIFICATION_BROWSER_THEME_CHANGED notification observer
uninstalls the theme.

[asargent_no_longer_on_chrome, 2015-09-14 22:32:02]

Sorry for the delay - I looked at more of the code in ExtensionService,
ExtensionPrefs, etc. and I agree this is a good change. 

lgtm

[pkotwicz, 2015-09-14 22:33:25]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-09-14 22:36:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319073005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319073005/1

[commit-bot: I haz the power, 2015-09-15 00:05:58]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-09-15 00:07:01]

Patchset 1 (id:??) landed as
https://crrev.com/315d1a31021a6d9e9b75ab206a0dd4df389e90f1
Cr-Commit-Position: refs/heads/master@{#348780}

[commit-bot: I haz the power, 2015-09-23 12:39:47]

Patchset 1 (id:??) landed as
https://crrev.com/315d1a31021a6d9e9b75ab206a0dd4df389e90f1
Cr-Commit-Position: refs/heads/master@{#348780}