Collect UMA data for Origin Trials

content/common/origin_trials/trial_token_status.h

+// Copyright 2016 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.
+
+#ifndef CONTENT_COMMON_ORIGIN_TRIALS_TRIAL_TOKEN_STATUS_H_
+#define CONTENT_COMMON_ORIGIN_TRIALS_TRIAL_TOKEN_STATUS_H_
+
+namespace content {
+
+enum TrialTokenStatus {

[Marijn Kruisselbrink, 2016/04/21 18:18:42]

Rather than having two enums and converting between them, you only need the
blink enum. Just move it to a separate header file, and add that file to the
long list of "strictly enum/POD header-only types" that are allowed to be
included in content/common/DEPS

[chasej, 2016/04/22 18:50:06]

I started with one enum. However, it seemed strange that TrialTokenValidator had
to reference a Blink enum when it really had no dependency on/knowledge of
Blink. IIRC, the intent was to have TrialTokenValidator be independent so it
could be used outside of the render process. I found this comment from an
earlier CL:
https://codereview.chromium.org/1752463002#msg9.

Converting between two enums isn't ideal, but it seem logical where it's
currently being done in WebTrialTokenValidatorImpl. More importantly, if the
TrialTokenValidator ends up being used from the browser/service workers, would
it still be valid/make sense to reference a Blink enum there? Of course, I could
use the Blink enum now, and defer that question.

What do you think?

[Marijn Kruisselbrink, 2016/04/22 21:51:06]

I agree that both options are not ideal. Not having duplicate enums seems nice,
and there are plenty of enums already that are defined in blink an used at all
layers in content. But I agree that usually is because it's a value that
eventually would be passed to/from blink anyway, while for something like
https://codereview.chromium.org/1763943002 (which I wasn't planning on
landing/cleaning up until there is a need for it) it wouldn't ever end up in
blink code.

I don't feel particularly strongly either way about this, but at least from a
policy/layering/technical point of view there is no reason not to use the same
enum in blink, content/renderer, content/common and possibly content/browser (as
long as the enum is defined in its own header file, allowing DEPS files to be
configured to only allow access to the enum and not any other blink interfaces).

[chasej, 2016/04/25 20:32:52]

I've changed to share a single WebOriginTrialTokenStatus enum, defined in a
separate file. I chose to add "Origin" to the name, to be more explicit. As this
will live in public/platform, it doesn't have the a parent origin_trials folder
to add context. Arguably, TrialToken and TrialTokenValidator should have
"Origin" in the name as well, but I don't believe it's worth renaming existing
classes at this point.

+  TRIAL_TOKEN_STATUS_SUCCESS = 0,
+  TRIAL_TOKEN_STATUS_EXPIRED,
+  TRIAL_TOKEN_STATUS_INVALID_SIGNATURE,
+  TRIAL_TOKEN_STATUS_MALFORMED,
+  // Embedder does not support origin trials
+  TRIAL_TOKEN_STATUS_NOT_SUPPORTED,
+  TRIAL_TOKEN_STATUS_WRONG_FEATURE,
+  TRIAL_TOKEN_STATUS_WRONG_ORIGIN,
+  TRIAL_TOKEN_STATUS_LAST = TRIAL_TOKEN_STATUS_WRONG_ORIGIN
+};
+
+}  // namespace content
+
+#endif  // CONTENT_COMMON_ORIGIN_TRIALS_TRIAL_TOKEN_STATUS_H_