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

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

Created:
10 years, 11 months ago by pink (ping after 24hrs)
Modified:
9 years, 7 months ago
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

Messages

Total messages: 3 (0 generated)
pink (ping after 24hrs)
10 years, 11 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 ...
10 years, 11 months ago (2010-01-21 16:03:32 UTC) #2
pink (ping after 24hrs)
10 years, 11 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

Powered by Google App Engine
This is Rietveld 408576698