Support parsing a Sec-WebSocket-Extensions header with multiple elements

net/websockets/websocket_extension_parser.cc

Index: net/websockets/websocket_extension_parser.cc
diff --git a/net/websockets/websocket_extension_parser.cc b/net/websockets/websocket_extension_parser.cc
index 28a2db16f2775e3410a47e35d51ea12792f63682..91e32a1305cbefaf0efe11059f04a8e24dc7da87 100644
--- a/net/websockets/websocket_extension_parser.cc
+++ b/net/websockets/websocket_extension_parser.cc
@@ -16,11 +16,28 @@ void WebSocketExtensionParser::Parse(const char* data, size_t size) {
   current_ = data;
   end_ = data + size;
   has_error_ = false;
+  extensions_.clear();
+
+  while (true) {
+    WebSocketExtension extension;
+    ConsumeExtension(&extension);
+    extensions_.push_back(extension);
+    if (has_error_) {

[eroman, 2015/03/31 21:11:12]

Should this be done before the call to extensions_push()?

As an aside, while reading this I find the has_error_ member to be a bit
fragile. I wonder if it would be clearer to have each of the "Consume*()"
methods instead return their success as a boolean, and mark the return value as
something that cannot be ignored to the compiler.

[tyoshino (SeeGerritForStatus), 2015/04/01 04:57:57]

Done.
Will do. Added TODO

+      extensions_.clear();
+      return;

[eroman, 2015/03/31 21:11:12]

I suggest doing a break instead, and leaving the check at the bottom to clear
extensions_ (so it is just done in one place).

[tyoshino (SeeGerritForStatus), 2015/04/01 04:57:57]

Done.

+    }
 
-  ConsumeExtension(&extension_);
-  if (has_error_) return;
-  ConsumeSpaces();
-  has_error_ = has_error_ || (current_ != end_);
+    ConsumeSpaces();
+    DCHECK(!has_error_);
+
+    if (!ConsumeIfMatch(',')) {
+      break;
+    }
+  }
+
+  has_error_ = has_error_ || current_ != end_;
+  if (has_error_)
+    extensions_.clear();
 }
 
 void WebSocketExtensionParser::Consume(char c) {