Deprecate http2_propagate_unknown_settings flag.

This CL also slays some dead methods (*Setting*Old) for old behavior.

This CL lands server change 193221621 by diannahu.

BUG=488484

Change-Id: I420b10a15cef120176cce8842616e47154afc43e
Reviewed-on: https://chromium-review.googlesource.com/1016702
Commit-Queue: Dianna Hu <diannahu@chromium.org>
Reviewed-by: Dianna Hu <diannahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551713}

net/quic/core/quic_headers_stream_test.cc

@@ -76,7 +76,6 @@ class MockVisitor : public SpdyFramerVisitorInterface {
   MOCK_METHOD2(OnRstStream,
                void(SpdyStreamId stream_id, SpdyErrorCode error_code));
   MOCK_METHOD0(OnSettings, void());
-  MOCK_METHOD2(OnSettingOld, void(SpdyKnownSettingsId id, uint32_t value));
   MOCK_METHOD2(OnSetting, void(SpdySettingsId id, uint32_t value));
   MOCK_METHOD0(OnSettingsAck, void());
   MOCK_METHOD0(OnSettingsEnd, void());

net/quic/core/quic_spdy_session.cc

@@ -119,50 +119,7 @@ class QuicSpdySession::SpdyFramerVisitor
                     QUIC_INVALID_HEADERS_STREAM_DATA);
   }
 
