author | Alessio Placitelli <alessio.placitelli@gmail.com> |
Wed, 22 Feb 2017 11:57:04 +0100 | |
changeset 392048 | 578d891011c317df2bdfa315791f765394c9302c |
parent 392047 | f8194edb8d411e9caab1d26cd283a9791c978c61 |
child 392049 | 093c7e787c9b1523b3364fdaac7a4481106002ca |
push id | 7198 |
push user | jlorenzo@mozilla.com |
push date | Tue, 18 Apr 2017 12:07:49 +0000 |
treeherder | mozilla-beta@d57aa49c3948 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | gfritzsche |
bugs | 1340542 |
milestone | 54.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/telemetry/TelemetryScalar.cpp +++ b/toolkit/components/telemetry/TelemetryScalar.cpp @@ -1399,16 +1399,21 @@ TelemetryScalar::Add(const nsACString& a * Adds the value to the given scalar. * * @param aId The scalar enum id. * @param aVal The numeric value to add to the scalar. */ void TelemetryScalar::Add(mozilla::Telemetry::ScalarID aId, uint32_t aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, false) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1433,16 +1438,21 @@ TelemetryScalar::Add(mozilla::Telemetry: * @param aId The scalar enum id. * @param aKey The key name. * @param aVal The numeric value to add to the scalar. */ void TelemetryScalar::Add(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, uint32_t aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, true) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1537,16 +1547,21 @@ TelemetryScalar::Set(const nsACString& a * Sets the scalar to the given numeric value. * * @param aId The scalar enum id. * @param aValue The numeric, unsigned value to set the scalar to. */ void TelemetryScalar::Set(mozilla::Telemetry::ScalarID aId, uint32_t aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, false) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1569,16 +1584,21 @@ TelemetryScalar::Set(mozilla::Telemetry: * Sets the scalar to the given string value. * * @param aId The scalar enum id. * @param aValue The string value to set the scalar to. */ void TelemetryScalar::Set(mozilla::Telemetry::ScalarID aId, const nsAString& aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, false) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1601,16 +1621,21 @@ TelemetryScalar::Set(mozilla::Telemetry: * Sets the scalar to the given boolean value. * * @param aId The scalar enum id. * @param aValue The boolean value to set the scalar to. */ void TelemetryScalar::Set(mozilla::Telemetry::ScalarID aId, bool aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, false) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1635,16 +1660,21 @@ TelemetryScalar::Set(mozilla::Telemetry: * @param aId The scalar enum id. * @param aKey The scalar key. * @param aValue The numeric, unsigned value to set the scalar to. */ void TelemetryScalar::Set(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, uint32_t aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, true) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1669,16 +1699,21 @@ TelemetryScalar::Set(mozilla::Telemetry: * @param aId The scalar enum id. * @param aKey The scalar key. * @param aValue The boolean value to set the scalar to. */ void TelemetryScalar::Set(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, bool aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, true) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1773,16 +1808,21 @@ TelemetryScalar::SetMaximum(const nsACSt * Sets the scalar to the maximum of the current and the passed value. * * @param aId The scalar enum id. * @param aValue The numeric value to set the scalar to. */ void TelemetryScalar::SetMaximum(mozilla::Telemetry::ScalarID aId, uint32_t aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, false) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -1807,16 +1847,21 @@ TelemetryScalar::SetMaximum(mozilla::Tel * @param aId The scalar enum id. * @param aKey The key name. * @param aValue The numeric value to set the scalar to. */ void TelemetryScalar::SetMaximum(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, uint32_t aValue) { + if (NS_WARN_IF(!IsValidEnumId(aId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + return; + } + StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (internal_CanRecordScalar(aId, true) != ScalarResult::Ok) { // We can't record this scalar. Bail out. return; } // Accumulate in the child process if needed. @@ -2115,16 +2160,21 @@ TelemetryScalar::UpdateChildData(GeckoPr MOZ_ASSERT(XRE_IsParentProcess(), "The stored child processes scalar data must be updated from the parent process."); StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (!internal_CanRecordBase()) { return; } for (auto& upd : aScalarActions) { + if (NS_WARN_IF(!IsValidEnumId(upd.mId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + continue; + } + if (internal_IsKeyedScalar(upd.mId)) { continue; } // Are we allowed to record this scalar? We don't need to check for // allowed processes here, that's taken care of when recording // in child processes. if (!internal_CanRecordForScalarID(upd.mId)) { @@ -2201,16 +2251,21 @@ TelemetryScalar::UpdateChildKeyedData(Ge MOZ_ASSERT(XRE_IsParentProcess(), "The stored child processes keyed scalar data must be updated from the parent process."); StaticMutexAutoLock locker(gTelemetryScalarsMutex); if (!internal_CanRecordBase()) { return; } for (auto& upd : aScalarActions) { + if (NS_WARN_IF(!IsValidEnumId(upd.mId))) { + MOZ_ASSERT_UNREACHABLE("Scalar usage requires valid ids."); + continue; + } + if (!internal_IsKeyedScalar(upd.mId)) { continue; } // Are we allowed to record this scalar? We don't need to check for // allowed processes here, that's taken care of when recording // in child processes. if (!internal_CanRecordForScalarID(upd.mId)) {
--- a/toolkit/components/telemetry/tests/gtest/TestScalars.cpp +++ b/toolkit/components/telemetry/tests/gtest/TestScalars.cpp @@ -314,8 +314,49 @@ TEST_F(TelemetryTestFixture, NonMainThre // Shutdown the thread. This also waits for the runnable to complete. testingThread->Shutdown(); // Check the recorded value. JS::RootedValue scalarsSnapshot(cx.GetJSContext()); GetScalarsSnapshot(false, cx.GetJSContext(), &scalarsSnapshot); CheckUintScalar("telemetry.test.unsigned_int_kind", cx.GetJSContext(), scalarsSnapshot, 37); } + +TEST_F(TelemetryTestFixture, ScalarUnknownID) { + AutoJSContextWithGlobal cx(mCleanGlobal); + + // Make sure we don't get scalars from other tests. + Unused << mTelemetry->ClearScalars(); + +// Don't run this part in debug builds as that intentionally asserts. +#ifndef DEBUG + const uint32_t kTestFakeIds[] = { + static_cast<uint32_t>(Telemetry::ScalarID::ScalarCount), + static_cast<uint32_t>(Telemetry::ScalarID::ScalarCount) + 378537, + std::numeric_limits<uint32_t>::max() + }; + + for (auto id : kTestFakeIds) { + Telemetry::ScalarID scalarId = static_cast<Telemetry::ScalarID>(id); + Telemetry::ScalarSet(scalarId, static_cast<uint32_t>(1)); + Telemetry::ScalarSet(scalarId, true); + Telemetry::ScalarSet(scalarId, NS_LITERAL_STRING("test")); + Telemetry::ScalarAdd(scalarId, 1); + Telemetry::ScalarSetMaximum(scalarId, 1); + + // Make sure that nothing was recorded in the plain scalars. + JS::RootedValue scalarsSnapshot(cx.GetJSContext()); + GetScalarsSnapshot(false, cx.GetJSContext(), &scalarsSnapshot); + ASSERT_TRUE(scalarsSnapshot.isUndefined()) << "No scalar must be recorded"; + + // Same for the keyed scalars. + Telemetry::ScalarSet(scalarId, NS_LITERAL_STRING("key1"), static_cast<uint32_t>(1)); + Telemetry::ScalarSet(scalarId, NS_LITERAL_STRING("key1"), true); + Telemetry::ScalarAdd(scalarId, NS_LITERAL_STRING("key1"), 1); + Telemetry::ScalarSetMaximum(scalarId, NS_LITERAL_STRING("key1"), 1); + + // Make sure that nothing was recorded in the keyed scalars. + JS::RootedValue keyedSnapshot(cx.GetJSContext()); + GetScalarsSnapshot(true, cx.GetJSContext(), &keyedSnapshot); + ASSERT_TRUE(keyedSnapshot.isUndefined()) << "No keyed scalar must be recorded"; + } +#endif +}