Issue 407043002: Content Verification: Don't access UI-thread objects on the IO thread

Issue 407043002: Content Verification: Don't access UI-thread objects on the IO thread (Closed)

Created:
6 years, 5 months ago by asargent_no_longer_on_chrome

Modified:
6 years, 5 months ago

Reviewers:
Ken Rockot(use gerrit already)

CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Project:
chromium

Visibility:
Public.

More Reviews

Description

Content Verification: Don't access UI-thread objects on the IO thread We ask to create ContentVerifyJob's on the IO thread as we read files from extension directories, but we are accidentally accessing state that lives on the UI thread (eg the ExtensionRegistry) while deciding whether to do that or not. This changes things so that as we discover extensions loading up, we copy the data we need to make those decisions to an object on the IO thread. Also fix a bug that was keeping BOOTSTRAP mode from working, and one or two other minor issues I found while I was staring at things. BUG=395873, 392216 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284804

Patch Set 1 #

Total comments: 2

Patch Set 2 : added DCHECK #

Created: 6 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+277 lines, -91 lines)			Patch
M	chrome/browser/extensions/extension_system_impl.cc	View		2 chunks	+10 lines, -2 lines	0 comments	Download
M	chrome/browser/extensions/user_script_master.cc	View	1	2 chunks	+9 lines, -5 lines	0 comments	Download
M	extensions/browser/content_hash_fetcher.h	View		5 chunks	+5 lines, -18 lines	0 comments	Download
M	extensions/browser/content_hash_fetcher.cc	View		6 chunks	+5 lines, -15 lines	0 comments	Download
M	extensions/browser/content_verifier.h	View		4 chunks	+33 lines, -7 lines	0 comments	Download
M	extensions/browser/content_verifier.cc	View		7 chunks	+96 lines, -38 lines	0 comments	Download
M	extensions/browser/content_verifier_delegate.h	View		1 chunk	+2 lines, -2 lines	0 comments	Download
A	extensions/browser/content_verifier_io_data.h	View		1 chunk	+53 lines, -0 lines	0 comments	Download
A	extensions/browser/content_verifier_io_data.cc	View		1 chunk	+57 lines, -0 lines	0 comments	Download
M	extensions/browser/content_verify_job.cc	View		1 chunk	+5 lines, -4 lines	0 comments	Download
M	extensions/extensions.gyp	View		1 chunk	+2 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Ken Rockot(use gerrit already)

lgtm https://codereview.chromium.org/407043002/diff/1/chrome/browser/extensions/user_script_master.cc File chrome/browser/extensions/user_script_master.cc (right): https://codereview.chromium.org/407043002/diff/1/chrome/browser/extensions/user_script_master.cc#newcode42 chrome/browser/extensions/user_script_master.cc:42: void VerifyContent(scoped_refptr<ContentVerifier> verifier, Could you DCHECK that you're ...

6 years, 5 months ago (2014-07-22 16:44:22 UTC) #2

asargent_no_longer_on_chrome

https://codereview.chromium.org/407043002/diff/1/chrome/browser/extensions/user_script_master.cc File chrome/browser/extensions/user_script_master.cc (right): https://codereview.chromium.org/407043002/diff/1/chrome/browser/extensions/user_script_master.cc#newcode42 chrome/browser/extensions/user_script_master.cc:42: void VerifyContent(scoped_refptr<ContentVerifier> verifier, On 2014/07/22 16:44:22, Ken Rockot wrote: ...

6 years, 5 months ago (2014-07-22 20:27:28 UTC) #3

asargent_no_longer_on_chrome

The CQ bit was checked by asargent@chromium.org

6 years, 5 months ago (2014-07-22 20:27:33 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/asargent@chromium.org/407043002/20001

6 years, 5 months ago (2014-07-22 20:29:38 UTC) #5

Message was sent while issue was closed.

Change committed as 284804

Expand Messages | Collapse Messages