Issue 4957005: Move themes garbage-collection out of shutdown code.

Issue 4957005: Move themes garbage-collection out of shutdown code. (Closed)

Created:
10 years, 1 month ago by akalin

Modified:
9 years, 7 months ago

Reviewers:
asargent_no_longer_on_chrome

CC:
chromium-reviews, ncarter (slow), ben+cc_chromium.org, Raghu Simha, Erik does not do reviews, idana, Aaron Boodman, pam+watch_chromium.org, Paweł Hajdan Jr., tim (not reviewing)

Base URL:
http://git.chromium.org/git/chromium.git@trunk

Visibility:
Public.

Description

Move themes garbage-collection out of shutdown code. Moved it to GarbageCollectExtensions(). This avoids some problems with shutdown ordering causing crashes. Re-enable themes sync integration tests on OS X (which were crashing). BUG=63285, 62869 TEST=themes sync integration tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=66347

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed asargent's comments #

Created: 10 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+11 lines, -25 lines)			Patch
M	chrome/browser/extensions/extensions_service.cc	View	1	2 chunks	+8 lines, -0 lines	0 comments	Download
M	chrome/browser/themes/browser_theme_provider.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/themes/browser_theme_provider.cc	View		1 chunk	+0 lines, -2 lines	0 comments	Download
M	chrome/test/live_sync/single_client_live_themes_sync_test.cc	View		2 chunks	+0 lines, -10 lines	0 comments	Download
M	chrome/test/live_sync/two_client_live_themes_sync_test.cc	View		2 chunks	+0 lines, -10 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

asargent_no_longer_on_chrome

lgtm http://codereview.chromium.org/4957005/diff/1/chrome/browser/extensions/extensions_service.cc File chrome/browser/extensions/extensions_service.cc (right): http://codereview.chromium.org/4957005/diff/1/chrome/browser/extensions/extensions_service.cc#newcode1466 chrome/browser/extensions/extensions_service.cc:1466: if (profile_) { I think it's safe to ...

10 years, 1 month ago (2010-11-16 23:13:36 UTC) #2

akalin

10 years, 1 month ago (2010-11-17 00:29:38 UTC) #3

Submitting.

http://codereview.chromium.org/4957005/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extensions_service.cc (right):

http://codereview.chromium.org/4957005/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extensions_service.cc:1466: if (profile_) {
On 2010/11/16 23:13:36, Antony Sargent wrote:
> I think it's safe to assume the profile_ is valid at this point. 

True, but i want to be defensive given that this may be called on a timer in the
future.

Added a comment.

Expand Messages | Collapse Messages