Cleanup TrialToken in preparation of moving it to content/common.

content/renderer/origin_trials/trial_token.cc

Index: content/renderer/origin_trials/trial_token.cc
diff --git a/content/renderer/origin_trials/trial_token.cc b/content/renderer/origin_trials/trial_token.cc
index 5cf69ac6edaaa30abe5326fdb596b38310892b87..0636c80a0b5f56545cff4c4084478e7f0837ece4 100644
--- a/content/renderer/origin_trials/trial_token.cc
+++ b/content/renderer/origin_trials/trial_token.cc
@@ -15,6 +15,8 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
+#include "content/public/common/content_client.h"
+#include "content/public/renderer/content_renderer_client.h"
 #include "url/origin.h"
 
 namespace content {
@@ -77,64 +79,60 @@ scoped_ptr<TrialToken> TrialToken::Parse(const std::string& token_text) {
   }
 
   // Ensure that the origin is a valid (non-unique) origin URL
-  GURL origin_url(origin_string);
-  if (url::Origin(origin_url).unique()) {
+  url::Origin origin = url::Origin(GURL(origin_string));
+  if (origin.unique()) {
     return nullptr;
   }
 
   // Signed data is (origin + "|" + feature_name + "|" + expiry).
   std::string data = token_contents.substr(signature.length() + 1);
 
-  return make_scoped_ptr(new TrialToken(version, signature, data, origin_url,
+  return make_scoped_ptr(new TrialToken(version, signature, data, origin,
                                         feature_name, expiry_timestamp));
 }
 
-TrialToken::TrialToken(uint8_t version,
-                       const std::string& signature,
-                       const std::string& data,
-                       const GURL& origin,
-                       const std::string& feature_name,
-                       uint64_t expiry_timestamp)
-    : version_(version),
-      signature_(signature),
-      data_(data),
-      origin_(origin),
-      feature_name_(feature_name),
-      expiry_timestamp_(expiry_timestamp) {}
+bool TrialToken::Verify(const url::Origin& origin,
+                        base::StringPiece feature_name) {
+  if (!IsAppropriate(origin, feature_name))
+    return false;
+  ContentClient* content_client = GetContentClient();

[iclelland, 2016/03/02 22:36:29]

Why is this logic moved here? The separation between the token and the validator
was originally so that the token could exist outside of any particular renderer,
but would be considered valid (or not) only within the context of a
ContentRenderClient, which the validator was bound to.

[Marijn Kruisselbrink, 2016/03/02 22:43:12]

I could have left this in part 2 of my change instead of course. But this had to
move somewhere other than TrialTokenValidator because TrialTokenValidator is
renderer only and the implementation of a blink interface. And the whole point
is that I'm trying to enable verification of tokens outside of the renderer.

If you'd rather keep it separate, I can rename the existing TrialTokenValidator
to WebTrialTokenValidatorImpl, and add a new TrialTokenValidator class that just
does the validation? (and I could even move this part of the CL to my other CL
where I make the actual move to content/common, if that makes you happier).

+  // TODO(iclelland): Allow for multiple signing keys, and iterate over all
+  // active keys here. https://crbug.com/543220
+  base::StringPiece public_key =
+      content_client->renderer()->GetOriginTrialPublicKey();
+  return IsValid(base::Time::Now(), public_key);
+}
 
-bool TrialToken::IsAppropriate(const std::string& origin,
-                               const std::string& feature_name) const {
+bool TrialToken::IsAppropriate(const url::Origin& origin,
+                               base::StringPiece feature_name) const {
   return ValidateOrigin(origin) && ValidateFeatureName(feature_name);
 }
 
 bool TrialToken::IsValid(const base::Time& now,
-                         const base::StringPiece& public_key) const {
-  // TODO(iclelland): Allow for multiple signing keys, and iterate over all
-  // active keys here. https://crbug.com/543220
+                         base::StringPiece public_key) const {
   return ValidateDate(now) && ValidateSignature(public_key);
 }
 
-bool TrialToken::ValidateOrigin(const std::string& origin) const {
-  return GURL(origin) == origin_;
+bool TrialToken::ValidateOrigin(const url::Origin& origin) const {
+  return origin == origin_;
 }
 
-bool TrialToken::ValidateFeatureName(const std::string& feature_name) const {
+bool TrialToken::ValidateFeatureName(base::StringPiece feature_name) const {
   return feature_name == feature_name_;
 }
 
 bool TrialToken::ValidateDate(const base::Time& now) const {
-  base::Time expiry_time = base::Time::FromDoubleT((double)expiry_timestamp_);
-  return expiry_time > now;
+  return expiry_time_ > now;
 }
 
-bool TrialToken::ValidateSignature(const base::StringPiece& public_key) const {
+bool TrialToken::ValidateSignature(base::StringPiece public_key) const {
   return ValidateSignature(signature_, data_, public_key);
 }
 
 // static
 bool TrialToken::ValidateSignature(const std::string& signature_text,
                                    const std::string& data,
-                                   const base::StringPiece& public_key) {
+                                   base::StringPiece public_key) {
   // Public key must be 32 bytes long for Ed25519.
   CHECK_EQ(public_key.length(), 32UL);
 
@@ -156,4 +154,17 @@ bool TrialToken::ValidateSignature(const std::string& signature_text,
   return (result != 0);
 }
 
+TrialToken::TrialToken(uint8_t version,
+                       const std::string& signature,
+                       const std::string& data,
+                       const url::Origin& origin,
+                       const std::string& feature_name,
+                       uint64_t expiry_timestamp)
+    : version_(version),
+      signature_(signature),
+      data_(data),
+      origin_(origin),
+      feature_name_(feature_name),
+      expiry_time_(base::Time::FromDoubleT(expiry_timestamp)) {}
+
 }  // namespace content