Add Platform Independent Eddystone Encoder

components/physical_web/eddystone/eddystone_encoder.cc

+// Copyright 2017 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.
+
+#include "components/physical_web/eddystone/eddystone_encoder.h"
+
+#include <string>
+#include <vector>
+
+#include "url/gurl.h"
+
+namespace {
+const char* kPrefixes[] = {"http://www.", "https://www.", "http://",
+                           "https://"};
+const size_t kNumPrefixes = sizeof(kPrefixes) / sizeof(char*);
+
+const char* kSuffixes[] = {".com/", ".org/", ".edu/", ".net/", ".info/",
+                           ".biz/", ".gov/", ".com",  ".org",  ".edu",
+                           ".net",  ".info", ".biz",  ".gov"};
+const size_t kNumSuffixes = sizeof(kSuffixes) / sizeof(char*);
+}
+
+namespace physical_web {
+std::vector<uint8_t> EncodeUrl(std::string c_url) {

[mattreynolds, 2017/03/08 19:14:46]

Use a const reference for the input parameter to avoid a copy:

const std::string& c_url

[iankc, 2017/03/09 20:45:44]

Done.

+  std::vector<uint8_t> encodedUrl;

[mattreynolds, 2017/03/08 19:14:46]

Use snake_case for common variables

[iankc, 2017/03/09 20:45:45]

Done.

+
+  if (c_url.empty()) {
+    encodedUrl.push_back(-1);

[mattreynolds, 2017/03/08 19:14:46]

Returning errors by setting invalid values in the returned array is strange and
makes extra work for callers. Can we change it to make it easier to check for
errors? Maybe something like:

int EncodeUrl(const std::string& url, std::vector<uint8_t>* encoded_url)

(In Google C++ style out params are always pointers)

[iankc, 2017/03/09 20:45:44]

Done.

+    return encodedUrl;
+  }
+
+  GURL gurl(c_url);
+  if (!gurl.is_valid()) {
+    encodedUrl.push_back(-2);
+    return encodedUrl;
+  }
+
+  if (gurl.HostIsIPAddress() || !gurl.SchemeIsHTTPOrHTTPS()) {
+    encodedUrl.push_back(-3);
+    return encodedUrl;
+  }
+
+  /*
+   * Necessary Declarations
+   */
+  const char* raw_url_position = c_url.c_str();
+
+  size_t prefixLengths[kNumPrefixes] = {};

[mattreynolds, 2017/03/08 19:14:46]

Can you make this static?  Then we can compute it once on the first call and
avoid recomputing it for future calls.

Also, the initializer is unnecessary since you're setting the contents below.

[iankc, 2017/03/09 20:45:45]

Done.

+  // Compute once because strlen is expensive.
+  for (size_t i = 0; i < kNumPrefixes; i++) {
+    prefixLengths[i] = strlen(kPrefixes[i]);
+  }
+
+  size_t suffixLengths[kNumSuffixes] = {};
+  for (size_t i = 0; i < kNumSuffixes; i++) {
+    suffixLengths[i] = strlen(kSuffixes[i]);
+  }
+  /*
+   * handle prefix
+   */
+  for (size_t i = 0; i < kNumPrefixes; i++) {
+    size_t prefixLen = prefixLengths[i];
+    if (strncmp(raw_url_position, kPrefixes[i], prefixLen) == 0) {
+      encodedUrl.push_back(i);
+      raw_url_position += prefixLen;
+      break;
+    }
+  }
+
+  /*
+   * handle suffixes
+   */
+  while (*raw_url_position) {
+    /* check for suffix match */
+    size_t i;
+    for (i = 0; i < kNumSuffixes; i++) {
+      size_t suffixLen = suffixLengths[i];
+      if (strncmp(raw_url_position, kSuffixes[i], suffixLen) == 0) {
+        encodedUrl.push_back(i);
+        raw_url_position += suffixLen;
+        break; /* from the for loop for checking against suffixes */
+      }
+    }
+    /* This is the default case where we've got an ordinary character which
+     * doesn't match a suffix. */
+    if (i == kNumSuffixes) {
+      encodedUrl.push_back(*raw_url_position++);
+    }
+  }
+
+  return encodedUrl;
+}
+}