Respect QuicAllowed policy for new streams

chrome/browser/policy/policy_network_browsertest.cc

 // Copyright (c) 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
@@ skipping 10 matching lines @@
 #include "net/http/http_transaction_factory.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace {
 
 void VerifyQuicEnabledStatus(net::URLRequestContextGetter* getter,
                              bool quic_enabled_expected,
                              const base::Closure& done_callback) {
   net::URLRequestContext* context = getter->GetURLRequestContext();
-  bool quic_enabled = context->http_transaction_factory()->GetSession()->
-        params().enable_quic;
-  EXPECT_EQ(quic_enabled_expected, quic_enabled);
+  bool quic_enabled_for_new_streams =

[Bence, 2016/12/02 18:53:26]

Please run git cl format to reformat this multi-line statement.

[pmarko, 2016/12/08 17:01:59]

Done.

+      context->http_transaction_factory()->GetSession()->
+      params().is_enable_quic_for_new_streams();
+  EXPECT_EQ(quic_enabled_expected, quic_enabled_for_new_streams);
 
   content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
                                    done_callback);
 }
 
 void VerifyQuicEnabledStatusInIOThread(bool quic_enabled_expected) {
   base::RunLoop run_loop;
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(VerifyQuicEnabledStatus,
                  base::RetainedRef(g_browser_process->system_request_context()),
                  quic_enabled_expected, run_loop.QuitClosure()));
   run_loop.Run();
 }
 
 }  // namespace
 
 namespace policy {
 
 // The tests are based on the assumption that command line flag kEnableQuic
 // guarantees that QUIC protocol is enabled which is the case at the moment
 // when these are being written.
 class QuicAllowedPolicyTestBase: public InProcessBrowserTest {
  public:
-  QuicAllowedPolicyTestBase() : InProcessBrowserTest(){}
+  QuicAllowedPolicyTestBase() : InProcessBrowserTest() {}
 
  protected:
   void SetUpInProcessBrowserTestFixture() override {
     base::CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnableQuic);
     EXPECT_CALL(provider_, IsInitializationComplete(testing::_))
         .WillRepeatedly(testing::Return(true));
 
     BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
     PolicyMap values;
     GetQuicAllowedPolicy(&values);
     provider_.UpdateChromePolicy(values);
   }
 
   virtual void GetQuicAllowedPolicy(PolicyMap* values) = 0;
 
+  MockConfigurationPolicyProvider provider_;
  private:
-  MockConfigurationPolicyProvider provider_;
   DISALLOW_COPY_AND_ASSIGN(QuicAllowedPolicyTestBase);
 };
 
 // Policy QuicAllowed set to false.
 class QuicAllowedPolicyIsFalse: public QuicAllowedPolicyTestBase {
  public:
-  QuicAllowedPolicyIsFalse() : QuicAllowedPolicyTestBase(){}
+  QuicAllowedPolicyIsFalse() : QuicAllowedPolicyTestBase() {}
 
  protected:
   void GetQuicAllowedPolicy(PolicyMap* values) override {
     values->Set(key::kQuicAllowed, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
                 POLICY_SOURCE_CLOUD,
                 base::MakeUnique<base::FundamentalValue>(false), nullptr);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(QuicAllowedPolicyIsFalse);
 };
 
 IN_PROC_BROWSER_TEST_F(QuicAllowedPolicyIsFalse, QuicDisallowed) {
   VerifyQuicEnabledStatusInIOThread(false);
 }
 
 // Policy QuicAllowed set to true.
 class QuicAllowedPolicyIsTrue: public QuicAllowedPolicyTestBase {
  public:
-  QuicAllowedPolicyIsTrue() : QuicAllowedPolicyTestBase(){}
+  QuicAllowedPolicyIsTrue() : QuicAllowedPolicyTestBase() {}
 
  protected:
   void GetQuicAllowedPolicy(PolicyMap* values) override {
     values->Set(key::kQuicAllowed, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
                 POLICY_SOURCE_CLOUD,
                 base::MakeUnique<base::FundamentalValue>(true), nullptr);
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(QuicAllowedPolicyIsTrue);
 };
 
 IN_PROC_BROWSER_TEST_F(QuicAllowedPolicyIsTrue, QuicAllowed) {
   VerifyQuicEnabledStatusInIOThread(true);
 }
 
 // Policy QuicAllowed is not set.
 class QuicAllowedPolicyIsNotSet: public QuicAllowedPolicyTestBase {
  public:
-  QuicAllowedPolicyIsNotSet() : QuicAllowedPolicyTestBase(){}
+  QuicAllowedPolicyIsNotSet() : QuicAllowedPolicyTestBase() {}
 
  protected:
   void GetQuicAllowedPolicy(PolicyMap* values) override {
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(QuicAllowedPolicyIsNotSet);
 };
 
 IN_PROC_BROWSER_TEST_F(QuicAllowedPolicyIsNotSet, NoQuicRegulations) {
   VerifyQuicEnabledStatusInIOThread(true);
 }
 
+// Policy QuicAllowed is set after IOThread initialization.
+class QuicAllowedPolicyIsSetToFalseAfterInit: public QuicAllowedPolicyTestBase {

[Bence, 2016/12/02 18:53:26]

Space required before ":".  With that, this line is too long.  Use git cl format
to break it up in conformance with the style guide.

[pmarko, 2016/12/08 17:01:59]

Done.

+ public:
+  QuicAllowedPolicyIsSetToFalseAfterInit() : QuicAllowedPolicyTestBase() {}

[Bence, 2016/12/02 18:53:25]

I believe that it is unnecessary to define this constructor; please remove it.

[pmarko, 2016/12/08 17:01:59]

The problem is that all classes in this file have DISALLOW_COPY_AND_ASSIGN which
as a side effect causes no default constructor to be implicitly created by the
compiler... Alternatives:
- remove DISALLOW_COPY_AND_ASSIGN and explicit default constructors on the
classes in this file
- leave it as it is

What do you think? I think that DISALLOW_COPY_AND_ASSIGN is not really necessary
in test fixtures (what against are we protecting against?) but I'm new so I
might be wrong.

[Bence, 2016/12/08 17:41:59]

Sorry, I didn't realize that.  Then I think it's better just to leave it as it
is.  Thanks.

[pmarko, 2016/12/08 19:56:54]

Acknowledged.

+
+ protected:
+  void GetQuicAllowedPolicy(PolicyMap* values) override {
+  }

[Bence, 2016/12/02 18:53:25]

Move this closing brace to the previous line.

[pmarko, 2016/12/08 17:01:59]

Done.

+
+  void SetUpOnMainThread() override {
+    PolicyMap values;
+    values.Set(key::kQuicAllowed, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+                POLICY_SOURCE_CLOUD,

[Bence, 2016/12/02 18:53:25]

Move these two lines one space to the left please.

[pmarko, 2016/12/08 17:01:59]

Done.

+                base::MakeUnique<base::FundamentalValue>(false), nullptr);
+    provider_.UpdateChromePolicy(values);
+    base::RunLoop().RunUntilIdle();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(QuicAllowedPolicyIsSetToFalseAfterInit);
+};
+
+IN_PROC_BROWSER_TEST_F(QuicAllowedPolicyIsSetToFalseAfterInit, QuicDisallowed) {
+  VerifyQuicEnabledStatusInIOThread(false);
+}
 }  // namespace policy