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

Issue 10911198: SPDY - disable SPDY based on command line and policy (Closed)

Created:
8 years, 3 months ago by ramant (doing other things)
Modified:
8 years, 3 months ago
Reviewers:
Joao da Silva, mmenke
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org, jar (doing other things), willchan no longer on Chromium
Visibility:
Public.

Description

SPDY - If spdy.disable is not enabled in policy file, then don't update if SPDY is enabled or not. Leave the current state (as modified by the command line argument or as defined by the default value). Will submit a separate CL not to use command line arguments that disable SPDY if Policy file specifies spdy.disable pref. R=mmenke@chromium.org, joaodasilva@chromium.org BUG=83895 TEST=browser unit tests. Specify --disable-spdy command line switch and in chrome://net-internals SPDY should say it is off. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=156863

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 3

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M chrome/browser/net/net_pref_observer.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 22 (0 generated)
ramant (doing other things)
8 years, 3 months ago (2012-09-12 01:00:20 UTC) #1
mmenke
http://codereview.chromium.org/10911198/diff/12001/chrome/browser/net/net_pref_observer.cc File chrome/browser/net/net_pref_observer.cc (right): http://codereview.chromium.org/10911198/diff/12001/chrome/browser/net/net_pref_observer.cc#newcode51 chrome/browser/net/net_pref_observer.cc:51: net::HttpStreamFactory::set_spdy_enabled(!*spdy_disabled_); This removes the ability of modifying the policy ...
8 years, 3 months ago (2012-09-12 01:31:17 UTC) #2
ramant (doing other things)
http://codereview.chromium.org/10911198/diff/12001/chrome/browser/net/net_pref_observer.cc File chrome/browser/net/net_pref_observer.cc (right): http://codereview.chromium.org/10911198/diff/12001/chrome/browser/net/net_pref_observer.cc#newcode51 chrome/browser/net/net_pref_observer.cc:51: net::HttpStreamFactory::set_spdy_enabled(!*spdy_disabled_); On 2012/09/12 01:31:17, Matt Menke wrote: > This ...
8 years, 3 months ago (2012-09-12 01:51:52 UTC) #3
mmenke
http://codereview.chromium.org/10911198/diff/12001/chrome/browser/net/net_pref_observer.cc File chrome/browser/net/net_pref_observer.cc (right): http://codereview.chromium.org/10911198/diff/12001/chrome/browser/net/net_pref_observer.cc#newcode51 chrome/browser/net/net_pref_observer.cc:51: net::HttpStreamFactory::set_spdy_enabled(!*spdy_disabled_); On 2012/09/12 01:51:52, ramant wrote: > On 2012/09/12 ...
8 years, 3 months ago (2012-09-12 02:02:45 UTC) #4
mmenke
Oh, and LGTM.
8 years, 3 months ago (2012-09-12 02:26:09 UTC) #5
ramant (doing other things)
On 2012/09/12 02:26:09, Matt Menke wrote: > Oh, and LGTM. Thanks very much mmenke.
8 years, 3 months ago (2012-09-12 02:54:00 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10911198/12001
8 years, 3 months ago (2012-09-12 02:54:31 UTC) #7
commit-bot: I haz the power
Presubmit check for 10911198-12001 failed and returned exit status 1. Running presubmit commit checks ...
8 years, 3 months ago (2012-09-12 02:54:34 UTC) #8
ramant (doing other things)
joaodasilva@: can I get OWNERs rubber stamp for policy_browsertest.cc changes? thanks much.
8 years, 3 months ago (2012-09-12 02:57:54 UTC) #9
Joao da Silva
This is not the way that the command line interacts with policies; usually policies always ...
8 years, 3 months ago (2012-09-12 10:51:43 UTC) #10
ramant (doing other things)
Hi Matt Menke, Joao da Silva, Made changes per Joao da Silva suggestion. Could you ...
8 years, 3 months ago (2012-09-13 01:39:39 UTC) #11
Joao da Silva
I initially thought --use-spdy was an on/off flag but I've now realized it can configure ...
8 years, 3 months ago (2012-09-13 10:19:15 UTC) #12
Joao da Silva
lgtm. Please update the issue description before landing.
8 years, 3 months ago (2012-09-14 08:52:40 UTC) #13
mmenke
On 2012/09/14 08:52:40, Joao da Silva wrote: > lgtm. Please update the issue description before ...
8 years, 3 months ago (2012-09-14 13:12:47 UTC) #14
mmenke
On 2012/09/14 13:12:47, Matt Menke wrote: > On 2012/09/14 08:52:40, Joao da Silva wrote: > ...
8 years, 3 months ago (2012-09-14 13:14:16 UTC) #15
Joao da Silva
On Fri, Sep 14, 2012 at 3:14 PM, <mmenke@chromium.org> wrote: > On 2012/09/14 13:12:47, Matt ...
8 years, 3 months ago (2012-09-14 13:18:22 UTC) #16
mmenke
On 2012/09/14 13:18:22, Joao da Silva wrote: > On Fri, Sep 14, 2012 at 3:14 ...
8 years, 3 months ago (2012-09-14 14:06:09 UTC) #17
ramant (doing other things)
On 2012/09/14 14:06:09, Matt Menke wrote: > On 2012/09/14 13:18:22, Joao da Silva wrote: > ...
8 years, 3 months ago (2012-09-14 15:59:44 UTC) #18
Joao da Silva
On Fri, Sep 14, 2012 at 5:59 PM, <rtenneti@chromium.org> wrote: > On 2012/09/14 14:06:09, Matt ...
8 years, 3 months ago (2012-09-14 16:05:07 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10911198/12005
8 years, 3 months ago (2012-09-14 17:16:08 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtenneti@chromium.org/10911198/12005
8 years, 3 months ago (2012-09-14 17:41:38 UTC) #21
commit-bot: I haz the power
8 years, 3 months ago (2012-09-14 19:53:18 UTC) #22
Change committed as 156863

Powered by Google App Engine
This is Rietveld 408576698