[Extensions] Fix lifetime bug in ExtensionAction/IconImage

[Extensions] Fix lifetime bug in ExtensionAction/IconImage

ExtensionActions can lazily load the default icon image, which is
implemented as an IconImage. The IconImage, in turn, is constructed with
a BrowserContext because it needs access to the ImageLoader (a
BrowserContextKeyedService). ExtensionActions are owned by the
ExtensionActionManager (another BrowserContextKeyedService with one
instance shared across incognito and normal profiles), but took a
BrowserContext as an argument when loading the default icon image.

This all caused a problem when an incognito profile was passed in as
the browser context to load the default icon for the extension action.
The extension action would then create an IconImage with the incognito
profile as a context, but the ExtensionAction (and thus the IconImage)
are owned by the ExtensionActionManager, and are therefore not deleted
upon the incognito profile's destruction. This means that if you have a
flow where the ExtensionAction's default icon is loaded via an incognito
profile, the incognito profile is deleted, and then the icon is later
used in the normal profile, there's a crash.

Fix this by having the ExtensionActionManager assign the IconImage to
the ExtensionAction using its own profile. Since the ExtensionActions
are owned by the ExtensionActionManager, and the ExtensionActionManager
is owned by the profile by being a BrowserContextKeyedService, this is
guaranteed to be safe. This also abstracts out the context of a profile
or BrowserContext from the ExtensionAction class, keeping it more in
line with its data-structure-like concept.

Add a regression test.

BUG=663726
(and possibly others)

Review-Url: https://codereview.chromium.org/2506273002
Cr-Commit-Position: refs/heads/master@{#433361}
(cherry picked from commit d09e1c4d5c955d73cc38f507ede1119f6c540320)

Committed: https://chromium.googlesource.com/chromium/src/+/bf76b5aaca31f715c74bb3274edf748901c1a6d7

[Devlin, 2016-11-30 02:07:45]

Description was changed from

==========
[Extensions] Fix lifetime bug in ExtensionAction/IconImage

ExtensionActions can lazily load the default icon image, which is
implemented as an IconImage. The IconImage, in turn, is constructed with
a BrowserContext because it needs access to the ImageLoader (a
BrowserContextKeyedService). ExtensionActions are owned by the
ExtensionActionManager (another BrowserContextKeyedService with one
instance shared across incognito and normal profiles), but took a
BrowserContext as an argument when loading the default icon image.

This all caused a problem when an incognito profile was passed in as
the browser context to load the default icon for the extension action.
The extension action would then create an IconImage with the incognito
profile as a context, but the ExtensionAction (and thus the IconImage)
are owned by the ExtensionActionManager, and are therefore not deleted
upon the incognito profile's destruction. This means that if you have a
flow where the ExtensionAction's default icon is loaded via an incognito
profile, the incognito profile is deleted, and then the icon is later
used in the normal profile, there's a crash.

Fix this by having the ExtensionActionManager assign the IconImage to
the ExtensionAction using its own profile. Since the ExtensionActions
are owned by the ExtensionActionManager, and the ExtensionActionManager
is owned by the profile by being a BrowserContextKeyedService, this is
guaranteed to be safe. This also abstracts out the context of a profile
or BrowserContext from the ExtensionAction class, keeping it more in
line with its data-structure-like concept.

Add a regression test.

BUG=663726 (and possibly others)

Review-Url: https://codereview.chromium.org/2506273002
Cr-Commit-Position: refs/heads/master@{#433361}
(cherry picked from commit d09e1c4d5c955d73cc38f507ede1119f6c540320)
==========

to

==========
[Extensions] Fix lifetime bug in ExtensionAction/IconImage

ExtensionActions can lazily load the default icon image, which is
implemented as an IconImage. The IconImage, in turn, is constructed with
a BrowserContext because it needs access to the ImageLoader (a
BrowserContextKeyedService). ExtensionActions are owned by the
ExtensionActionManager (another BrowserContextKeyedService with one
instance shared across incognito and normal profiles), but took a
BrowserContext as an argument when loading the default icon image.

This all caused a problem when an incognito profile was passed in as
the browser context to load the default icon for the extension action.
The extension action would then create an IconImage with the incognito
profile as a context, but the ExtensionAction (and thus the IconImage)
are owned by the ExtensionActionManager, and are therefore not deleted
upon the incognito profile's destruction. This means that if you have a
flow where the ExtensionAction's default icon is loaded via an incognito
profile, the incognito profile is deleted, and then the icon is later
used in the normal profile, there's a crash.

Fix this by having the ExtensionActionManager assign the IconImage to
the ExtensionAction using its own profile. Since the ExtensionActions
are owned by the ExtensionActionManager, and the ExtensionActionManager
is owned by the profile by being a BrowserContextKeyedService, this is
guaranteed to be safe. This also abstracts out the context of a profile
or BrowserContext from the ExtensionAction class, keeping it more in
line with its data-structure-like concept.

Add a regression test.

BUG=663726 (and possibly others)

Review-Url: https://codereview.chromium.org/2506273002
Cr-Commit-Position: refs/heads/master@{#433361}
(cherry picked from commit d09e1c4d5c955d73cc38f507ede1119f6c540320)

Committed:
https://chromium.googlesource.com/chromium/src/+/bf76b5aaca31f715c74bb3274edf...
==========

[Devlin, 2016-11-30 02:07:47]

Committed patchset #1 (id:1) manually as
bf76b5aaca31f715c74bb3274edf748901c1a6d7.