[WebSocketExtensionParser] Have Consume.* methods return bool instead of using has_error()

net/websockets/websocket_extension_parser.h

 // Copyright 2013 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.
 
 #ifndef NET_WEBSOCKETS_WEBSOCKET_EXTENSION_PARSER_H_
 #define NET_WEBSOCKETS_WEBSOCKET_EXTENSION_PARSER_H_
 
 #include <string>
 #include <vector>
 
 #include "base/strings/string_piece.h"
 #include "net/base/net_export.h"
 #include "net/websockets/websocket_extension.h"
 
 namespace net {
 
 class NET_EXPORT_PRIVATE WebSocketExtensionParser {
  public:
   WebSocketExtensionParser();
   ~WebSocketExtensionParser();
 
   // Parses the given string as a Sec-WebSocket-Extensions header value.
   //
   // There must be no newline characters in the input. LWS-concatenation must
   // have already been done before calling this method.
-  void Parse(const char* data, size_t size);
-  void Parse(const std::string& data) {
-    Parse(data.data(), data.size());
+  //
+  // Returns true if the method was successful (no syntax error was found).
+  bool Parse(const char* data, size_t size);
+  bool Parse(const std::string& data) {
+    return Parse(data.data(), data.size());
   }
 
-  // Returns true if the last Parse() method call encountered any syntax error.
-  bool has_error() const { return has_error_; }
   // Returns the result of the last Parse() method call.
   const std::vector<WebSocketExtension>& extensions() const {
     return extensions_;
   }
 
  private:
   // TODO(tyoshino): Make the following methods return success/fail.

[eroman, 2015/04/06 19:07:52]

You can now remove this TODO

[tyoshino (SeeGerritForStatus), 2015/04/13 14:11:59]

Done.

-  void Consume(char c);
-  void ConsumeExtension(WebSocketExtension* extension);
-  void ConsumeExtensionParameter(WebSocketExtension::Parameter* parameter);
-  void ConsumeToken(base::StringPiece* token);
-  void ConsumeQuotedToken(std::string* token);
+  bool Consume(char c);

[eroman, 2015/04/06 19:07:52]

Something else you may consider is to add WARN_UNUSED_RESULT to all of these
methods.

This can be something you do temporarily to help ensure that the refactor
updates all callsites correctly, or something you leave in the code to make sure
future consumers test for the error.

This comment is optional and up to your judgement, but I have found it very
useful when testing my own code.

[tyoshino (SeeGerritForStatus), 2015/04/13 14:11:59]

Done.

+  bool ConsumeExtension(WebSocketExtension* extension);
+  bool ConsumeExtensionParameter(WebSocketExtension::Parameter* parameter);
+  bool ConsumeToken(base::StringPiece* token);
+  bool ConsumeQuotedToken(std::string* token);
   void ConsumeSpaces();
   bool Lookahead(char c);
   bool ConsumeIfMatch(char c);
   size_t UnconsumedBytes() const { return end_ - current_; }
 
   static bool IsControl(char c);
   static bool IsSeparator(char c);
 
   // The current position in the input string.
   const char* current_;
   // The pointer of the end of the input string.
   const char* end_;
-  bool has_error_;
   std::vector<WebSocketExtension> extensions_;
 
   DISALLOW_COPY_AND_ASSIGN(WebSocketExtensionParser);
 };
 
 }  // namespace net
 
 #endif  // NET_WEBSOCKETS_WEBSOCKET_EXTENSION_PARSER_H_