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

Issue 173020: Make Mac first run store sentinel in Profile directory. (Closed)

Created:
11 years, 4 months ago by jeremy
Modified:
9 years, 6 months ago
Reviewers:
Mark Mentovai
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Make Mac first run store sentinel in Profile directory. * Added code to migrate from previous defaults-based first run. * Renamed linux_guid -> posix_guid. BUG=19260 TEST=Open current official release and go through first run UI, then open a release compiled with this patch. First run UI should not be displayed again. TEST=First Run UI should only be displayed once on first run and not anew on each launch. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=23686

Patch Set 1 #

Patch Set 2 : spelling #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+243 lines, -222 lines) Patch
M chrome/app/breakpad_linux.cc View 4 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/first_run.cc View 2 chunks +0 lines, -3 lines 1 comment Download
M chrome/browser/first_run_mac.mm View 1 5 chunks +83 lines, -19 lines 4 comments Download
A chrome/browser/first_run_migration_mac_unittest.mm View 1 1 chunk +62 lines, -0 lines 2 comments Download
M chrome/browser/google_update_settings_linux.cc View 1 chunk +0 lines, -62 lines 0 comments Download
M chrome/browser/google_update_settings_linux_unittest.cc View 1 chunk +0 lines, -23 lines 0 comments Download
M chrome/browser/google_update_settings_mac.mm View 1 chunk +0 lines, -49 lines 0 comments Download
M chrome/browser/google_update_settings_mac_unittest.mm View 1 chunk +0 lines, -53 lines 0 comments Download
A chrome/browser/google_update_settings_posix.cc View 1 chunk +62 lines, -0 lines 1 comment Download
A chrome/browser/google_update_settings_posix_unittest.cc View 1 chunk +23 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/browser_render_process_host.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome.gyp View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/common/child_process_host.cc View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
jeremy
11 years, 4 months ago (2009-08-18 21:38:51 UTC) #1
Mark Mentovai
11 years, 4 months ago (2009-08-18 21:51:46 UTC) #2
LG with some changes.

http://codereview.chromium.org/173020/diff/28/1002
File chrome/browser/first_run.cc (right):

http://codereview.chromium.org/173020/diff/28/1002#newcode61
Line 61: // A troolean, 0 means not yet set, 1 means set to true, 2 set to
false.
Ought to be an enum.

http://codereview.chromium.org/173020/diff/28/1003
File chrome/browser/first_run_mac.mm (right):

http://codereview.chromium.org/173020/diff/28/1003#newcode22
Line 22: // in the near future once most people are upgraded.
Put a TODO on it.  Put specific conditions for removal.  The style guide covers
this.

http://codereview.chromium.org/173020/diff/28/1003#newcode27
Line 27: // returns - is this the first run?
What's this mean?

http://codereview.chromium.org/173020/diff/28/1003#newcode62
Line 62: // returns - true - performed migration, false - no previous first run
Use complete sentences, especially when you're describing what a function is
supposed to do.

http://codereview.chromium.org/173020/diff/28/1003#newcode190
Line 190: #else
I'd just leave the #else out.  You can keep the comment, but it doesn't need an
#else.

http://codereview.chromium.org/173020/diff/28/1004
File chrome/browser/first_run_migration_mac_unittest.mm (right):

http://codereview.chromium.org/173020/diff/28/1004#newcode5
Line 5: #include <Foundation/Foundation.h>
#import this one.

http://codereview.chromium.org/173020/diff/28/1004#newcode14
Line 14: // for details.
I'm surprised you're adding a test for temporary code.

http://codereview.chromium.org/173020/diff/28/1009
File chrome/browser/google_update_settings_posix.cc (right):

http://codereview.chromium.org/173020/diff/28/1009#newcode15
Line 15: std::string posix_guid;
But we don't ever use this on the Mac.

I'd #ifdef it to keep our code/data size down.

Powered by Google App Engine
This is Rietveld 408576698