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

Issue 333049: Make KeystoneGlue less antisocial (Closed)

Created:
11 years, 1 month ago by Mark Mentovai
Modified:
9 years, 7 months ago
Reviewers:
TVL
CC:
chromium-reviews_googlegroups.com, John Grabowski, ben+cc_chromium.org, pam+watch_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Make KeystoneGlue less antisocial. Remove -clearRecentStatus which really tied KeystoneGlue too closely to AboutWindowController. If KeystoneGlue ever wound up with any other clients, this relationship would have been harmful. AboutWindowController now maintains a static variable to handle what -clearRecentStatus had been used for. Remove -releaseDefaultKeystoneGlue, which was only intended to be used for testing, as a workaround for test flake. The test flake has been removed. BUG=none TEST=none r30215

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -28 lines) Patch
M chrome/app/keystone_glue.h View 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/app/keystone_glue.mm View 2 chunks +0 lines, -9 lines 0 comments Download
M chrome/app/keystone_glue_unittest.mm View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/cocoa/about_window_controller.mm View 1 5 chunks +15 lines, -7 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Mark Mentovai
11 years, 1 month ago (2009-10-27 16:09:42 UTC) #1
TVL
LGTM with question about variable name http://codereview.chromium.org/333049/diff/1/5 File chrome/browser/cocoa/about_window_controller.mm (right): http://codereview.chromium.org/333049/diff/1/5#newcode104 Line 104: static BOOL ...
11 years, 1 month ago (2009-10-27 16:28:22 UTC) #2
Mark Mentovai
11 years, 1 month ago (2009-10-27 16:31:43 UTC) #3
thomasvl@chromium.org wrote:
> http://codereview.chromium.org/333049/diff/1/5#newcode104
> Line 104: static BOOL showingInstallFailedStatus = NO;
> Would hasShownInstallFailureStatus make more sense?

That makes it sound like "has an about box ever shown the install
failure status?" but that's not what we're using it for.

How about recentShownInstallFailureStatus?

Mark

Powered by Google App Engine
This is Rietveld 408576698