Backed out 2 changesets (bug 1685402) for build bustages in toolkit/components/glean/bindings/private/Ping.cpp. CLOSED TREE

Backed out changeset f9eb32e83608 (bug 1685402)
Backed out changeset 00dc350a9371 (bug 1685402)

toolkit/components/glean/api/src/private/ping.rs

@@ -42,23 +42,6 @@ impl Ping {
             ))
         }
     }
-
-    /// **Test-only API**
-    ///
-    /// Attach a callback to be called right before a new ping is submitted.
-    /// The provided function is called exactly once before submitting a ping.
-    ///
-    /// Note: The callback will be called on any call to submit.
-    /// A ping might not be sent afterwards, e.g. if the ping is otherwise empty (and
-    /// `send_if_empty` is `false`).
-    pub fn test_before_next_submit(&self, cb: impl FnOnce(Option<&str>) + Send + 'static) {
-        match self {
-            Ping::Parent(p) => p.test_before_next_submit(cb),
-            Ping::Child => {
-                panic!("Cannot use ping test API from non-parent process!");
-            }
-        };
-    }
 }
 
 #[inherent(pub)]
@@ -89,11 +72,6 @@ mod test {
     use super::*;
     use crate::common_test::*;
 
-    use std::sync::{
-        atomic::{AtomicBool, Ordering},
-        Arc,
-    };
-
     // Smoke test for what should be the generated code.
     static PROTOTYPE_PING: Lazy<Ping> = Lazy::new(|| Ping::new("prototype", false, true, vec![]));
 
@@ -101,13 +79,8 @@ mod test {
     fn smoke_test_custom_ping() {
         let _lock = lock_test();
 
-        let called = Arc::new(AtomicBool::new(false));
+        // We can only check that nothing explodes.
-        let rcalled = Arc::clone(&called);
+        // More comprehensive tests are blocked on bug 1673660.
-        PROTOTYPE_PING.test_before_next_submit(move |reason| {
-            (*rcalled).store(true, Ordering::Relaxed);
-            assert_eq!(None, reason);
-        });
         PROTOTYPE_PING.submit(None);
-        assert!((*called).load(Ordering::Relaxed));
     }
 }

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

@@ -12,47 +12,6 @@
 
 namespace mozilla::glean {
 
-namespace impl {
-
-using CallbackMapType = nsTHashMap<uint32_t, PingTestCallback>;
-using MetricIdToCallbackMutex = StaticDataMutex<UniquePtr<CallbackMapType>>;
-static MetricIdToCallbackMutex::AutoLock GetCallbackMapLock() {
-  static MetricIdToCallbackMutex sCallbacks("sCallbacks");
-  auto lock = sCallbacks.Lock();
-  if (!*lock) {
-    *lock = MakeUnique<CallbackMapType>();
-  }
-  return lock;
-}
-
-void Ping::Submit(const nsACString& aReason) const {
-#ifdef MOZ_GLEAN_ANDROID
-  Unused << mId;
-#else
-  {
-    auto lock = GetCallbackMapLock();
-    auto callback = lock.ref()->Extract(mId);
-    if (callback) {
-      callback.extract()(aReason);
-    }
-  }
-  fog_submit_ping_by_id(mId, &aReason);
-#endif
-}
-
-void Ping::TestBeforeNextSubmit(PingTestCallback&& aCallback) const {
-#ifdef MOZ_GLEAN_ANDROID
-  return;
-#else
-  {
-    auto lock = GetCallbackMapLock();
-    lock.ref()->InsertOrUpdate(mId, aCallback);
-  }
-#endif
-}
-
-}  // namespace impl
-
 NS_IMPL_CLASSINFO(GleanPing, nullptr, 0, {0})
 NS_IMPL_ISUPPORTS_CI(GleanPing, nsIGleanPing)
 
@@ -62,16 +21,4 @@ GleanPing::Submit(const nsACString& aReason) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-GleanPing::TestBeforeNextSubmit(nsIGleanPingTestCallback* aCallback) {
-  if (NS_WARN_IF(!aCallback)) {
-    return NS_ERROR_INVALID_ARG;
-  }
-  // Throw the bare ptr into a COM ptr to keep it around in the lambda.
-  nsCOMPtr<nsIGleanPingTestCallback> callback = aCallback;
-  mPing.TestBeforeNextSubmit(
-      [callback](const nsACString& aReason) { callback->Call(aReason); });
-  return NS_OK;
-}
-
 }  // namespace mozilla::glean

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

@@ -7,7 +7,6 @@
 #ifndef mozilla_glean_Ping_h
 #define mozilla_glean_Ping_h
 
-#include "mozilla/DataMutex.h"
 #include "mozilla/glean/fog_ffi_generated.h"
 #include "mozilla/Maybe.h"
 #include "nsIGleanMetrics.h"
@@ -15,8 +14,6 @@
 
 namespace mozilla::glean {
 
-typedef std::function<void(const nsACString& aReason)> PingTestCallback;
-
 namespace impl {
 
 class Ping {
@@ -43,21 +40,13 @@ class Ping {
    * @param aReason - Optional. The reason the ping is being submitted.
    *                  Must match one of the configured `reason_codes`.
    */
-  void Submit(const nsACString& aReason = nsCString()) const;
+  void Submit(const nsACString& aReason = nsCString()) const {
-
+#ifdef MOZ_GLEAN_ANDROID
-  /**
+    Unused << mId;
-   * **Test-only API**
+#else
-   *
+    fog_submit_ping_by_id(mId, &aReason);
-   * Register a callback to be called right before this ping is next submitted.
+#endif
-   * The provided function is called exactly once before submitting.
+  }
-   *
-   * Note: The callback will be called on any call to submit.
-   * A ping may not be sent afterwards, e.g. if the ping is empty and
-   * `send_if_empty` is `false`
-   *
-   * @param aCallback - The callback to call on the next submit.
-   */
-  void TestBeforeNextSubmit(PingTestCallback&& aCallback) const;
 
  private:
   const uint32_t mId;

toolkit/components/glean/gtest/TestFog.cpp

@@ -169,16 +169,11 @@ TEST(FOG, TestCppMemoryDistWorks)
 
 TEST(FOG, TestCppPings)
 {
-  test_only::one_ping_one_bool.Set(false);
+  auto ping = mozilla::glean_pings::OnePingOnly;
-  const auto& ping = mozilla::glean_pings::OnePingOnly;
+  mozilla::Unused << ping;
-  bool submitted = false;
+  // That's it. That's the test. It will fail to compile if it's missing.
-  ping.TestBeforeNextSubmit([&submitted](const nsACString& aReason) {
+  // For a test that actually submits the ping, we have integration tests.
-    submitted = true;
+  // See also bug 1681742.
-    ASSERT_EQ(false, test_only::one_ping_one_bool.TestGetValue().ref());
-  });
-  ping.Submit();
-  ASSERT_TRUE(submitted)
-  << "Must have actually called the lambda.";
 }
 
 TEST(FOG, TestCppStringLists)

toolkit/components/glean/test_metrics.yaml

@@ -272,23 +272,6 @@ test_only:
       - test-ping
     telemetry_mirror: TELEMETRY_TEST_MIRROR_FOR_LABELED_BOOL
 
-  one_ping_one_bool:
-    type: boolean
-    description: |
-      One bool for one ping only.
-      This is a test-only metric.
-    bugs:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1685402
-    data_reviews:
-      - https://bugzilla.mozilla.org/show_bug.cgi?id=1685402#c1
-    data_sensitivity:
-      - technical
-    expires: never
-    notification_emails:
-      - glean-team@mozilla.com
-    send_in_pings:
-      - one-ping-only
-
 
 test_only.ipc:
   a_counter:

toolkit/components/glean/xpcom/nsIGleanMetrics.idl

@@ -184,12 +184,6 @@ interface nsIGleanMemoryDistribution : nsISupports
 	jsval testGetValue([optional] in ACString aPingName);
 };
 
-[scriptable, function, uuid(e5447f62-4b03-497c-81e9-6ab683d20380)]
-interface nsIGleanPingTestCallback : nsISupports
-{
-  void call(in ACString aReason);
-};
-
 [scriptable, uuid(5223a48b-687d-47ff-a629-fd4a72d1ecfa)]
 interface nsIGleanPing : nsISupports
 {
@@ -213,20 +207,6 @@ interface nsIGleanPing : nsISupports
    *                  Must match one of the configured `reason_codes`.
    */
 	void submit([optional] in ACString aReason);
-
-  /**
-   * **Test-only API**
-   *
-   * Register a callback to be called right before this ping is next submitted.
-   * The provided function is called exactly once before submitting.
-   *
-   * Note: The callback will be called on any call to submit.
-   * A ping might not be sent afterwards, e.g. if the ping is empty and
-   * `send_if_empty` is `false`.
-   *
-   * @param aCallback - The callback to call on the next submit.
-   */
-  void testBeforeNextSubmit(in nsIGleanPingTestCallback aCallback);
 };
 
 [scriptable, uuid(d84a3555-46f1-48c1-9122-e8e88b069d2b)]

toolkit/components/glean/xpcshell/test_Glean.js

@@ -186,14 +186,8 @@ add_task(async function test_fog_memory_distribution_works() {
 
 add_task(function test_fog_custom_pings() {
   Assert.ok("onePingOnly" in GleanPings);
-  let submitted = false;
+  // Don't bother sending it, we'll test that in the integration suite.
-  Glean.testOnly.onePingOneBool.set(false);
+  // See also bug 1681742.
-  GleanPings.onePingOnly.testBeforeNextSubmit(reason => {
-    submitted = true;
-    Assert.equal(false, Glean.testOnly.onePingOneBool.testGetValue());
-  });
-  GleanPings.onePingOnly.submit();
-  Assert.ok(submitted, "Ping was submitted, callback was called.");
 });
 
 add_task(async function test_fog_timing_distribution_works() {