Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(70)

Issue 2537263003: [Extensions] Fix lifetime bug in ExtensionAction/IconImage (Closed)

Created:
2 years, 3 months ago by Devlin
Modified:
2 years, 3 months ago
Reviewers:
CC:
chromium-reviews
Target Ref:
refs/pending/branch-heads/2924
Project:
chromium
Visibility:
Public.

Description

[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

Patch Set 1 #

Messages

Total messages: 2 (1 generated)
Devlin
2 years, 3 months ago (2016-11-30 02:07:47 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
bf76b5aaca31f715c74bb3274edf748901c1a6d7.

Powered by Google App Engine
This is Rietveld 408576698