Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(929)

Issue 247113002: [Extension] Protect all installed extension from extension storage GC (Closed)

Created:
6 years, 8 months ago by tzik
Modified:
6 years, 7 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, kinuko+watch
Visibility:
Public.

Description

[Extension] Protect all installed extension from extension storage GC ExtensionGarbageCollector collects isolated storages of disabled apps as garbages, this may cause unexpected data loss for temporarily disabled extensions. This CL protects non-enabled installed app's isolated storage from GC. BUG=328637 R=ajwong@chromium.org, mek@chromium.org, rdevlin.cronin@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266473

Patch Set 1 #

Patch Set 2 : update .h comment #

Total comments: 7

Patch Set 3 : drop comma #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -7 lines) Patch
M chrome/browser/extensions/extension_garbage_collector.h View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_garbage_collector.cc View 1 chunk +4 lines, -4 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
tzik
Hi, Marijn and Albert. Please take a look. Regarding crbug.com/328637, I'd like to protect storage ...
6 years, 8 months ago (2014-04-22 10:16:41 UTC) #1
Marijn Kruisselbrink
On 2014/04/22 10:16:41, tzik wrote: > Hi, Marijn and Albert. Please take a look. > ...
6 years, 8 months ago (2014-04-22 16:35:20 UTC) #2
awong
On 2014/04/22 16:35:20, Marijn Kruisselbrink wrote: > On 2014/04/22 10:16:41, tzik wrote: > > Hi, ...
6 years, 8 months ago (2014-04-22 18:13:39 UTC) #3
tzik
On 2014/04/22 16:35:20, Marijn Kruisselbrink wrote: > On 2014/04/22 10:16:41, tzik wrote: > > Hi, ...
6 years, 8 months ago (2014-04-23 04:15:12 UTC) #4
tzik
On 2014/04/22 18:13:39, awong wrote: > On 2014/04/22 16:35:20, Marijn Kruisselbrink wrote: > > On ...
6 years, 8 months ago (2014-04-23 04:38:15 UTC) #5
tzik
6 years, 8 months ago (2014-04-23 04:38:22 UTC) #6
Marijn Kruisselbrink
On 2014/04/23 04:15:12, tzik wrote: > On 2014/04/22 16:35:20, Marijn Kruisselbrink wrote: > > On ...
6 years, 8 months ago (2014-04-23 18:19:43 UTC) #7
Devlin
https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc File chrome/browser/extensions/extension_garbage_collector.cc (right): https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc#newcode224 chrome/browser/extensions/extension_garbage_collector.cc:224: ExtensionRegistry::Get(context_)->GenerateInstalledExtensionsSet(); It's a shame we have to keep around ...
6 years, 8 months ago (2014-04-23 21:16:49 UTC) #8
Devlin
https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc File chrome/browser/extensions/extension_garbage_collector.cc (right): https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc#newcode224 chrome/browser/extensions/extension_garbage_collector.cc:224: ExtensionRegistry::Get(context_)->GenerateInstalledExtensionsSet(); It's a shame we have to keep around ...
6 years, 8 months ago (2014-04-23 21:16:50 UTC) #9
Marijn Kruisselbrink
https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc File chrome/browser/extensions/extension_garbage_collector.cc (right): https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc#newcode224 chrome/browser/extensions/extension_garbage_collector.cc:224: ExtensionRegistry::Get(context_)->GenerateInstalledExtensionsSet(); On 2014/04/23 21:16:53, D Cronin wrote: > It's ...
6 years, 8 months ago (2014-04-23 23:09:51 UTC) #10
Devlin
https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc File chrome/browser/extensions/extension_garbage_collector.cc (right): https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.cc#newcode224 chrome/browser/extensions/extension_garbage_collector.cc:224: ExtensionRegistry::Get(context_)->GenerateInstalledExtensionsSet(); On 2014/04/23 23:09:51, Marijn Kruisselbrink wrote: > On ...
6 years, 8 months ago (2014-04-23 23:21:06 UTC) #11
awong
Chrome Apps all have "Isolated storage." Thus this is all the storage for the app ...
6 years, 8 months ago (2014-04-24 00:23:50 UTC) #12
Devlin
ah, okay, thanks for the explanations. lgtm.
6 years, 8 months ago (2014-04-24 15:53:48 UTC) #13
awong
LGTM w/ 2 small comment nits. https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.h File chrome/browser/extensions/extension_garbage_collector.h (right): https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.h#newcode63 chrome/browser/extensions/extension_garbage_collector.h:63: // Garbage collects ...
6 years, 8 months ago (2014-04-24 16:42:11 UTC) #14
tzik
https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.h File chrome/browser/extensions/extension_garbage_collector.h (right): https://codereview.chromium.org/247113002/diff/20001/chrome/browser/extensions/extension_garbage_collector.h#newcode63 chrome/browser/extensions/extension_garbage_collector.h:63: // Garbage collects apps/extensions isolated storage, if it is ...
6 years, 8 months ago (2014-04-25 06:29:56 UTC) #15
tzik
The CQ bit was checked by tzik@chromium.org
6 years, 8 months ago (2014-04-25 06:30:02 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tzik@chromium.org/247113002/30001
6 years, 8 months ago (2014-04-25 08:24:36 UTC) #17
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-25 08:40:24 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on linux_chromium_gn_rel
6 years, 8 months ago (2014-04-25 08:40:25 UTC) #19
tzik
The CQ bit was checked by tzik@chromium.org
6 years, 8 months ago (2014-04-26 03:50:08 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tzik@chromium.org/247113002/30001
6 years, 8 months ago (2014-04-26 03:50:40 UTC) #21
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 8 months ago (2014-04-26 04:34:57 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on win_chromium_rel
6 years, 8 months ago (2014-04-26 04:34:58 UTC) #23
tzik
6 years, 7 months ago (2014-04-28 07:39:48 UTC) #24
Message was sent while issue was closed.
Committed patchset #3 manually as r266473 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698