author | Chris H-C <chutten@mozilla.com> |
Thu, 17 Dec 2020 12:42:26 +0000 | |
changeset 561103 | c2a095ef323d631ead11cf5bc906b9208d5be1c4 |
parent 561102 | 60e886fcc3db9f0ed4e173732653ef00ccead3c3 |
child 561104 | b2cf325325e1ae08de0e387015ae2b0ac02a18a8 |
push id | 38040 |
push user | csabou@mozilla.com |
push date | Thu, 17 Dec 2020 21:49:27 +0000 |
treeherder | mozilla-central@a08abfa374b6 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | janerik |
bugs | 1682960 |
milestone | 86.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/toolkit/components/glean/api/src/ffi/macros.rs +++ b/toolkit/components/glean/api/src/ffi/macros.rs @@ -26,30 +26,36 @@ macro_rules! metric_get { /// /// * `$map` - The name of the hash map within `metrics::__glean_metric_maps` /// as generated by glean_parser. /// * `$id` - The ID of the metric to get. /// * `$storage` - the storage name to look into. macro_rules! test_has { ($map:ident, $id:ident, $storage:ident) => {{ let metric = metric_get!($map, $id); - let storage = $storage.to_utf8(); - let storage = Some(&storage[..]); - metric.test_get_value(storage).is_some() + let storage = if $storage.is_empty() { + None + } else { + Some($storage.to_utf8()) + }; + metric.test_get_value(storage.as_deref()).is_some() }}; } /// Get the currently stored value for the metric identified by its ID. /// /// # Arguments /// /// * `$map` - The name of the hash map within `metrics::__glean_metric_maps` /// as generated by glean_parser. /// * `$id` - The ID of the metric to get. /// * `$storage` - the storage name to look into. macro_rules! test_get { ($map:ident, $id:ident, $storage:ident) => {{ let metric = metric_get!($map, $id); - let storage = $storage.to_utf8(); - let storage = Some(&storage[..]); - metric.test_get_value(storage).unwrap() + let storage = if $storage.is_empty() { + None + } else { + Some($storage.to_utf8()) + }; + metric.test_get_value(storage.as_deref()).unwrap() }}; }
--- a/toolkit/components/glean/bindings/private/Boolean.h +++ b/toolkit/components/glean/bindings/private/Boolean.h @@ -33,23 +33,26 @@ class BooleanMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric. */ - Maybe<bool> TestGetValue(const nsACString& aStorageName) const { - if (!fog_boolean_test_has_value(mId, &aStorageName)) { + Maybe<bool> TestGetValue(const nsACString& aPingName = nsCString()) const { + if (!fog_boolean_test_has_value(mId, &aPingName)) { return Nothing(); } - return Some(fog_boolean_test_get_value(mId, &aStorageName)); + return Some(fog_boolean_test_get_value(mId, &aPingName)); } private: const uint32_t mId; }; } // namespace impl class GleanBoolean final : public nsIGleanBoolean {
--- a/toolkit/components/glean/bindings/private/Counter.h +++ b/toolkit/components/glean/bindings/private/Counter.h @@ -33,23 +33,26 @@ class CounterMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ - Maybe<int32_t> TestGetValue(const nsACString& aStorageName) const { - if (!fog_counter_test_has_value(mId, &aStorageName)) { + Maybe<int32_t> TestGetValue(const nsACString& aPingName = nsCString()) const { + if (!fog_counter_test_has_value(mId, &aPingName)) { return Nothing(); } - return Some(fog_counter_test_get_value(mId, &aStorageName)); + return Some(fog_counter_test_get_value(mId, &aPingName)); } private: const uint32_t mId; }; } // namespace impl class GleanCounter final : public nsIGleanCounter {
--- a/toolkit/components/glean/bindings/private/Datetime.h +++ b/toolkit/components/glean/bindings/private/Datetime.h @@ -48,24 +48,28 @@ class DatetimeMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ - Maybe<nsCString> TestGetValue(const nsACString& aStorageName) const { - if (!fog_datetime_test_has_value(mId, &aStorageName)) { + Maybe<nsCString> TestGetValue( + const nsACString& aPingName = nsCString()) const { + if (!fog_datetime_test_has_value(mId, &aPingName)) { return Nothing(); } nsCString ret; - fog_datetime_test_get_value(mId, &aStorageName, &ret); + fog_datetime_test_get_value(mId, &aPingName, &ret); return Some(ret); } private: const uint32_t mId; }; } // namespace impl
--- a/toolkit/components/glean/bindings/private/Event.h +++ b/toolkit/components/glean/bindings/private/Event.h @@ -68,21 +68,24 @@ class EventMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ Maybe<nsTArray<RecordedEvent>> TestGetValue( - const nsACString& aStorageName) const { - if (!fog_event_test_has_value(mId, &aStorageName)) { + const nsACString& aPingName = nsCString()) const { + if (!fog_event_test_has_value(mId, &aPingName)) { return Nothing(); } // TODO(bug 1678567): Implement this. nsTArray<RecordedEvent> empty; return Some(std::move(empty)); }
--- a/toolkit/components/glean/bindings/private/MemoryDistribution.h +++ b/toolkit/components/glean/bindings/private/MemoryDistribution.h @@ -46,19 +46,23 @@ class MemoryDistributionMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ - Maybe<DistributionData> TestGetValue(const nsACString& aPingName) const { + Maybe<DistributionData> TestGetValue( + const nsACString& aPingName = nsCString()) const { if (!fog_memory_distribution_test_has_value(mId, &aPingName)) { return Nothing(); } nsTArray<uint64_t> buckets; nsTArray<uint64_t> counts; DistributionData ret; fog_memory_distribution_test_get_value(mId, &aPingName, &ret.sum, &buckets, &counts);
--- a/toolkit/components/glean/bindings/private/Ping.h +++ b/toolkit/components/glean/bindings/private/Ping.h @@ -35,17 +35,17 @@ class Ping { * Pings always contain the `ping_info` and `client_info` sections. * See [ping * sections](https://mozilla.github.io/glean/book/user/pings/index.html#ping-sections) * for details. * * @param aReason - Optional. The reason the ping is being submitted. * Must match one of the configured `reason_codes`. */ - void Submit(const nsACString& aReason) const { + void Submit(const nsACString& aReason = nsCString()) const { fog_submit_ping_by_id(mId, &aReason); } private: const uint32_t mId; }; } // namespace impl
--- a/toolkit/components/glean/bindings/private/String.h +++ b/toolkit/components/glean/bindings/private/String.h @@ -37,24 +37,28 @@ class StringMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ - Maybe<nsCString> TestGetValue(const nsACString& aStorageName) const { - if (!fog_string_test_has_value(mId, &aStorageName)) { + Maybe<nsCString> TestGetValue( + const nsACString& aPingName = nsCString()) const { + if (!fog_string_test_has_value(mId, &aPingName)) { return Nothing(); } nsCString ret; - fog_string_test_get_value(mId, &aStorageName, &ret); + fog_string_test_get_value(mId, &aPingName, &ret); return Some(ret); } private: const uint32_t mId; }; } // namespace impl
--- a/toolkit/components/glean/bindings/private/Timespan.h +++ b/toolkit/components/glean/bindings/private/Timespan.h @@ -45,23 +45,26 @@ class TimespanMetric { * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ - Maybe<int64_t> TestGetValue(const nsACString& aStorageName) const { - if (!fog_timespan_test_has_value(mId, &aStorageName)) { + Maybe<int64_t> TestGetValue(const nsACString& aPingName = nsCString()) const { + if (!fog_timespan_test_has_value(mId, &aPingName)) { return Nothing(); } - return Some(fog_timespan_test_get_value(mId, &aStorageName)); + return Some(fog_timespan_test_get_value(mId, &aPingName)); } private: const uint32_t mId; }; } // namespace impl class GleanTimespan final : public nsIGleanTimespan {
--- a/toolkit/components/glean/bindings/private/Uuid.h +++ b/toolkit/components/glean/bindings/private/Uuid.h @@ -38,26 +38,29 @@ class UuidMetric { * Gets the currently stored value as a hyphenated string. * * 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. - * Panics if there is no value to get. + * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. * * @return value of the stored metric, or Nothing() if there is no value. */ - Maybe<nsCString> TestGetValue(const nsACString& aStorageName) const { - if (!fog_uuid_test_has_value(mId, &aStorageName)) { + Maybe<nsCString> TestGetValue( + const nsACString& aPingName = nsCString()) const { + if (!fog_uuid_test_has_value(mId, &aPingName)) { return Nothing(); } nsCString ret; - fog_uuid_test_get_value(mId, &aStorageName, &ret); + fog_uuid_test_get_value(mId, &aPingName, &ret); return Some(ret); } private: const uint32_t mId; }; } // namespace impl
--- a/toolkit/components/glean/gtest/TestFog.cpp +++ b/toolkit/components/glean/gtest/TestFog.cpp @@ -58,16 +58,18 @@ TEST(FOG, BuiltinPingsRegistered) TEST(FOG, TestCppCounterWorks) { mozilla::glean::test_only::bad_code.Add(42); ASSERT_EQ( 42, mozilla::glean::test_only::bad_code.TestGetValue("test-ping"_ns).value()); + // And test that the ping name's optional, while you're at it: + ASSERT_EQ(42, test_only::bad_code.TestGetValue().value()); } TEST(FOG, TestCppStringWorks) { auto kValue = "cheez!"_ns; mozilla::glean::test_only::cheesy_string.Set(kValue); ASSERT_STREQ(kValue.get(), mozilla::glean::test_only::cheesy_string
--- a/toolkit/components/glean/xpcom/nsIGleanMetrics.idl +++ b/toolkit/components/glean/xpcom/nsIGleanMetrics.idl @@ -22,19 +22,22 @@ interface nsIGleanBoolean : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or undefined if there is no value. */ - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); }; [scriptable, uuid(aa15fd20-1e8a-11eb-9bec-0800200c9a66)] interface nsIGleanDatetime : nsISupports { /** * Set the datetime to the provided value, or the local now. * @@ -50,20 +53,23 @@ interface nsIGleanDatetime : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or undefined if there is no value. */ [implicit_jscontext] - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); }; [scriptable, uuid(05b89d2a-d57c-11ea-82da-3f63399a6f5a)] interface nsIGleanCounter : nsISupports { /* * Increases the counter by `amount`. * @@ -78,19 +84,22 @@ interface nsIGleanCounter : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or undefined if there is no value. */ - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); }; [scriptable, uuid(eea5ed46-16ba-46cd-bb1f-504581987fe1)] interface nsIGleanMemoryDistribution : nsISupports { /* * Accumulates the provided sample in the metric. * @@ -109,20 +118,23 @@ interface nsIGleanMemoryDistribution : n * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or Nothing() if there is no value. */ [implicit_jscontext] - jsval testGetValue(in ACString aStorageName); + jsval testGetValue([optional] in ACString aPingName); }; [scriptable, uuid(5223a48b-687d-47ff-a629-fd4a72d1ecfa)] interface nsIGleanPing : nsISupports { /** * Collect and submit the ping for eventual upload. * @@ -162,20 +174,23 @@ interface nsIGleanString : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or undefined if there is no value. */ [implicit_jscontext] - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); }; [scriptable, uuid(2586530c-030f-11eb-93cb-cbf30d25225a)] interface nsIGleanTimespan : nsISupports { /** * Start tracking time for the provided metric. * @@ -202,19 +217,22 @@ interface nsIGleanTimespan : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or undefined if there is no value. */ - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); }; [scriptable, uuid(395700e7-06f6-46be-adcc-ea58977fda6d)] interface nsIGleanUuid : nsISupports { /** * Set to the specified value. * @@ -234,20 +252,23 @@ interface nsIGleanUuid : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric, or undefined if there is no value. */ [implicit_jscontext] - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); }; [scriptable, uuid(1b01424a-1f55-11eb-92a5-0754f6c3f240)] interface nsIGleanEvent : nsISupports { /* * Record an event. * @@ -263,13 +284,16 @@ interface nsIGleanEvent : nsISupports * * 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. * + * @param aPingName The (optional) name of the ping to retrieve the metric + * for. Defaults to the first value in `send_in_pings`. + * * @return value of the stored metric. */ [implicit_jscontext] - jsval testGetValue(in AUTF8String aStorageName); + jsval testGetValue([optional] in AUTF8String aPingName); };
--- a/toolkit/components/glean/xpcshell/test_Glean.js +++ b/toolkit/components/glean/xpcshell/test_Glean.js @@ -130,16 +130,18 @@ add_task({ skip_if: () => true }, functi const received = Glean.test_only.what_a_date.testGetValue("test-ping"); Assert.ok(received.startsWith("2020-06-11T12:00:00")); }); add_task(function test_fog_boolean_works() { Glean.test_only.can_we_flag_it.set(false); Assert.equal(false, Glean.test_only.can_we_flag_it.testGetValue("test-ping")); + // While you're here, might as well test that the ping name's optional. + Assert.equal(false, Glean.test_only.can_we_flag_it.testGetValue()); }); add_task(async function test_fog_event_works() { Glean.test_only_ipc.no_extra_event.record(); // FIXME(bug 1678567): Check that the value was recorded when we can. // Assert.ok(Glean.test_only_ipc.no_extra_event.testGetValue("store1")); let extra = { extra1: "can set extras", extra2: "passing more data" };