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

Issue 5519007: Fix a race in the ChromeFrame metrics service start up code. This race could ... (Closed)

Created:
10 years ago by ananta
Modified:
9 years, 6 months ago
Reviewers:
MAD, hansl, grt (UTC plus 2)
CC:
chromium-reviews, amit
Visibility:
Public.

Description

Fix a race in the ChromeFrame metrics service start up code. This race could be triggered if multiple BHO's attempted to start the metrics service object from different threads. Fix is to lock access to the Start and Stop methods. Fixes bug http://b/issue?id=3251823 BUG=3251823 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=68261

Patch Set 1 #

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -1 line) Patch
M chrome_frame/metrics_service.h View 2 chunks +4 lines, -1 line 0 comments Download
M chrome_frame/metrics_service.cc View 1 4 chunks +7 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
ananta
10 years ago (2010-12-03 22:25:10 UTC) #1
hansl
LGTM.
10 years ago (2010-12-03 22:34:15 UTC) #2
hansl
Just to make sure, this is assuming tests pass with this change. On 2010/12/03 22:34:15, ...
10 years ago (2010-12-03 22:34:34 UTC) #3
grt (UTC plus 2)
10 years ago (2010-12-06 15:31:46 UTC) #4
On Fri, Dec 3, 2010 at 5:25 PM, <ananta@chromium.org> wrote:

> ...
> Index: chrome_frame/metrics_service.cc
> ===================================================================
> --- chrome_frame/metrics_service.cc     (revision 68089)
> +++ chrome_frame/metrics_service.cc     (working copy)
> ...



> @@ -378,6 +380,7 @@
>   thread_ = PlatformThread::CurrentId();
>
>   user_permits_upload_ = GoogleUpdateSettings::GetCollectStatsConsent();
> +  user_permits_upload_ = true;
>
>
It looks like this line was inserted for development/testing purposes.  Does
it belong?

Powered by Google App Engine
This is Rietveld 408576698