Move common PVer4 code into a V4ProtocolManagerUtil class.

components/safe_browsing_db/v4_protocol_manager_util.h

+// Copyright 2016 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 COMPONENTS_SAFE_BROWSING_DB_V4_PROTOCOL_MANAGER_UTIL_H_
+#define COMPONENTS_SAFE_BROWSING_DB_V4_PROTOCOL_MANAGER_UTIL_H_
+
+// A class that implements the stateless methods used by the GetHashUpdate and
+// GetFullHash stubby calls made by Chrome using the SafeBrowsing V4 protocol.
+
+#include <string>
+
+#include "base/gtest_prod_util.h"
+#include "net/url_request/url_request_status.h"
+#include "url/gurl.h"
+
+namespace {

[Nathan Parker, 2016/02/22 22:39:32]

Do you want this in an anonymous namespace in the .h file?  I guess that makes
it visible to anyone who includes the .h, but won't collide with any other
same-named types in other .h files.  Seems better to just put it in the SB
namespace, unless there's a specific reason to do it this way.

[vakh (use Gerrit instead), 2016/02/23 01:39:36]

Done.

+
+// Enumerate parsing failures for histogramming purposes.  DO NOT CHANGE
+// THE ORDERING OF THESE VALUES.
+enum ParseResultType {

[Nathan Parker, 2016/02/22 22:39:32]

Is this the union of Full-Hash checks and update checks?  

[vakh (use Gerrit instead), 2016/02/23 01:39:36]

At first I thought both modules could share this, but it looks like they'll have
pretty different parse errors so moving it to v4_get_hash_protocol_manager.cc in
anon namespace.

+  // Error parsing the protocol buffer from a string.
+  PARSE_FROM_STRING_ERROR = 0,
+
+  // A match in the response had an unexpected THREAT_ENTRY_TYPE.
+  UNEXPECTED_THREAT_ENTRY_TYPE_ERROR = 1,
+
+  // A match in the response had an unexpected THREAT_TYPE.
+  UNEXPECTED_THREAT_TYPE_ERROR = 2,
+
+  // A match in the response had an unexpected PLATFORM_TYPE.
+  UNEXPECTED_PLATFORM_TYPE_ERROR = 3,
+
+  // A match in the response contained no metadata where metadata was
+  // expected.
+  NO_METADATA_ERROR = 4,
+
+  // A match in the response contained a ThreatType that was inconsistent
+  // with the other matches.
+  INCONSISTENT_THREAT_TYPE_ERROR = 5,
+
+  // Memory space for histograms is determined by the max.  ALWAYS
+  // ADD NEW VALUES BEFORE THIS ONE.
+  PARSE_RESULT_TYPE_MAX = 6
+};
+
+}  // namespace
+
+namespace safe_browsing {
+
+// Safe Browsing V4 server URL prefix.
+extern const char kSbV4UrlPrefix[];

[Nathan Parker, 2016/02/22 22:39:32]

If this isn't needed by anyone other that the v4_protocol_manager_util.cc, then
you could just put it there.

[vakh (use Gerrit instead), 2016/02/23 01:39:36]

Done.

+
+// Config passed to the constructor of a V4 protocol manager.
+struct V4ProtocolConfig {
+  // The safe browsing client name sent in each request.
+  std::string client_name;
+
+  // Current product version sent in each request.
+  std::string version;
+
+  // The Google API key.
+  std::string key_param;
+};
+
+class V4ProtocolManagerUtil {
+ public:
+  ~V4ProtocolManagerUtil() {};

[Nathan Parker, 2016/02/22 22:39:32]

Probably don't need this.  And you could make the constructor private so noone
can instantiate it.

[vakh (use Gerrit instead), 2016/02/23 01:39:36]

Done.

+
+  // Record HTTP response code when there's no error in fetching an HTTP
+  // request, and the error code, when there is.
+  // |metric_name| is the name of the UMA metric to record the response code or
+  // error code against, |status| represents the status of the HTTP request, and
+  // |response code| represents the HTTP response code received from the server.
+  static void RecordHttpResponseOrErrorCode(const char* metric_name,
+                                            const net::URLRequestStatus& status,
+                                            int response_code);
+
+  // Generates a Pver4 request URL.
+  // |request_base64| is the serialized request protocol buffer encoded in
+  // base 64.
+  // |method_name| is the name of the method to call, as specified in the proto,
+  // |config| is an instance of V4ProtocolConfig that stores the client config.
+  static GURL GetRequestUrl(const std::string& request_base64,
+                            const std::string& method_name,
+                            const V4ProtocolConfig& config);
+
+  // Worker function for calculating the GetHash backoff times.
+  // |multiplier| is doubled for each consecutive error after the
+  // first, and |error_count| is incremented with each call.
+  static base::TimeDelta GetNextBackOffInterval(size_t* error_count,

[kcarattini, 2016/02/23 00:49:13]

If this is specific to GetHash then why put it in util? I thought it was, but if
not, please update the comment.

[vakh (use Gerrit instead), 2016/02/23 01:39:36]

Done. Updated the comment.
Backoff is used by both classes.

+                                                size_t* multiplier);
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4ProtocolManagerUtilTest,
+                           TestBackOffLogic);
+  FRIEND_TEST_ALL_PREFIXES(SafeBrowsingV4ProtocolManagerUtilTest,
+                           TestGetRequestUrl);
+
+  // Composes a URL using |prefix|, |method| (e.g.: encodedFullHashes).
+  // |request_base64|, |client_id|, |version| and |key_param|. |prefix|
+  // should contain the entire url prefix including scheme, host and path.
+  static std::string ComposeUrl(const std::string& prefix,
+                                const std::string& method,
+                                const std::string& request_base64,
+                                const std::string& client_id,
+                                const std::string& version,
+                                const std::string& key_param);
+
+  DISALLOW_COPY_AND_ASSIGN(V4ProtocolManagerUtil);
+};
+
+}  // namespace safe_browsing
+
+#endif  // COMPONENTS_SAFE_BROWSING_DB_V4_PROTOCOL_MANAGER_UTIL_H_