Introduce WebRequestResourceType.

extensions/browser/api/web_request/web_request_api_helpers.cc

Index: extensions/browser/api/web_request/web_request_api_helpers.cc
diff --git a/extensions/browser/api/web_request/web_request_api_helpers.cc b/extensions/browser/api/web_request/web_request_api_helpers.cc
index bdd91f1dfe04310d620abafce152939a6acd4dcb..527931d89b3740121545f80d39110b4f7068dd47 100644
--- a/extensions/browser/api/web_request/web_request_api_helpers.cc
+++ b/extensions/browser/api/web_request/web_request_api_helpers.cc
@@ -43,7 +43,7 @@
 // top of this file.
 
 using base::Time;
-using content::ResourceType;
+using extensions::WebRequestResourceType;
 using net::cookie_util::ParsedRequestCookie;
 using net::cookie_util::ParsedRequestCookies;
 
@@ -56,49 +56,51 @@ 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();
@@ -1248,32 +1250,28 @@ std::unique_ptr<base::DictionaryValue> CreateHeaderDictionary(
   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]) {
-      found = true;
+      DCHECK_NE(WebRequestResourceType::WEBSOCKET, kResourceTypeValues[i]);
       types->push_back(kResourceTypeValues[i]);
+      found = true;
     }
   }
   return found;