Bug 1673642 - Implement C++ and JS APIs for the Boolean Metric Type. r=chutten

BONUS: Improve the docs for adding new metric type in C++ and JS,
added two missing steps to the C++ part and changed the layout
a bit to make it easier to follow.

Differential Revision: https://phabricator.services.mozilla.com/D95314

toolkit/components/glean/api/src/ffi/mod.rs

@@ -17,3 +17,16 @@ define_metric_ffi!(TIMESPAN_MAP {
     start -> fog_timespan_start(),
     stop -> fog_timespan_stop(),
 });
+
+define_metric_ffi!(BOOLEAN_MAP {
+    test_has -> fog_boolean_test_has_value,
+    test_get -> fog_boolean_test_get_value: u8,
+});
+
+#[no_mangle]
+pub extern "C" fn fog_boolean_set(id: u32, value: u8) {
+    match crate::metrics::__glean_metric_maps::BOOLEAN_MAP.get(&id.into()) {
+        Some(metric) => metric.set(value != 0),
+        None => panic!("No metric for id {}", id),
+    }
+}

toolkit/components/glean/bindings/MetricTypes.h

@@ -7,5 +7,6 @@
 
 #include "mozilla/glean/Counter.h"
 #include "mozilla/glean/Timespan.h"
+#include "mozilla/glean/Boolean.h"
 
 #endif  // mozilla_Glean_MetricTypes_h

toolkit/components/glean/bindings/private/Boolean.cpp

