Introduce WebRequestResourceType.

extensions/browser/api/web_request/web_request_api_helpers.cc

 // Copyright (c) 2012 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 "extensions/browser/api/web_request/web_request_api_helpers.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 25 matching lines @@
 #include "net/log/net_log_event_type.h"
 #include "net/log/net_log_parameters_callback.h"
 #include "net/log/net_log_with_source.h"
 #include "net/url_request/url_request.h"
 #include "url/url_constants.h"
 
 // TODO(battre): move all static functions into an anonymous namespace at the
 // top of this file.
 
 using base::Time;
-using content::ResourceType;
+using extensions::WebRequestResourceType;
 using net::cookie_util::ParsedRequestCookie;
 using net::cookie_util::ParsedRequestCookies;
 
 namespace keys = extension_web_request_api_constants;
 
 namespace extension_web_request_api_helpers {
 
 namespace {
 
 // Multiple ResourceTypes may map to the same string, but the converse is not
 // possible.
 static const char* kResourceTypeStrings[] = {
-  "main_frame",
-  "sub_frame",
-  "stylesheet",
-  "script",
-  "image",
-  "font",
-  "object",
-  "script",
-  "script",
-  "image",
-  "xmlhttprequest",
-  "ping",
-  "script",
-  "object",
-  "other",
+    "main_frame",
+    "sub_frame",
+    "stylesheet",
+    "script",
+    "image",
+    "font",
+    "object",
+    "script",
+    "script",
+    "image",
+    "xmlhttprequest",
+    "ping",
+    "script",
+    "object",
+    "websocket",
+    "other",
 };
 
 const size_t kResourceTypeStringsLength = arraysize(kResourceTypeStrings);
 
-static ResourceType kResourceTypeValues[] = {
-  content::RESOURCE_TYPE_MAIN_FRAME,
-  content::RESOURCE_TYPE_SUB_FRAME,
-  content::RESOURCE_TYPE_STYLESHEET,
-  content::RESOURCE_TYPE_SCRIPT,
-  content::RESOURCE_TYPE_IMAGE,
-  content::RESOURCE_TYPE_FONT_RESOURCE,
-  content::RESOURCE_TYPE_OBJECT,
-  content::RESOURCE_TYPE_WORKER,
-  content::RESOURCE_TYPE_SHARED_WORKER,
-  content::RESOURCE_TYPE_FAVICON,
-  content::RESOURCE_TYPE_XHR,
-  content::RESOURCE_TYPE_PING,
-  content::RESOURCE_TYPE_SERVICE_WORKER,
-  content::RESOURCE_TYPE_PLUGIN_RESOURCE,
-  content::RESOURCE_TYPE_LAST_TYPE,  // represents "other"
+static WebRequestResourceType kResourceTypeValues[] = {
+    WebRequestResourceType::MAIN_FRAME,
+    WebRequestResourceType::SUB_FRAME,
+    WebRequestResourceType::STYLESHEET,
+    WebRequestResourceType::SCRIPT,
+    WebRequestResourceType::IMAGE,
+    WebRequestResourceType::FONT_RESOURCE,
+    WebRequestResourceType::OBJECT,
+    WebRequestResourceType::WORKER,
+    WebRequestResourceType::SHARED_WORKER,
+    WebRequestResourceType::FAVICON,
+    WebRequestResourceType::XHR,
+    WebRequestResourceType::PING,
+    WebRequestResourceType::SERVICE_WORKER,
+    WebRequestResourceType::PLUGIN_RESOURCE,
+    WebRequestResourceType::WEBSOCKET,
+    WebRequestResourceType::LAST_TYPE,  // Represents "other".

[Devlin, 2017/02/16 15:13:05]

if these aren't included in UMA anywhere, it could make sense to rename this to
be OTHER (since we're no longer strictly bound by content::ResourceType
definitions).

[pkalinnikov, 2017/02/16 15:52:45]

Done.

[tyoshino (SeeGerritForStatus), 2017/02/16 16:26:34]

sounds good

 };
 
 const size_t kResourceTypeValuesLength = arraysize(kResourceTypeValues);
 
 static_assert(kResourceTypeStringsLength == kResourceTypeValuesLength,
-              "Sizes of string lists and ResourceType lists should be equal");
+              "Sizes of string lists and ResourceType lists should be equal.");
 
-typedef std::vector<linked_ptr<net::ParsedCookie> > ParsedResponseCookies;
+using ParsedResponseCookies = std::vector<linked_ptr<net::ParsedCookie>>;
 
 void ClearCacheOnNavigationOnUI() {
   web_cache::WebCacheManager::GetInstance()->ClearCacheOnNavigation();
 }
 
 bool ParseCookieLifetime(net::ParsedCookie* cookie,
                          int64_t* seconds_till_expiry) {
   // 'Max-Age' is processed first because according to:
   // http://tools.ietf.org/html/rfc6265#section-5.3 'Max-Age' attribute
   // overrides 'Expires' attribute.
@@ skipping 1129 matching lines @@
   header->SetString(keys::kHeaderNameKey, name);
   if (base::IsStringUTF8(value)) {
     header->SetString(keys::kHeaderValueKey, value);
   } else {
     header->Set(keys::kHeaderBinaryValueKey,
                 StringToCharList(value));
   }
   return header;
 }
 
-bool IsRelevantResourceType(ResourceType type) {
-  ResourceType* iter =
-      std::find(kResourceTypeValues,
-                kResourceTypeValues + kResourceTypeValuesLength,
-                type);
-  return iter != (kResourceTypeValues + kResourceTypeValuesLength);
+bool IsRelevantResourceType(content::ResourceType type) {
+  auto* end = kResourceTypeValues + kResourceTypeValuesLength;
+  return std::find(kResourceTypeValues, end,
+                   extensions::ToWebRequestResourceType(type)) != end;
 }
 
-const char* ResourceTypeToString(ResourceType type) {
-  ResourceType* iter =
-      std::find(kResourceTypeValues,
-                kResourceTypeValues + kResourceTypeValuesLength,
-                type);
-  if (iter == (kResourceTypeValues + kResourceTypeValuesLength))
-    return "other";
-
-  return kResourceTypeStrings[iter - kResourceTypeValues];
+const char* ResourceTypeToString(WebRequestResourceType type) {
+  DCHECK_NE(WebRequestResourceType::WEBSOCKET, type);
+  auto* end = kResourceTypeValues + kResourceTypeValuesLength;
+  auto* iter = std::find(kResourceTypeValues, end, type);
+  return iter != end ? kResourceTypeStrings[iter - kResourceTypeValues]
+                     : "other";

[tyoshino (SeeGerritForStatus), 2017/02/16 16:13:58]

Not a change proposal. Let me explain my understanding. Please review it to
check if it's correct.

Maybe we have linear look up here not to depend on the assumption that the
integer values assigned to content::ResourceType starts from 0 and don't leap.

It's a bit unfortunate from performance perspective if we need this even after
we introduce the extensions directory's own enum. Because of that, I was about
to propose changing kResourceTypeStrings to list the string for all enum items
including ones correspond to "other" and was wondering if the negative impact to
ParseResourceType() by increasing the number of elements to scan would be
bigger.

But it looks ToWebRequestResourceType() is more performance sensitive and we
shouldn't introduce any look up to it, right?

WebRequestEventDetails() has both ResourceTypeToString() and
ToWebRequestResourceType().
WebRequestConditionAttributeResourceType::IsFulfilled() and
ExtensionWebRequestEventRouter::GetMatchingListeners() have only
ToWebRequestResourceType() and are performance sensitive. So, we shouldn't make
them slow.

Correct?

[pkalinnikov, 2017/02/16 19:27:30]

Please see the new change. Looks like it addressed this concern as well.

 }
 
 bool ParseResourceType(const std::string& type_str,
-                       std::vector<ResourceType>* types) {
+                       std::vector<WebRequestResourceType>* types) {
   bool found = false;
   for (size_t i = 0; i < kResourceTypeStringsLength; ++i) {
     if (type_str == kResourceTypeStrings[i]) {
+      DCHECK_NE(WebRequestResourceType::WEBSOCKET, kResourceTypeValues[i]);
+      types->push_back(kResourceTypeValues[i]);
       found = true;
-      types->push_back(kResourceTypeValues[i]);
     }
   }
   return found;
 }
 
 }  // namespace extension_web_request_api_helpers