Chromium Code Reviews
Help | Chromium Project | Sign in
(49)

Issue 546106: Ensure method to set up cmd-w/cmd-shift-w runs on the main thread, add a DCHE... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 4 months ago by pink
Modified:
4 years ago
Reviewers:
TVL, rohitrao
CC:
chromium-reviews_googlegroups.com, John Grabowski, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Ensure method to set up cmd-w/cmd-shift-w runs on the main thread, add a DCHECK to try to catch instances where it is called away from the main thread. BUG=32786 TEST=cmd-w/cmd-shift-w should always be correct. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=36755

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -4 lines) Patch
M chrome/browser/app_controller_mac.mm View 1 chunk +6 lines, -4 lines 1 comment Download
Trybot results:  mac 
Commit: CQ not working?

Messages

Total messages: 3 (0 generated)
pink
5 years, 4 months ago (2010-01-21 15:55:36 UTC) #1
TVL
lgtm http://codereview.chromium.org/546106/diff/1/2 File chrome/browser/app_controller_mac.mm (right): http://codereview.chromium.org/546106/diff/1/2#newcode279 chrome/browser/app_controller_mac.mm:279: DCHECK([NSThread currentThread] == [NSThread mainThread]); why not just ...
5 years, 4 months ago (2010-01-21 16:03:32 UTC) #2
pink
5 years, 4 months ago (2010-01-21 16:06:46 UTC) #3
Duh. Fixed.

On Thu, Jan 21, 2010 at 11:03 AM,  <thomasvl@chromium.org> wrote:
> lgtm
>
>
> http://codereview.chromium.org/546106/diff/1/2
> File chrome/browser/app_controller_mac.mm (right):
>
> http://codereview.chromium.org/546106/diff/1/2#newcode279
> chrome/browser/app_controller_mac.mm:279: DCHECK([NSThread
> currentThread] == [NSThread mainThread]);
> why not just [NSThread isMainThread]?
>
> http://codereview.chromium.org/546106
>



-- 
Mike Pinkerton
Mac Weenie
pinkerton@google.com
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be