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

Issue 523151: Fix crash due to parsing of raw branch code from Google Update (Closed)

Created:
10 years, 11 months ago by Finnur
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, kuchhal, ben+cc_chromium.org, John Grabowski
Visibility:
Public.

Description

We should not return the raw Google Update branch code if we fail to determine what branch we are on. We should just assume stable. We should also not add the branch code information to the version string, since it needs to be parsable by Version object. If we append the raw branch code, it might contain dots, which confuses the parsing done by the Version object. BUG=31772 TEST=Change your branch code to something like 3.0-dev or 2.0-dev-somerandomstring. Chrome should not crash when you open the About box. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35763

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -50 lines) Patch
M chrome/browser/views/about_chrome_view.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/views/about_chrome_view.cc View 2 chunks +9 lines, -8 lines 0 comments Download
M chrome/common/platform_util_win.cc View 3 chunks +19 lines, -42 lines 2 comments Download
M chrome/installer/util/google_update_constants.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/installer/util/google_update_constants.cc View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Finnur
10 years, 11 months ago (2010-01-07 23:40:04 UTC) #1
Erik does not do reviews
10 years, 11 months ago (2010-01-07 23:50:51 UTC) #2
LGTM

http://codereview.chromium.org/523151/diff/1/4
File chrome/common/platform_util_win.cc (right):

http://codereview.chromium.org/523151/diff/1/4#newcode165
chrome/common/platform_util_win.cc:165: std::wstring update_branch = L"stable"; 
// The default if we get confused.
per mark's comment, this shouldn't be the default (and it always gets clobbered
anyway).

http://codereview.chromium.org/523151/diff/1/4#newcode199
chrome/common/platform_util_win.cc:199: update_branch = L"";
Add a comment here about why we don't return "stable" (per mark's info).

Powered by Google App Engine
This is Rietveld 408576698