@@ -0,0 +1,40 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/glean/Boolean.h"
+
+#include "nsString.h"
+#include "mozilla/Components.h"
+#include "nsIClassInfoImpl.h"
+
+namespace mozilla {
+namespace glean {
+
+NS_IMPL_CLASSINFO(GleanBoolean, nullptr, 0, {0})
+NS_IMPL_ISUPPORTS_CI(GleanBoolean, nsIGleanBoolean)
+
+NS_IMETHODIMP
+GleanBoolean::Set(bool value, JSContext* cx) {
+  this->mBoolean.Set(value);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+GleanBoolean::TestHasValue(const nsACString& aStorageName, JSContext* cx,
+                           bool* result) {
+  *result = this->mBoolean.TestHasValue(PromiseFlatCString(aStorageName).get());
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+GleanBoolean::TestGetValue(const nsACString& aStorageName, JSContext* cx,
+                           bool* result) {
+  *result = this->mBoolean.TestGetValue(PromiseFlatCString(aStorageName).get());
+  return NS_OK;
+}
+
+}  // namespace glean
+}  // namespace mozilla

toolkit/components/glean/bindings/private/Boolean.h

@@ -0,0 +1,90 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_glean_GleanBoolean_h
+#define mozilla_glean_GleanBoolean_h
+
+#include "nsIGleanMetrics.h"
+
+namespace mozilla {
+namespace glean {
+
+namespace impl {
+extern "C" {
+void fog_boolean_set(uint32_t id, bool value);
+uint32_t fog_boolean_test_has_value(uint32_t id, const char* storageName);
+uint32_t fog_boolean_test_get_value(uint32_t id, const char* storageName);
+}
+
+class BooleanMetric {
+ public:
+  constexpr explicit BooleanMetric(uint32_t id) : mId(id) {}
+
+  /**
+   * Set to the specified boolean value.
+   *
+   * @param value the value to set.
+   */
+  void Set(bool value) const { fog_boolean_set(mId, int(value)); }
+
+  /**
+   * **Test-only API**
+   *
+   * Tests whether a value is stored for the metric.
+   *
+   * This function will attempt to await the last parent-process task (if any)
+   * writing to the the metric's storage engine before returning a value.
+   * This function will not wait for data from child processes.
+   *
+   * Parent process only. Panics in child processes.
+   *
+   * @param aStorageName the name of the ping to retrieve the metric for.
+   * @return true if metric value exists, otherwise false
+   */
+  bool TestHasValue(const char* aStorageName) const {
+    return fog_boolean_test_has_value(mId, aStorageName) != 0;
+  }
+
+  /**
+   * **Test-only API**
+   *
+   * Gets the currently stored value as a boolean.
+   *
+   * This function will attempt to await the last parent-process task (if any)
+   * writing to the the metric's storage engine before returning a value.
+   * This function will not wait for data from child processes.
+   *
+   * This doesn't clear the stored value.
+   * Parent process only. Panics in child processes.
+   *
+   * @return value of the stored metric.
+   */
+  bool TestGetValue(const char* aStorageName) const {
+    return fog_boolean_test_get_value(mId, aStorageName) != 0;
+  }
+
+ private:
+  const uint32_t mId;
+};
+}  // namespace impl
+
+class GleanBoolean final : public nsIGleanBoolean {
+ public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIGLEANBOOLEAN
+
+  explicit GleanBoolean(uint32_t id) : mBoolean(id){};
+
+ private:
+  virtual ~GleanBoolean() = default;
+
+  const impl::BooleanMetric mBoolean;
+};
+
+}  // namespace glean
+}  // namespace mozilla
+
+#endif /* mozilla_glean_GleanBoolean.h */

toolkit/components/glean/build_scripts/glean_parser_ext/util.py

@@ -29,7 +29,7 @@ def generate_metric_ids(objs):
     return lambda metric: metric_id_mapping[(metric.category, metric.name)]
 
 
-IMPLEMENTED_CPP_TYPES = ["counter", "timespan"]
+IMPLEMENTED_CPP_TYPES = ["counter", "timespan", "boolean"]
 
 
 def is_implemented_metric_type(typ):

toolkit/components/glean/docs/new_metric_types.md

@@ -81,43 +81,42 @@ We treat them both together since, though they're different languages,
 they're both implemented in C++ and share much of their implementation.
 
 Each metric type has six pieces you'll need to cover:
-1. **MLA FFI** - Using our convenient macros,
-   define the Multi-Language Architecture's FFI layer above the Rust API in
-   [`api/src/ffi/mod.rs`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/api/src/ffi/mod.rs).
-2. **C++ Impl** - Implement a type called `XMetric`
-   (e.g. `CounterMetric`) in `mozilla::glean::impl` in
-   [`bindings/private/`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/private/).
-   Its methods should be named the same as the ones in the Rust API,
-   transformed to `CamelCase`. They should all be public.
-3. **IDL** - Duplicate the public API (including its docs) to
-   [`xpcom/nsIGleanMetrics.idl`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/xpcom/nsIGleanMetrics.idl)
-   with the name `nsIGleanX` (e.g. `nsIGleanCounter`).
-   Inherit from `nsISupports`.
-   The naming style for members here is `lowerCamelCase`.
-   You'll need a
-   [GUID](https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Generating_GUIDs)
-   because this is XPCOM,
-   but you'll only need the canonical form since we're only exposing to JS.
-4. **JS Impl** - Add an `nsIGleanX`-deriving, `XMetric`-owning type called `GleanX`
-   (e.g. `GleanCounter`) in the same header and `.cpp` as `XMetric` in
-   [`bindings/private/`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/private/).
-   Don't declare any methods beyond a ctor
-   (takes a `uint32_t` metric id, init-constructs a `impl::XMetric` member)
-   and dtor (`default`): the IDL will do the rest so long as you remember to add
-   `NS_DECL_ISUPPORTS` and `NS_DECL_NSIGLEANX`.
-   In the definition of `GleanX`, member identifiers are back to
-   `CamelCase` and need macros like `NS_IMETHODIMP`.
-   Delegate operations to the owned `XMetric`, returning `NS_OK`
-   no matter what in non-test methods.
-   (Test-only methods can return `NS_ERROR` codes on failures).
-5. **Tests** - Two languages means two test suites.
-   Add a never-expiring test-only metric of your type to
-   [`test_metrics.yaml`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/test_metrics.yaml).
-   Feel free to be clever with the name, but be sure to make clear that it is test-only.
-    * **C++ Tests (GTest)** - Add a small test case to
-      [`gtest/TestFog.cpp`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/gtest/TestFog.cpp).
-      For more details, peruse the [testing docs](testing.md).
-    * **JS Tests (xpcshell)** - Add a small test case to
-      [`xpcshell/test_Glean.js`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/xpcshell/test_Glean.js).
-      For more details, peruse the [testing docs](testing.md).
-6. **API Documentation** - TODO
+
+### 1. MLA FFI
+
+- Using our convenient macros, define the Multi-Language Architecture's FFI layer above the Rust API in [`api/src/ffi/mod.rs`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/api/src/ffi/mod.rs).
+
+### 2. C++ Impl
+
+- Implement a type called `XMetric` (e.g. `CounterMetric`) in `mozilla::glean::impl` in [`bindings/private/`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/private/).
+    - Its methods should be named the same as the ones in the Rust API, transformed to `CamelCase`.
+    - They should all be public.
+- Include the new metric type in [`bindings/MetricTypes.h`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/MetricTypes.h)
+- Include the new files in [`moz.build`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/MetricTypes.h). The header file should be added to `EXPORTS.mozilla.glean` and the `.cpp` file should be added to `UNIFIED_SOURCES`.
+
+### 3. IDL
+
+- Duplicate the public API (including its docs) to [`xpcom/nsIGleanMetrics.idl`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/xpcom/nsIGleanMetrics.idl) with the name `nsIGleanX` (e.g. `nsIGleanCounter`).
+    - Inherit from `nsISupports`.
+    - The naming style for members here is `lowerCamelCase`. You'll need a [GUID](https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Generating_GUIDs) because this is XPCOM, but you'll only need the canonical form since we're only exposing to JS.
+
+### 4. JS Impl
+
+- Add an `nsIGleanX`-deriving, `XMetric`-owning type called `GleanX` (e.g. `GleanCounter`) in the same header and `.cpp` as `XMetric` in [`bindings/private/`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/bindings/private/).
+    - Don't declare any methods beyond a ctor (takes a `uint32_t` metric id, init-constructs a `impl::XMetric` member) and dtor (`default`): the IDL will do the rest so long as you remember to add `NS_DECL_ISUPPORTS` and `NS_DECL_NSIGLEANX`.
+    - In the definition of `GleanX`, member identifiers are back to `CamelCase` and need macros like `NS_IMETHODIMP`. Delegate operations to the owned `XMetric`, returning `NS_OK` no matter what in non-test methods. (Test-only methods can return `NS_ERROR` codes on failures).
+
+### 5. Tests
+
+Two languages means two test suites.
+
+- Add a never-expiring test-only metric of your type to [`test_metrics.yaml`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/test_metrics.yaml).
+    - Feel free to be clever with the name, but be sure to make clear that it is test-only.
+- **C++ Tests (GTest)** - Add a small test case to [`gtest/TestFog.cpp`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/gtest/TestFog.cpp).
+    - For more details, peruse the [testing docs](testing.md).
+- **JS Tests (xpcshell)** - Add a small test case to [`xpcshell/test_Glean.js`](https://hg.mozilla.org/mozilla-central/file/tip/toolkit/components/glean/xpcshell/test_Glean.js).
+    - For more details, peruse the [testing docs](testing.md).
+
+### 6. API Documentation
+
+TODO

toolkit/components/glean/gtest/TestFog.cpp

@@ -69,3 +69,13 @@ TEST(FOG, TestCppTimespanWorks)
   ASSERT_TRUE(
       mozilla::glean::test_only::can_we_time_it.TestGetValue("test-ping") > 0);
 }
+
+TEST(FOG, TestCppBooleanWorks)
+{
+  mozilla::glean::test_only::can_we_flag_it.Set(false);
+
+  ASSERT_TRUE(
+      mozilla::glean::test_only::can_we_flag_it.TestHasValue("test-ping"));
+  ASSERT_EQ(false, mozilla::glean::test_only::can_we_flag_it.TestGetValue(
+                       "test-ping"));
+}

toolkit/components/glean/moz.build

@@ -22,6 +22,7 @@ if CONFIG["MOZ_GLEAN"]:
         "bindings/Category.h",
         "bindings/Glean.h",
         "bindings/MetricTypes.h",
+        "bindings/private/Boolean.h",
         "bindings/private/Counter.h",
         "bindings/private/Timespan.h",
     ]
@@ -29,6 +30,7 @@ if CONFIG["MOZ_GLEAN"]:
     UNIFIED_SOURCES += [
         "bindings/Category.cpp",
         "bindings/Glean.cpp",
+        "bindings/private/Boolean.cpp",
         "bindings/private/Counter.cpp",
         "bindings/private/Timespan.cpp",
         "ipc/FOGIPC.cpp",

toolkit/components/glean/test_metrics.yaml

@@ -47,6 +47,26 @@ test_only:
     send_in_pings:
       - test-ping
 
+  can_we_flag_it:
+    type: boolean
+    description: |
+      Test metric for a boolean.
+      This is a test-only metric.
+    bugs:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1646165
+    data_reviews:
+      - https://bugzilla.mozilla.org/show_bug.cgi?id=1646165#c1
+    data_sensitivity:
+      - technical
+    notification_emails:
+      - glean-team@mozilla.com
+    expires: never
+    send_in_pings:
+      - store1
+    no_lint:
+      - COMMON_PREFIX
+      - test-ping
+
 test_only.ipc:
   a_counter:
     type: counter

toolkit/components/glean/xpcom/nsIGleanMetrics.idl

@@ -5,6 +5,52 @@
 
 #include "nsISupports.idl"
 
+[scriptable, uuid(d3180fe0-19fa-11eb-8b6f-0800200c9a66)]
+interface nsIGleanBoolean : nsISupports
+{
+  /**
+   * Set to the specified boolean value.
+   *
+   * @param value the value to set.
+   */
+  [implicit_jscontext]
+  void set(in bool value);
+
+  /**
+   * **Test-only API**
+   *
+   * Tests whether a value is stored for the metric.
+   *
+   * This function will attempt to await the last parent-process task (if any)
+   * writing to the the metric's storage engine before returning a value.
+   * This function will not wait for data from child processes.
+   *
+   * Parent process only. Panics in child processes.
+   *
+   * @param aStorageName the name of the ping to retrieve the metric for.
+   * @return true if metric value exists, otherwise false
+   */
+  [implicit_jscontext]
+  bool testHasValue(in ACString aStorageName);
+
+  /**
+   * **Test-only API**
+   *
+   * Gets the currently stored value as a boolean.
+   *
+   * This function will attempt to await the last parent-process task (if any)
+   * writing to the the metric's storage engine before returning a value.
+   * This function will not wait for data from child processes.
+   *
+   * This doesn't clear the stored value.
+   * Parent process only. Panics in child processes.
+   *
+   * @return value of the stored metric.
+   */
+  [implicit_jscontext]
+  bool testGetValue(in ACString aStorageName);
+};
+
 [scriptable, uuid(05b89d2a-d57c-11ea-82da-3f63399a6f5a)]
 interface nsIGleanCounter : nsISupports
 {

toolkit/components/glean/xpcshell/test_Glean.js

@@ -42,3 +42,9 @@ add_task(async function test_fog_timespan_works() {
   Assert.ok(Glean.test_only.can_we_time_it.testHasValue("test-ping"));
   Assert.ok(Glean.test_only.can_we_time_it.testGetValue("test-ping") > 0);
 });
+
+add_task(function test_fog_boolean_works() {
+  Glean.test_only.can_we_flag_it.set(false);
+  Assert.ok(Glean.test_only.can_we_flag_it.testHasValue("test-ping"));
+  Assert.equal(false, Glean.test_only.can_we_flag_it.testGetValue("test-ping"));
+});