-  void OnSettingOld(SpdyKnownSettingsId id, uint32_t value) override {
-    QUIC_BUG_IF(GetQuicRestartFlag(http2_propagate_unknown_settings));
-    if (!GetQuicReloadableFlag(quic_respect_http2_settings_frame)) {
-      CloseConnection("SPDY SETTINGS frame received.",
-                      QUIC_INVALID_HEADERS_STREAM_DATA);
-      return;
-    }
-    switch (id) {
-      case SETTINGS_HEADER_TABLE_SIZE:
-        session_->UpdateHeaderEncoderTableSize(value);
-        break;
-      case SETTINGS_ENABLE_PUSH:
-        if (session_->perspective() == Perspective::IS_SERVER) {
-          // See rfc7540, Section 6.5.2.
-          if (value > 1) {
-            CloseConnection(
-                QuicStrCat("Invalid value for SETTINGS_ENABLE_PUSH: ", value),
-                QUIC_INVALID_HEADERS_STREAM_DATA);
-            return;
-          }
-          session_->UpdateEnableServerPush(value > 0);
-          break;
-        } else {
-          CloseConnection(
-              QuicStrCat("Unsupported field of HTTP/2 SETTINGS frame: ", id),
-              QUIC_INVALID_HEADERS_STREAM_DATA);
-        }
-        break;
-      // TODO(fayang): Need to support SETTINGS_MAX_HEADER_LIST_SIZE when
-      // clients are actually sending it.
-      case SETTINGS_MAX_HEADER_LIST_SIZE:
-        if (GetQuicReloadableFlag(quic_send_max_header_list_size)) {
-          break;
-        }
-        QUIC_FALLTHROUGH_INTENDED;
-      default:
-        CloseConnection(
-            QuicStrCat("Unsupported field of HTTP/2 SETTINGS frame: ", id),
-            QUIC_INVALID_HEADERS_STREAM_DATA);
-    }
-  }
-
   void OnSetting(SpdySettingsId id, uint32_t value) override {
-    QUIC_BUG_IF(!GetQuicRestartFlag(http2_propagate_unknown_settings));
     if (!GetQuicReloadableFlag(quic_respect_http2_settings_frame)) {
       CloseConnection("SPDY SETTINGS frame received.",
                       QUIC_INVALID_HEADERS_STREAM_DATA);

net/quic/http/decoder/quic_http_frame_decoder_adapter.cc

@@ -453,11 +453,7 @@ void QuicHttpDecoderAdapter::OnSetting(
   }
   // TODO(quic): Consider whether to add support for handling unknown SETTINGS
   //     IDs, which currently cause a connection close.
-  if (GetQuicRestartFlag(http2_propagate_unknown_settings)) {
-    visitor()->OnSetting(setting_id, setting_fields.value);
-  } else {
-    visitor()->OnSettingOld(setting_id, setting_fields.value);
-  }
+  visitor()->OnSetting(setting_id, setting_fields.value);
 }
 
 void QuicHttpDecoderAdapter::OnSettingsEnd() {

net/spdy/chromium/buffered_spdy_framer.cc

@@ -140,10 +140,6 @@ void BufferedSpdyFramer::OnSettings() {
   visitor_->OnSettings();
 }
 
-void BufferedSpdyFramer::OnSettingOld(SpdyKnownSettingsId id, uint32_t value) {
-  visitor_->OnSetting(id, value);
-}
-
 void BufferedSpdyFramer::OnSetting(SpdySettingsId id, uint32_t value) {
   visitor_->OnSetting(id, value);
 }

net/spdy/chromium/buffered_spdy_framer.h

@@ -158,7 +158,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer
       SpdyStreamId stream_id) override;
   void OnHeaderFrameEnd(SpdyStreamId stream_id) override;
   void OnSettings() override;
-  void OnSettingOld(SpdyKnownSettingsId id, uint32_t value) override;
   void OnSetting(SpdySettingsId id, uint32_t value) override;
   void OnSettingsAck() override;
   void OnSettingsEnd() override;

net/spdy/core/http2_frame_decoder_adapter.cc

@@ -475,23 +475,10 @@ void Http2DecoderAdapter::OnSettingsStart(const Http2FrameHeader& header) {
 void Http2DecoderAdapter::OnSetting(const Http2SettingFields& setting_fields) {
   DVLOG(1) << "OnSetting: " << setting_fields;
   const auto parameter = static_cast<SpdySettingsId>(setting_fields.parameter);
-  if (GetSpdyRestartFlag(http2_propagate_unknown_settings)) {
-    visitor()->OnSetting(parameter, setting_fields.value);
-    if (extension_ != nullptr) {
-      extension_->OnSetting(parameter, setting_fields.value);
-    }
-    return;
-  }
-  SpdyKnownSettingsId setting_id;
-  if (!ParseSettingsId(parameter, &setting_id)) {
-    if (extension_ == nullptr) {
-      DVLOG(1) << "No extension for unknown setting id: " << setting_fields;
-    } else {
-      extension_->OnSetting(parameter, setting_fields.value);
-    }
-    return;
+  visitor()->OnSetting(parameter, setting_fields.value);
+  if (extension_ != nullptr) {
+    extension_->OnSetting(parameter, setting_fields.value);
   }
-  visitor()->OnSettingOld(setting_id, setting_fields.value);
 }
 
 void Http2DecoderAdapter::OnSettingsEnd() {

net/spdy/core/http2_frame_decoder_adapter.h

@@ -394,12 +394,6 @@ class SPDY_EXPORT_PRIVATE SpdyFramerVisitorInterface {
   // Called when a SETTINGS frame is received.
   virtual void OnSettings() {}
 
-  // Called when a complete setting within a SETTINGS frame has been parsed and
-  // validated.
-  // TODO(diannahu): Remove with deprecation of
-  //     GetSpdyRestartFlag(http2_propagate_unknown_settings).
-  virtual void OnSettingOld(SpdyKnownSettingsId id, uint32_t value) = 0;
-
   // Called when a complete setting within a SETTINGS frame has been parsed.
   // Note that |id| may or may not be a SETTINGS ID defined in the HTTP/2 spec.
   virtual void OnSetting(SpdySettingsId id, uint32_t value) = 0;

net/spdy/core/mock_spdy_framer_visitor.h

@@ -40,7 +40,6 @@ class MockSpdyFramerVisitor : public SpdyFramerVisitorInterface {
   MOCK_METHOD2(OnRstStream,
                void(SpdyStreamId stream_id, SpdyErrorCode error_code));
   MOCK_METHOD0(OnSettings, void());
-  MOCK_METHOD2(OnSettingOld, void(SpdyKnownSettingsId id, uint32_t value));
   MOCK_METHOD2(OnSetting, void(SpdySettingsId id, uint32_t value));
   MOCK_METHOD2(OnPing, void(SpdyPingId unique_id, bool is_ack));
   MOCK_METHOD0(OnSettingsEnd, void());

net/spdy/core/spdy_deframer_visitor.cc

@@ -170,7 +170,6 @@ class SpdyTestDeframerImpl : public SpdyTestDeframer,
                      SpdyStreamId promised_stream_id,
                      bool end) override;
   void OnRstStream(SpdyStreamId stream_id, SpdyErrorCode error_code) override;
-  void OnSettingOld(SpdyKnownSettingsId id, uint32_t value) override;
   void OnSetting(SpdySettingsId id, uint32_t value) override;
   void OnSettings() override;
   void OnSettingsAck() override;
@@ -609,18 +608,6 @@ void SpdyTestDeframerImpl::OnRstStream(SpdyStreamId stream_id,
       SpdyMakeUnique<SpdyRstStreamIR>(stream_id, error_code));
 }
 
-// Called for an individual setting. There is no negotiation; the sender is
-// stating the value that the sender is using.
-void SpdyTestDeframerImpl::OnSettingOld(SpdyKnownSettingsId id,
-                                        uint32_t value) {
-  DVLOG(1) << "OnSettingOld id: " << id << std::hex << "    value: " << value;
-  CHECK_EQ(frame_type_, SETTINGS) << "   frame_type_="
-                                  << Http2FrameTypeToString(frame_type_);
-  CHECK(settings_);
-  settings_->push_back(std::make_pair(id, value));
-  settings_ir_->AddSetting(id, value);
-}
-
 // Called for an individual setting. There is no negotiation; the sender is
 // stating the value that the sender is using.
 void SpdyTestDeframerImpl::OnSetting(SpdySettingsId id, uint32_t value) {

net/spdy/core/spdy_framer_test.cc

@@ -333,11 +333,6 @@ class TestSpdyVisitor : public SpdyFramerVisitorInterface,
     ++fin_frame_count_;
   }
 
-  void OnSettingOld(SpdyKnownSettingsId id, uint32_t value) override {
-    VLOG(1) << "OnSetting(" << id << ", " << std::hex << value << ")";
-    ++setting_count_;
-  }
-
   void OnSetting(SpdySettingsId id, uint32_t value) override {
     VLOG(1) << "OnSetting(" << id << ", " << std::hex << value << ")";
     ++setting_count_;
@@ -3014,11 +3009,7 @@ TEST_P(SpdyFramerTest, ReadUnknownSettingsId) {
 
   // In HTTP/2, we ignore unknown settings because of extensions. However, we
   // pass the SETTINGS to the visitor, which can decide how to handle them.
-  if (GetSpdyRestartFlag(http2_propagate_unknown_settings)) {
-    EXPECT_EQ(1, visitor.setting_count_);
-  } else {
-    EXPECT_EQ(0, visitor.setting_count_);
-  }
+  EXPECT_EQ(1, visitor.setting_count_);
   EXPECT_EQ(0, visitor.error_count_);
 }
 
@@ -3043,24 +3034,14 @@ TEST_P(SpdyFramerTest, ReadKnownAndUnknownSettingsWithExtension) {
 
   // In HTTP/2, we ignore unknown settings because of extensions. However, we
   // pass the SETTINGS to the visitor, which can decide how to handle them.
-  if (GetSpdyRestartFlag(http2_propagate_unknown_settings)) {
-    EXPECT_EQ(3, visitor.setting_count_);
-  } else {
-    EXPECT_EQ(1, visitor.setting_count_);
-  }
+  EXPECT_EQ(3, visitor.setting_count_);
   EXPECT_EQ(0, visitor.error_count_);
 
-  if (GetSpdyRestartFlag(http2_propagate_unknown_settings)) {
-    // The extension receives all SETTINGS, including the non-standard SETTINGS.
-    EXPECT_THAT(
-        extension.settings_received_,
-        testing::ElementsAre(testing::Pair(16, 2), testing::Pair(95, 65538),
-                             testing::Pair(2, 1)));
-  } else {
-    EXPECT_THAT(
-        extension.settings_received_,
-        testing::ElementsAre(testing::Pair(16, 2), testing::Pair(95, 65538)));
-  }
+  // The extension receives all SETTINGS, including the non-standard SETTINGS.
+  EXPECT_THAT(
+      extension.settings_received_,
+      testing::ElementsAre(testing::Pair(16, 2), testing::Pair(95, 65538),
+                           testing::Pair(2, 1)));
 }
 
 // Tests handling of SETTINGS frame with entries out of order.
@@ -3912,11 +3893,7 @@ TEST_P(SpdyFramerTest, SettingsFrameFlags) {
       EXPECT_CALL(visitor, OnError(_));
     } else {
       EXPECT_CALL(visitor, OnSettings());
-      if (GetSpdyRestartFlag(http2_propagate_unknown_settings)) {
-        EXPECT_CALL(visitor, OnSetting(SETTINGS_INITIAL_WINDOW_SIZE, 16));
-      } else {
-        EXPECT_CALL(visitor, OnSettingOld(SETTINGS_INITIAL_WINDOW_SIZE, 16));
-      }
+      EXPECT_CALL(visitor, OnSetting(SETTINGS_INITIAL_WINDOW_SIZE, 16));
       EXPECT_CALL(visitor, OnSettingsEnd());
     }
 

net/spdy/core/spdy_no_op_visitor.h

@@ -39,7 +39,6 @@ class SpdyNoOpVisitor : public SpdyFramerVisitorInterface,
   void OnStreamEnd(SpdyStreamId stream_id) override {}
   void OnStreamPadding(SpdyStreamId stream_id, size_t len) override {}
   void OnRstStream(SpdyStreamId stream_id, SpdyErrorCode error_code) override {}
-  void OnSettingOld(SpdyKnownSettingsId id, uint32_t value) override {}
   void OnSetting(SpdySettingsId id, uint32_t value) override {}
   void OnPing(SpdyPingId unique_id, bool is_ack) override {}
   void OnSettingsEnd() override {}

net/spdy/platform/impl/spdy_flags_impl.cc

@@ -6,9 +6,4 @@
 
 namespace net {
 
-// If true, Http2FrameDecoderAdapter will pass decoded HTTP/2 SETTINGS through
-// the SpdyFramerVisitorInterface callback OnSetting(), which will also accept
-// unknown SETTINGS IDs.
-bool http2_propagate_unknown_settings = true;
-
 }  // namespace net

net/spdy/platform/impl/spdy_flags_impl.h

@@ -9,8 +9,6 @@
 
 namespace net {
 
-NET_EXPORT_PRIVATE extern bool http2_propagate_unknown_settings;
-
 inline bool GetSpdyReloadableFlagImpl(bool flag) {
   return flag;
 }