Issue 2918: Fix for issue 2362: On-demand update hang with "Checking for update..."...

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 2918: Fix for issue 2362: On-demand update hang with "Checking for update..."... (Closed)

Created:
12 years, 3 months ago by Finnur

Modified:
9 years, 6 months ago

Reviewers:
jar (doing other things), jar1

CC:
chromium-reviews_googlegroups.com, darin (slow to review)

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

Description

Fix for issue 2362: On-demand update hang with "Checking for update..." We have been restructuring the message loop code lately and the MessageLoop on the file thread is not dispatching messages as it was before. I have changed the file thread to start with type MessageLoop::TYPE_IO, which pumps messages in such a way that Google Update can communicate back to us. I'm not sure what the best way to test this is but I ran the UI tests and the unit tests and they all pass. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=2334

Patch Set 1 #

Created: 12 years, 3 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+3 lines, -1 line)			Patch
	M	chrome/browser/browser_process_impl.cc	View		1 chunk	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

jar (doing other things)

12 years, 3 months ago (2008-09-17 04:36:58 UTC) #2

LGTM (with landing caveat below)

I suspect his was an oversight about the needs of this thread.  He's been
working to distiguish the threads based on their needs, and this use case must
have slipped by.  Darin may have some alternate suggestion (perchance moving
this service/response to another thread?).  For now, your change is quite
compact and to the point.  

Please watch the tree very carefully when you land for performance regressions. 
I'm not sure if there have been any optimizations on this thread that took
advantage of the semantics before you change (but I don't think there were).

You should look at the performance dashboard, which shows all the graphs of all
the performance results.

Thanks,

Jim

Expand Messages | Collapse Messages