[DRAFT] Allow a user to revert to their previous theme without closing infobar

Chrome side work for allowing a user to revert to their previous theme without closing infobar

BUG=257724
TEST=ThemeServiceTest.*
R=yoz
TBR=ben (for ProfileResetterTest and InstantExtendedTest)

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215547

[pkotwicz, 2013-07-24 01:30:42]

yoz@, I was hoping to get your feedback.

The problem I am trying to solve:
- Right now the web store does not allow a user to switch back to their previous
theme if the user does not close the "Installed Theme" infobar.
- When the infobar is visible, the old theme is not deleted when a new one is
installed. (The infobars leak memory which makes the issue worse.
(https://code.google.com/p/chromium/issues/detail?id=62154)
- The web store polls for which themes are installed and displays the UI based
on that. (Because multiple themes can be installed, the web store ends up
displaying incorrect information)

My hope with this CL was to disable themes which are not in use.
This CL is close to complete but it interferes with externally updated
extensions (They start out disabled by default) so I would need to add a new 
DisableReason. This makes things more complicated so I thought I would ask for
advice before continuing further.

The other option would be to create a new private API that the web store can use
to determine which theme is installed.
The web store will probably end up triggering a new download of the theme (even
though we already have it but that should be ok).

[Yoyo Zhou, 2013-07-24 20:45:03]

On 2013/07/24 01:30:42, pkotwicz wrote:
> yoz@, I was hoping to get your feedback.
> 
> The problem I am trying to solve:
> - Right now the web store does not allow a user to switch back to their
previous
> theme if the user does not close the "Installed Theme" infobar.

Is there an extra or missing negation in this sentence?

[Yoyo Zhou, 2013-07-24 20:55:56]

On 2013/07/24 20:45:03, Yoyo Zhou wrote:
> On 2013/07/24 01:30:42, pkotwicz wrote:
> > yoz@, I was hoping to get your feedback.
> > 
> > The problem I am trying to solve:
> > - Right now the web store does not allow a user to switch back to their
> previous
> > theme if the user does not close the "Installed Theme" infobar.
> 
> Is there an extra or missing negation in this sentence?

Oh, sorry, I understand. You can only switch back via the infobar, not the web
store.

[Yoyo Zhou, 2013-07-24 21:55:03]

Can you explain what you mean by "interferes with externally updated
extensions"? At a glance, I don't see why that should happen.

Following the bug: yes, it looks like CWS doesn't currently know if extensions
are enabled or disabled, so it would have to be updated to keep track of that.

https://chromiumcodereview.appspot.com/19462009/diff/2001/chrome/browser/them...
File chrome/browser/themes/theme_service.cc (right):

https://chromiumcodereview.appspot.com/19462009/diff/2001/chrome/browser/them...
chrome/browser/themes/theme_service.cc:58: // Wait this many seconds after
startup to garbage collect unused themes.
Why wait?

https://chromiumcodereview.appspot.com/19462009/diff/2001/chrome/browser/them...
chrome/browser/themes/theme_service.cc:273: // The theme may be initially
disabled. Wait till it is loaded (if ever)
I don't see why we need to listen for extension installs or enables
specifically. See comment below.

https://chromiumcodereview.appspot.com/19462009/diff/2001/chrome/browser/them...
chrome/browser/themes/theme_service.cc:284: installed_pending_load_id_ ==
extension->id()) {
What's the scenario where we load a theme and don't expect to set it? (When we
enable an extension, it also gets loaded.) Whenever this would happen, the theme
would be enabled and yet not active, which seems to be what you're trying to
fix.

[pkotwicz, 2013-07-25 01:25:29]

Thanks for the review.

I will get back to this CL over the weekend. (I am working on fixing this in my
20% time and I think I still have a fair bit to learn about the extension
service before I can create a CL I am confident of)

https://codereview.chromium.org/19462009/diff/2001/chrome/browser/themes/them...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/2001/chrome/browser/themes/them...
chrome/browser/themes/theme_service.cc:273: // The theme may be initially
disabled. Wait till it is loaded (if ever)
You are right. I should be able to move registering for the
NOTIFICATION_EXTENSION_LOADED notification and I can avoid listening to
NOTIFICATION_EXTENSION_INSTALLED and NOTIFICATION_EXTENSION_ENABLED as a result.

As background the ThemeService is created before the ExtensionService is inited
but after ExtensionSystem is created.
It can also be used before the ExtensionService has finished initing which makes
things ugly.

https://codereview.chromium.org/19462009/diff/2001/chrome/browser/themes/them...
File chrome/browser/themes/theme_syncable_service.cc (left):

https://codereview.chromium.org/19462009/diff/2001/chrome/browser/themes/them...
chrome/browser/themes/theme_syncable_service.cc:221: if
(!extensions_service->IsExtensionEnabled(id)) {
I was mostly worried about interference with externally updated extensions here
because ThemeService::SetTheme() will enable the extension if it is not enabled.
I am pretty sure it is not a concern because the theme will only be synced if
the theme extension is enabled on another device.

[pkotwicz, 2013-07-29 06:06:50]

yoz@ can you please take another look?

I decided to keep on listening for NOTIFICATION_EXTENSION_INSTALLED and
NOTIFICATION_EXTENSION_ENABLED. The reason for my decision is that we do a poor
job in RemoveUnusedThemes().
We ignore blacklisted themes (I am unsure if there is such a thing) and we
ignore themes which are disabled via management policy. (In both of these cases
the interesting scenario is if an extension is unblacklisted).
Currently, ExtensionService::CheckManagementPolicy() only unloads extensions
which are blacklisted via prefs::kExtensionInstallDenyList and does not reload
extensions which are no longer blacklisted. I do not want to risk bugs if this
ever changes.

I now only uninstall external themes with disable reason
Extension::DISABLE_USER_ACTION. This fixes the issues wrt initially disabled
external extensions.

[pkotwicz, 2013-07-29 06:12:35]

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:61: const int
kRemoveUnusedThemesStartupDelay = 30;
I wait here because we garbage collected extensions on a timeout.
ExtensionService::GarbageCollectExtensions.

[Yoyo Zhou, 2013-07-30 00:28:55]

On 2013/07/29 06:06:50, pkotwicz wrote:
> yoz@ can you please take another look?
> 
> I decided to keep on listening for NOTIFICATION_EXTENSION_INSTALLED and
> NOTIFICATION_EXTENSION_ENABLED. The reason for my decision is that we do a
poor
> job in RemoveUnusedThemes().
> We ignore blacklisted themes (I am unsure if there is such a thing) and we
> ignore themes which are disabled via management policy. (In both of these
cases
> the interesting scenario is if an extension is unblacklisted).

Can we do a better job in RemoveUnusedThemes then, and avoid unblacklisting
themes? If a theme isn't enabled, and it doesn't have an infobar to reenable it,
it shouldn't be installed. See my comments on going over
ExtensionService::blacklisted_extensions().

> Currently, ExtensionService::CheckManagementPolicy() only unloads extensions
> which are blacklisted via prefs::kExtensionInstallDenyList and does not reload
> extensions which are no longer blacklisted. I do not want to risk bugs if this
> ever changes.

This sounds like a bug. cc'ing kalman to see if he knows about this.

[Yoyo Zhou, 2013-07-30 00:29:18]

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:61: const int
kRemoveUnusedThemesStartupDelay = 30;
On 2013/07/29 06:12:35, pkotwicz wrote:
> I wait here because we garbage collected extensions on a timeout.
> ExtensionService::GarbageCollectExtensions.

Ok, please add a comment even if it's just "see ExtensionService::...".

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:229: if (extension->is_theme() &&
I don't see which case we need to worry about extensions that are loaded but yet
were neither installed nor enabled.

I'm afraid there are going to be subtle bugs here. What if we have an external
theme, we install a theme from the webstore, and then try to accept the external
theme?

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:320: // Uninstall themes with disable
reason DISABLE_USER_ACTION. We cannot blanket
Suppose you open Chrome and install a new theme, and you have an infobar for the
old one. Then 30s after startup this runs. What happens?

Ths might be an existing bug - it appears the old theme is gone, and it reverts
back to the default theme instead.

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:337: // blacklisted by a management
policy provider.
Seems like we can go over service->blacklisted_extensions() too. We shouldn't
end up with blacklisted themes that aren't being used.

[not at google - send to devlin, 2013-07-30 01:57:12]

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:337: // blacklisted by a management
policy provider.
On 2013/07/30 00:29:18, Yoyo Zhou wrote:
> Seems like we can go over service->blacklisted_extensions() too. We shouldn't
> end up with blacklisted themes that aren't being used.

there are two types of blacklists, management policy blacklist and the more
common google blacklist.

service->blacklist() is the latter.

I don't understand any of this stuff but it seems very complicated. Inspecting
the state of extensions like this is bound to lead to subtle bugs, can you look
at the installed extension set instead or something?

[pkotwicz, 2013-07-31 21:42:52]

yoz@, can you please take another look?

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:61: const int
kRemoveUnusedThemesStartupDelay = 30;
On 2013/07/30 00:29:18, Yoyo Zhou wrote:
> On 2013/07/29 06:12:35, pkotwicz wrote:
> > I wait here because we garbage collected extensions on a timeout.
> > ExtensionService::GarbageCollectExtensions.
> 
> Ok, please add a comment even if it's just "see ExtensionService::...".

Done.

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:229: if (extension->is_theme() &&
- On ToT/stable it is not possible to enable an externally loaded theme.
- With this CL, the more complex scenario you described works.

The fact that it was never possible to install an externally loaded theme may
indicate that we should just disable externally loaded themes. (That would
probably be a different CL), but it would slightly simplify
ThemeService::RemoveUnusedThemes() in this CL.

The scenario I am concerned about: (Scenario B)
- A user installs a theme and exits chrome.
- The policy is changed to disable the theme the user has active. (Via setting
ExtensionInstallBlacklist)

User starts chrome, the now policy blacklisted theme is not loaded.
(ManagementPolicy::UserMayLoad returns false). The theme now sticks around for
ever. My concern is that if the theme is ever unblacklisted that it will be set.

Ideally, ThemeService::RemoveUnusedThemes() would clean those themes up but
making that change requires a lot more in depth knowledge of extensions than I
have.
Note that Scenario B is broken because despite the extension not being loaded,
the ThemeService still uses the management policy blacklisted theme. (This is a
bug with and without this CL)

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:320: // Uninstall themes with disable
reason DISABLE_USER_ACTION. We cannot blanket
This CL fixes that issue too. RemoveUnusedThemes() checks |number_of_infobars_|.

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:337: // blacklisted by a management
policy provider.
Ideally, we would clean up themes in both blacklists (but that's outside of the
scope of this CL)

I now use ExtensionService::GenerateInstalledExtensionSet() which cuts down on
the number of loops.
kalman@, thanks for the suggestion!

[Yoyo Zhou, 2013-07-31 22:13:00]

Ok, LGTM.

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:229: if (extension->is_theme() &&
On 2013/07/31 21:42:52, pkotwicz wrote:
> - On ToT/stable it is not possible to enable an externally loaded theme.
> - With this CL, the more complex scenario you described works.
> 
> The fact that it was never possible to install an externally loaded theme may
> indicate that we should just disable externally loaded themes. (That would
> probably be a different CL), but it would slightly simplify
> ThemeService::RemoveUnusedThemes() in this CL.
> 
> The scenario I am concerned about: (Scenario B)
> - A user installs a theme and exits chrome.
> - The policy is changed to disable the theme the user has active. (Via setting
> ExtensionInstallBlacklist)
> 
> User starts chrome, the now policy blacklisted theme is not loaded.
> (ManagementPolicy::UserMayLoad returns false). The theme now sticks around for
> ever. My concern is that if the theme is ever unblacklisted that it will be
set.

Well, if we restart Chrome, we don't seem to call SetTheme on the existing
theme. (Besides, we don't listen to EXTENSION_LOADED here until after
EXTENSIONS_READY, which is after the initial load of extensions.) So it will
only be set if it goes through EnableExtension (I'm not sure if that happens -
kalman?). There may be other subtle bugs, though.

> Ideally, ThemeService::RemoveUnusedThemes() would clean those themes up but
> making that change requires a lot more in depth knowledge of extensions than I
> have.
> Note that Scenario B is broken because despite the extension not being loaded,
> the ThemeService still uses the management policy blacklisted theme. (This is
a
> bug with and without this CL)

https://codereview.chromium.org/19462009/diff/30001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/30001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:325: // themes because externally
installed extensions are initially disabled.
If external themes never worked, I'm ok with not supporting them.

[not at google - send to devlin, 2013-07-31 22:19:13]

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:229: if (extension->is_theme() &&
On 2013/07/31 22:13:00, Yoyo Zhou wrote:
> On 2013/07/31 21:42:52, pkotwicz wrote:
> > - On ToT/stable it is not possible to enable an externally loaded theme.
> > - With this CL, the more complex scenario you described works.
> > 
> > The fact that it was never possible to install an externally loaded theme
may
> > indicate that we should just disable externally loaded themes. (That would
> > probably be a different CL), but it would slightly simplify
> > ThemeService::RemoveUnusedThemes() in this CL.
> > 
> > The scenario I am concerned about: (Scenario B)
> > - A user installs a theme and exits chrome.
> > - The policy is changed to disable the theme the user has active. (Via
setting
> > ExtensionInstallBlacklist)
> > 
> > User starts chrome, the now policy blacklisted theme is not loaded.
> > (ManagementPolicy::UserMayLoad returns false). The theme now sticks around
for
> > ever. My concern is that if the theme is ever unblacklisted that it will be
> set.
> 
> Well, if we restart Chrome, we don't seem to call SetTheme on the existing
> theme. (Besides, we don't listen to EXTENSION_LOADED here until after
> EXTENSIONS_READY, which is after the initial load of extensions.) So it will
> only be set if it goes through EnableExtension (I'm not sure if that happens -
> kalman?). There may be other subtle bugs, though.
> 
> > Ideally, ThemeService::RemoveUnusedThemes() would clean those themes up but
> > making that change requires a lot more in depth knowledge of extensions than
I
> > have.
> > Note that Scenario B is broken because despite the extension not being
loaded,
> > the ThemeService still uses the management policy blacklisted theme. (This
is
> a
> > bug with and without this CL)

No idea off the top of my head, jyasskin is probably the most familiar with
extension loading stuff.

[pkotwicz, 2013-07-31 22:37:22]

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
File chrome/browser/themes/theme_service.cc (right):

https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
chrome/browser/themes/theme_service.cc:229: if (extension->is_theme() &&
- I have an idea as to how to fix the bug wrt to the theme being loaded if the
managemenet policy blacklisted the theme. (I will do that in a separate CL
though)
- yoz@, I am not too sure if I understand your most recent comment. I have not
found a way that a theme which is unblacklisted by the management policy can get
loaded after the EXTENSIONS_READY notification is sent. (I am concerned that a
future CL may change this)

[Yoyo Zhou, 2013-07-31 22:41:57]

On 2013/07/31 22:37:22, pkotwicz wrote:
>
https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
> File chrome/browser/themes/theme_service.cc (right):
> 
>
https://codereview.chromium.org/19462009/diff/11001/chrome/browser/themes/the...
> chrome/browser/themes/theme_service.cc:229: if (extension->is_theme() &&
> - I have an idea as to how to fix the bug wrt to the theme being loaded if the
> managemenet policy blacklisted the theme. (I will do that in a separate CL
> though)
> - yoz@, I am not too sure if I understand your most recent comment. I have not
> found a way that a theme which is unblacklisted by the management policy can
get
> loaded after the EXTENSIONS_READY notification is sent. (I am concerned that a
> future CL may change this)

Ah, I was saying that I don't see how such a theme could be loaded now, but
you're right that it could break.

[commit-bot: I haz the power, 2013-08-03 19:44:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19462009/41001

[commit-bot: I haz the power, 2013-08-03 20:39:02]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-08-04 15:06:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19462009/89001

[commit-bot: I haz the power, 2013-08-04 15:18:02]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-08-04 15:59:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19462009/89001

[commit-bot: I haz the power, 2013-08-04 16:10:19]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[commit-bot: I haz the power, 2013-08-04 17:23:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19462009/89001

[commit-bot: I haz the power, 2013-08-04 18:53:33]

Retried try job too often on mac_rel for step(s) remoting_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-08-04 20:27:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/19462009/129001

[commit-bot: I haz the power, 2013-08-05 01:18:42]

Change committed as 215547

[Jeffrey Yasskin, 2013-08-06 21:46:01]

I suspect there's a problem with test isolation here, but I'm not sure what
exactly it is. Any ideas would be welcome:

https://chromiumcodereview.appspot.com/19462009/diff/129001/chrome/browser/th...
File chrome/browser/themes/theme_service.cc (right):

https://chromiumcodereview.appspot.com/19462009/diff/129001/chrome/browser/th...
chrome/browser/themes/theme_service.cc:396:
base::MessageLoop::current()->PostTask(FROM_HERE,
In local test runs, I'm getting stack traces like:

#0  scoped_refptr<base::internal::IncomingTaskQueue>::operator-> (this=0x150)
    at ../../base/memory/ref_counted.h:256
#1  0x00007ffff5438caf in base::MessageLoop::PostTask(tracked_objects::Location
const&, base::Callback<void ()> const&) (this=0x0, from_here=..., task=...)
    at ../../base/message_loop/message_loop.cc:268
#2  0x0000000002ec6220 in ThemeService::ClearAllThemeData (this=0x27cd8feae020)
    at ../../chrome/browser/themes/theme_service.cc:395
#3  0x0000000003a57523 in GtkThemeService::ClearAllThemeData
(this=0x27cd8feae020)
    at ../../chrome/browser/ui/gtk/gtk_theme_service.cc:637
#4  0x0000000003a55bac in GtkThemeService::SetNativeTheme (this=0x27cd8feae020)
    at ../../chrome/browser/ui/gtk/gtk_theme_service.cc:364
#5  0x0000000002ec62f2 in ThemeService::LoadThemePrefs (this=0x27cd8feae020)
    at ../../chrome/browser/themes/theme_service.cc:408
#6  0x0000000003a57552 in GtkThemeService::LoadThemePrefs (this=0x27cd8feae020)
    at ../../chrome/browser/ui/gtk/gtk_theme_service.cc:647
#7  0x0000000002ec4134 in ThemeService::Init (this=0x27cd8feae020,
profile=0x27cd8fc51630)
    at ../../chrome/browser/themes/theme_service.cc:100
#8  0x0000000003a55024 in GtkThemeService::Init (this=0x27cd8feae020,
profile=0x27cd8fc51630)
    at ../../chrome/browser/ui/gtk/gtk_theme_service.cc:300
#9  0x0000000002ec84db in ThemeServiceFactory::BuildServiceInstanceFor
(this=0x27cd8ff7f260, 
    profile=0x27cd8fc51630) at
../../chrome/browser/themes/theme_service_factory.cc:64
#10 0x00007ffff7f4fb7a in
BrowserContextKeyedServiceFactory::GetServiceForBrowserContext (
    this=0x27cd8ff7f260, context=0x27cd8fc51630, create=true)
    at
../../components/browser_context_keyed_service/browser_context_keyed_service_factory.cc:84
#11 0x00007ffff7f50072 in BrowserContextKeyedServiceFactory::CreateServiceNow (
    this=0x27cd8ff7f260, context=0x27cd8fc51630)
    at
../../components/browser_context_keyed_service/browser_context_keyed_service_factory.cc:129
#12 0x00007ffff7f4c033 in
BrowserContextDependencyManager::CreateBrowserContextServices (
    this=0x27cd8ff6e0f0, context=0x27cd8fc51630, is_testing_context=true)
    at
../../components/browser_context_keyed_service/browser_context_dependency_manager.cc:77
#13 0x0000000003757eba in TestingProfile::Init (this=0x27cd8fc51630)
    at ../../chrome/test/base/testing_profile.cc:312
#14 0x0000000003757989 in TestingProfile::TestingProfile (this=0x27cd8fc51630)
    at ../../chrome/test/base/testing_profile.cc:179
#15 0x0000000000512edf in
apps::ShellWindowGeometryCacheTest::ShellWindowGeometryCacheTest (
    this=0x27cd8fc51620) at
../../apps/shell_window_geometry_cache_unittest.cc:31
#16 0x000000000051489f in
apps::ShellWindowGeometryCacheTest_GetGeometryEmptyStore_Test::ShellWindowGeometryCacheTest_GetGeometryEmptyStore_Test
(this=0x27cd8fc51620)
    at ../../apps/shell_window_geometry_cache_unittest.cc:99
#17 0x0000000000514875 in
apps::ShellWindowGeometryCacheTest_GetGeometryEmptyStore_Test::ShellWindowGeometryCacheTest_GetGeometryEmptyStore_Test
(this=0x27cd8fc51620)
    at ../../apps/shell_window_geometry_cache_unittest.cc:99


Which look to be caused by a TestingProfile getting created before the
MessageLoop. It seems like my execution is taking the !managed &&
ShouldInitWithNativeTheme path in ThemeService::LoadThemePrefs, which the bots
and kalman's local build aren't taking.

[Jeffrey Yasskin, 2013-08-06 23:00:25]

On 2013/08/06 21:46:01, Jeffrey Yasskin wrote:
> I suspect there's a problem with test isolation here, but I'm not sure what
> exactly it is. Any ideas would be welcome:

I filed http://crbug.com/269155.