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

Issue 12770035: Make the TCP Fast Open Flag modifiable from about:flags. (Closed)

Created:
7 years, 9 months ago by Randy Smith (Not in Mondays)
Modified:
7 years, 8 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Make the TCP Fast Open Flag modifiable from about:flags. R=rch@chromium.org BUG=175623 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193453

Patch Set 1 #

Patch Set 2 : Moved location of initialization and included printfs. #

Patch Set 3 : Added an extra bit of debugging. #

Patch Set 4 : Sync'd to r193064 #

Patch Set 5 : Removed debugging code. #

Total comments: 2

Patch Set 6 : Added TODO in response to comment. #

Total comments: 2

Patch Set 7 : Replace tabs with spaces. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -6 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 1 chunk +7 lines, -0 lines 2 comments Download
M chrome/browser/io_thread.cc View 1 2 3 4 5 2 chunks +8 lines, -0 lines 0 comments Download
M content/browser/browser_main_loop.cc View 1 2 3 2 chunks +0 lines, -6 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
Ryan Hamilton
I thought I replied to this earlier, but it looks like I didn't. LGTM, but ...
7 years, 8 months ago (2013-04-02 15:35:25 UTC) #1
Randy Smith (Not in Mondays)
On 2013/04/02 15:35:25, Ryan Hamilton wrote: > I thought I replied to this earlier, but ...
7 years, 8 months ago (2013-04-02 15:53:57 UTC) #2
Randy Smith (Not in Mondays)
Ryan: After poking around some in the OWNERS files and looking at who has hacked ...
7 years, 8 months ago (2013-04-09 21:33:39 UTC) #3
Ryan Hamilton
LGTM https://codereview.chromium.org/12770035/diff/12002/chrome/browser/io_thread.cc File chrome/browser/io_thread.cc (right): https://codereview.chromium.org/12770035/diff/12002/chrome/browser/io_thread.cc#newcode674 chrome/browser/io_thread.cc:674: net::SetTCPFastOpenEnabled(true); SetTCPFastOpenEnabled predates your involvement, so I hesitate ...
7 years, 8 months ago (2013-04-09 22:08:25 UTC) #4
Randy Smith (Not in Mondays)
Scott, could you review? I believe it's a rubberstamp--I've already had a full review from ...
7 years, 8 months ago (2013-04-10 15:20:11 UTC) #5
Ryan Hamilton
On 2013/04/10 15:20:11, rdsmith wrote: > I've added the TODO, but a) I don't completely ...
7 years, 8 months ago (2013-04-10 15:31:00 UTC) #6
sky
LGTM https://codereview.chromium.org/12770035/diff/20001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/12770035/diff/20001/chrome/app/generated_resources.grd#newcode6512 chrome/app/generated_resources.grd:6512: + Enable TCP Fast Open nit: indent one ...
7 years, 8 months ago (2013-04-10 16:27:22 UTC) #7
Randy Smith (Not in Mondays)
Thanks! https://codereview.chromium.org/12770035/diff/20001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/12770035/diff/20001/chrome/app/generated_resources.grd#newcode6512 chrome/app/generated_resources.grd:6512: + Enable TCP Fast Open On 2013/04/10 16:27:22, ...
7 years, 8 months ago (2013-04-10 17:12:36 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdsmith@chromium.org/12770035/26001
7 years, 8 months ago (2013-04-10 17:12:52 UTC) #9
commit-bot: I haz the power
Change committed as 193453
7 years, 8 months ago (2013-04-10 20:52:24 UTC) #10
Ryan Sleevi
On 2013/04/10 20:52:24, I haz the power (commit-bot) wrote: > Change committed as 193453 Can ...
7 years, 8 months ago (2013-04-11 19:29:18 UTC) #11
Randy Smith (Not in Mondays)
On 2013/04/11 19:29:18, Ryan Sleevi wrote: > On 2013/04/10 20:52:24, I haz the power (commit-bot) ...
7 years, 8 months ago (2013-04-11 19:31:21 UTC) #12
willchan no longer on Chromium
https://codereview.chromium.org/12770035/diff/26001/chrome/browser/about_flags.cc File chrome/browser/about_flags.cc (right): https://codereview.chromium.org/12770035/diff/26001/chrome/browser/about_flags.cc#newcode1315 chrome/browser/about_flags.cc:1315: kOsAll, Very minor, but we should only enable this ...
7 years, 8 months ago (2013-04-11 22:07:41 UTC) #13
Randy Smith (Not in Mondays)
7 years, 8 months ago (2013-04-12 20:10:56 UTC) #14
Message was sent while issue was closed.
https://codereview.chromium.org/12770035/diff/26001/chrome/browser/about_flag...
File chrome/browser/about_flags.cc (right):

https://codereview.chromium.org/12770035/diff/26001/chrome/browser/about_flag...
chrome/browser/about_flags.cc:1315: kOsAll,
On 2013/04/11 22:07:41, willchan wrote:
> Very minor, but we should only enable this for Linux, ChromeOS, and Android
> (even though I don't think Android kernels are new enough to support this
yet).
> Francois's public G+ followers are assuming that since this appears on
Windows,
> Windows must support TCP Fast Open. Obviously not a big deal, but might be
nice
> to do so to reduce confusion.

Uploaded CL: https://codereview.chromium.org/14234018

Powered by Google App Engine
This is Rietveld 408576698