bindings: Make RecordMetchType take dictionary CookieStoreGetOptions

Makes it clear that |kUnspecified| means CookieStoreGetOptions.matchType is missing. Bug: 839389 Change-Id: I71a94e7b41a8bbfb12aa64ef9b0e5c0e0a844791 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210180 Reviewed-by: Ayu Ishii <ayui@chromium.org> Commit-Queue: Hitoshi Yoshida <peria@chromium.org> Cr-Commit-Position: refs/heads/master@{#777552}
Denger-Network · Jun 11, 2020 · a5b9427 · a5b9427
1 parent 6ffdc0b
commit a5b9427
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 11 deletions.
diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store.cc b/third_party/blink/renderer/modules/cookie_store/cookie_store.cc
@@ -256,7 +256,7 @@ ScriptPromise CookieStore::getAll(ScriptState* script_state,
                                   ExceptionState& exception_state) {
   UseCounter::Count(CurrentExecutionContext(script_state->GetIsolate()),
                     WebFeature::kCookieStoreAPI);
-  RecordMatchType(options->matchType());
+  RecordMatchType(*options);
 
   return DoRead(script_state, options, &CookieStore::GetAllForUrlToGetAllResult,
                 exception_state);
@@ -275,7 +275,7 @@ ScriptPromise CookieStore::get(ScriptState* script_state,
                                ExceptionState& exception_state) {
   UseCounter::Count(CurrentExecutionContext(script_state->GetIsolate()),
                     WebFeature::kCookieStoreAPI);
-  RecordMatchType(options->matchType());
+  RecordMatchType(*options);
 
   return DoRead(script_state, options, &CookieStore::GetAllForUrlToGetResult,
                 exception_state);

diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store_manager.cc b/third_party/blink/renderer/modules/cookie_store/cookie_store_manager.cc
@@ -121,7 +121,7 @@ ScriptPromise CookieStoreManager::subscribe(
   Vector<mojom::blink::CookieChangeSubscriptionPtr> backend_subscriptions;
   backend_subscriptions.ReserveInitialCapacity(subscriptions.size());
   for (const CookieStoreGetOptions* subscription : subscriptions) {
-    RecordMatchType(subscription->matchType());
+    RecordMatchType(*subscription);
     mojom::blink::CookieChangeSubscriptionPtr backend_subscription =
         ToBackendSubscription(default_cookie_url_, subscription,
                               exception_state);
@@ -147,7 +147,7 @@ ScriptPromise CookieStoreManager::unsubscribe(
   Vector<mojom::blink::CookieChangeSubscriptionPtr> backend_subscriptions;
   backend_subscriptions.ReserveInitialCapacity(subscriptions.size());
   for (const CookieStoreGetOptions* subscription : subscriptions) {
-    RecordMatchType(subscription->matchType());
+    RecordMatchType(*subscription);
     mojom::blink::CookieChangeSubscriptionPtr backend_subscription =
         ToBackendSubscription(default_cookie_url_, subscription,
                               exception_state);

diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store_metrics.cc b/third_party/blink/renderer/modules/cookie_store/cookie_store_metrics.cc
@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/modules/cookie_store/cookie_store_metrics.h"
 
 #include "base/metrics/histogram_macros.h"
+#include "third_party/blink/renderer/bindings/modules/v8/v8_cookie_store_get_options.h"
 
 namespace blink {
 
@@ -15,16 +16,22 @@ enum class MatchTypeOption {
   kUnspecified = 0,
   kEquals = 1,
   kStartsWith = 2,
-  kMaxValue = kStartsWith
+  kMaxValue = kStartsWith,
 };
 
-void RecordMatchType(const String& matchType) {
+void RecordMatchType(const CookieStoreGetOptions& options) {
   MatchTypeOption uma_match_type;
-  if (matchType.IsEmpty()) {
+  // TODO(crbug.com/1092328): Switch by V8CookieMatchType::Enum.
+  if (!options.hasMatchType()) {
     uma_match_type = MatchTypeOption::kUnspecified;
-  } else if (matchType == "starts-with") {
+  } else if (options.matchType() == "equals") {
+    uma_match_type = MatchTypeOption::kEquals;
+  } else if (options.matchType() == "starts-with") {
     uma_match_type = MatchTypeOption::kStartsWith;
   } else {
+    NOTREACHED();
+    // In case of an invalid value, we assume it's "equals" for the consistency
+    // of UMA.
     uma_match_type = MatchTypeOption::kEquals;
   }
   UMA_HISTOGRAM_ENUMERATION("Blink.CookieStore.MatchType", uma_match_type);

diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store_metrics.h b/third_party/blink/renderer/modules/cookie_store/cookie_store_metrics.h
@@ -5,12 +5,12 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_COOKIE_STORE_COOKIE_STORE_METRICS_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_COOKIE_STORE_COOKIE_STORE_METRICS_H_
 
-#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
-
 namespace blink {
 
+class CookieStoreGetOptions;
+
 // Record explicitly set MatchType option with UMA.
-void RecordMatchType(const String& matchType);
+void RecordMatchType(const CookieStoreGetOptions& options);
 
 }  // namespace blink