UMA for Notification action buttons

third_party/WebKit/Source/modules/notifications/ServiceWorkerRegistrationNotifications.cpp

 // Copyright 2014 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 "config.h"
 #include "modules/notifications/ServiceWorkerRegistrationNotifications.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
@@ skipping 42 matching lines @@
 
     // If permission for notification's origin is not "granted", reject promise with a TypeError exception, and terminate these substeps.
     if (Notification::checkPermission(executionContext) != WebNotificationPermissionAllowed)
         return ScriptPromise::reject(scriptState, V8ThrowException::createTypeError(scriptState->isolate(), "No notification permission has been granted for this origin."));
 
     // Validate the developer-provided values to get a WebNotificationData object.
     WebNotificationData data = createWebNotificationData(executionContext, title, options, exceptionState);
     if (exceptionState.hadException())
         return exceptionState.reject(scriptState);
 
+    // Log number of actions developer provided, even if it's more than we currently support.
+    Platform::current()->histogramCustomCounts("Notifications.PersistentNotificationActionCount", options.actions().size(), 0, 10, 12 /* one extra bucket for > 10 */);

[Alexei Svitkine (slow), 2015/10/28 19:09:09]

If you want a single bucket for each distinct counts (i.e. each bucket has a
range of 1 value), suggest using  histogramEnumeration(),

[johnme, 2015/10/29 14:24:41]

We don't strictly care about having distinct buckets, especially for larger
numbers of buttons (in practice it'd be useful for 0, 1 and 2 to have distinct
buckets). Converting the count to an enum feels rather hacky. So keeping it as a
histogram seems reasonable?

[Alexei Svitkine (slow), 2015/10/29 19:24:05]

Well, the way you're using counts() right now is equivalent to the enumeration
function. (Other than a few errors you have such as specifying a min 0 instead
of 1, since the API already gives you an underflow bucket from 0 to min value).
So it would be simpler and less error-prone to call that function.

On the other hand, if you don't mind having broader ranges at the high end, you
can actually specify a higher max value than 10 and continue using Counts().

I am ok with either approach, but your current one can be simplified using the
other function.

[johnme, 2015/11/23 15:06:09]

Gotcha. I've switched to histogramEnumeration/LinearHistogram as you suggested
since we care more about exact counts for small values than we care about the
order of magnitude for excessively large values provided (even though those are
possible, since this is developer-supplied input).

It feels a little dirty that we'll be deliberately making use of the underflow
bucket to pass in zeroes, but I guess that's normal?

+
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     WebNotificationShowCallbacks* callbacks = new CallbackPromiseAdapter<void, void>(resolver);
 
     SecurityOrigin* origin = executionContext->securityOrigin();
     WebNotificationManager* notificationManager = Platform::current()->notificationManager();
     ASSERT(notificationManager);
 
     notificationManager->showPersistent(WebSecurityOrigin(origin), data, serviceWorkerRegistration.webRegistration(), callbacks);
     return promise;
 }
 
 ScriptPromise ServiceWorkerRegistrationNotifications::getNotifications(ScriptState* scriptState, ServiceWorkerRegistration& serviceWorkerRegistration, const GetNotificationOptions& options)
 {
     ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
     ScriptPromise promise = resolver->promise();
 
     WebNotificationGetCallbacks* callbacks = new CallbackPromiseAdapter<NotificationArray, void>(resolver);
 
     WebNotificationManager* notificationManager = Platform::current()->notificationManager();
     ASSERT(notificationManager);
 
     notificationManager->getNotifications(options.tag(), serviceWorkerRegistration.webRegistration(), callbacks);
     return promise;
 }
 
 } // namespace blink