Issue 2093009: Run WebSocket experiment if reporting is active.

Issue 2093009: Run WebSocket experiment if reporting is active. (Closed)

Created:
10 years, 7 months ago by ukai

Modified:
9 years, 7 months ago

Reviewers:
tony, brettw

CC:
chromium-reviews, jam, Paweł Hajdan Jr., ben+cc_chromium.org, brettw-cc_chromium.org, cbentzel+watch_chromium.org, darin-cc_chromium.org

Visibility:
Public.

Description

Run WebSocket experiment if reporting is active. BUG=44626 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47775

Patch Set 2 : Fix for tony's review #

Total comments: 1

Created: 10 years, 7 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+12 lines, -2 lines)			Patch
	M	chrome/browser/browser_main.cc	View	1	2 chunks	+3 lines, -2 lines	1 comment	Download
	M	chrome/browser/net/websocket_experiment/websocket_experiment_runner.h	View		1 chunk	+9 lines, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

tony

http://codereview.chromium.org/2093009/diff/1/2 File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/2093009/diff/1/2#newcode1162 chrome/browser/browser_main.cc:1162: if (metrics->reporting_active()) I think checking reporting_active() is a bit ...

10 years, 7 months ago (2010-05-20 03:09:33 UTC) #2

tony

On 2010/05/20 03:09:33, tony wrote: > http://codereview.chromium.org/2093009/diff/1/2 > File chrome/browser/browser_main.cc (right): > > http://codereview.chromium.org/2093009/diff/1/2#newcode1162 > ...

10 years, 7 months ago (2010-05-20 03:17:14 UTC) #3

ukai

10 years, 7 months ago (2010-05-20 04:33:16 UTC) #4

tony

LGTM, thanks! http://codereview.chromium.org/2093009/diff/5001/6001 File chrome/browser/browser_main.cc (right): http://codereview.chromium.org/2093009/diff/5001/6001#newcode502 chrome/browser/browser_main.cc:502: chrome_browser_net_websocket_experiment::WebSocketExperimentRunner::Start(); Nit: 80 cols.

10 years, 7 months ago (2010-05-20 04:34:45 UTC) #5

brettw

10 years, 7 months ago (2010-05-20 04:40:47 UTC) #6

LGTM2, thanks for working on this.

Brett

On Wed, May 19, 2010 at 9:33 PM,  <ukai@chromium.org> wrote:
>
> http://codereview.chromium.org/2093009/diff/1/2
> File chrome/browser/browser_main.cc (right):
>
> http://codereview.chromium.org/2093009/diff/1/2#newcode1162
> chrome/browser/browser_main.cc:1162: if (metrics->reporting_active())
> On 2010/05/20 03:09:33, tony wrote:
>>
>> I think checking reporting_active() is a bit confusing.  I would just
>
> move this
>>
>> into InitializeMetrics next to metrics->Start().
>
> Done.
>
> http://codereview.chromium.org/2093009/show
>

Expand Messages | Collapse Messages