[Extensions] Expand message tracker to track non-SW contexts.
Before this change the tracker didn't have a catch-all for messages
that go to destinations that are not a service worker. This is
necessary because there are places during message tracking where it is
difficult to determine what specific kind of destination type is being
targeted.
This is fine though because these are only for comparison to service
workers so them being less granular is okay.
This also makes some adjustments to naming as a result.
Bug: 371011217
Change-Id: I8639d5a1ab351b941525ca0a284ac488340e5b0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941000
Reviewed-by: Emilia Paz <emiliapaz@chromium.org>
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1371809}
diff --git a/extensions/browser/message_tracker.cc b/extensions/browser/message_tracker.cc
index 2a07d56..178ed21 100644
--- a/extensions/browser/message_tracker.cc
+++ b/extensions/browser/message_tracker.cc
@@ -25,15 +25,13 @@
MessageTracker::TestObserver* g_test_observer = nullptr;
std::string GetBackgroundStringForBackgroundType(
- MessageTracker::MessageDestinationBackgroundType& type) {
+ MessageTracker::MessageDestinationType& type) {
switch (type) {
- case MessageTracker::MessageDestinationBackgroundType::kNone:
- return "None";
- case MessageTracker::MessageDestinationBackgroundType::kPersistentPage:
- return "PersistentBackgroundPage";
- case MessageTracker::MessageDestinationBackgroundType::kEventPage:
- return "EventPage";
- case MessageTracker::MessageDestinationBackgroundType::kServiceWorker:
+ case MessageTracker::MessageDestinationType::kUnknown:
+ return "Unknown";
+ case MessageTracker::MessageDestinationType::kNonServiceWorker:
+ return "NonServiceWorker";
+ case MessageTracker::MessageDestinationType::kServiceWorker:
return "ServiceWorker";
}
}
@@ -83,7 +81,7 @@
MessageTracker::TrackedMessage::TrackedMessage(
const MessageDeliveryStage status,
- const MessageDestinationBackgroundType destination_background_type)
+ const MessageDestinationType destination_background_type)
: stage_(status),
destination_background_type_(destination_background_type) {
start_time_ = base::Time::Now();
@@ -97,7 +95,7 @@
return stage_;
}
-MessageTracker::MessageDestinationBackgroundType&
+MessageTracker::MessageDestinationType&
MessageTracker::TrackedMessage::destination_background_type() {
return destination_background_type_;
}
@@ -124,7 +122,7 @@
void MessageTracker::NotifyStartTrackingMessageDelivery(
const base::UnguessableToken& message_id,
const MessageDeliveryStage stage,
- const MessageDestinationBackgroundType destination_background_type) {
+ const MessageDestinationType destination_background_type) {
CHECK(!base::Contains(tracked_messages_, message_id));
tracked_messages_.emplace(message_id,
TrackedMessage(stage, destination_background_type));
diff --git a/extensions/browser/message_tracker.h b/extensions/browser/message_tracker.h
index 4839c5f..b14c096 100644
--- a/extensions/browser/message_tracker.h
+++ b/extensions/browser/message_tracker.h
@@ -28,14 +28,19 @@
// the message process for too long and becomes "stale".
class MessageTracker : public KeyedService {
public:
- // These values are persisted to logs. Entries should not be renumbered and
- // numeric values should never be reused.
- enum MessageDestinationBackgroundType {
- kNone = 0,
- kPersistentPage = 1,
- kEventPage = 2,
- kServiceWorker = 3,
- kMaxValue = kServiceWorker,
+ // Although this enum isn't directly used as a histogram value, it is used to
+ // name histograms. If the values change in any way, then the usages should be
+ // updated in this class and histograms.xml.
+ enum class MessageDestinationType {
+ // The message destination is not currently at the current point in the
+ // messaging processing.
+ kUnknown = 0,
+ // Extension contexts that are not a service worker (persistent background
+ // pages, event pages, content scripts, popup scripts, etc).
+ kNonServiceWorker = 1,
+ // An extension's background service worker context (script).
+ kServiceWorker = 2,
+
};
// These values are persisted to logs. Entries should not be renumbered and
@@ -64,18 +69,18 @@
public:
explicit TrackedMessage(
const MessageDeliveryStage stage,
- const MessageDestinationBackgroundType destination_background_type);
+ const MessageDestinationType destination_background_type);
~TrackedMessage() = default;
void ResetTimeout();
MessageDeliveryStage& stage();
- MessageDestinationBackgroundType& destination_background_type();
+ MessageDestinationType& destination_background_type();
const base::Time& start_time() { return start_time_; }
private:
MessageDeliveryStage stage_ = MessageDeliveryStage::kUnknown;
- MessageDestinationBackgroundType destination_background_type_ =
- MessageDestinationBackgroundType::kNone;
+ MessageDestinationType destination_background_type_ =
+ MessageDestinationType::kNonServiceWorker;
base::Time start_time_;
};
@@ -88,7 +93,7 @@
void NotifyStartTrackingMessageDelivery(
const base::UnguessableToken& message_id,
const MessageDeliveryStage stage,
- const MessageDestinationBackgroundType destination_background_type_);
+ const MessageDestinationType destination_background_type_);
// Notifies the tracker that a message has moved to the next stage in the
// messaging process. The message is tracked until it is delivered or becomes
diff --git a/extensions/browser/message_tracker_unittest.cc b/extensions/browser/message_tracker_unittest.cc
index a1fb3588..e37dc400 100644
--- a/extensions/browser/message_tracker_unittest.cc
+++ b/extensions/browser/message_tracker_unittest.cc
@@ -72,7 +72,7 @@
base::UnguessableToken message_id = base::UnguessableToken::Create();
message_tracker()->NotifyStartTrackingMessageDelivery(
message_id, MessageTracker::MessageDeliveryStage::kUnknown,
- MessageTracker::MessageDestinationBackgroundType::kServiceWorker);
+ MessageTracker::MessageDestinationType::kServiceWorker);
message_tracker()->NotifyUpdateMessageDelivery(
message_id,
@@ -86,7 +86,7 @@
class MessageTrackerMultiBackgroundDestinationUnitTest
: public MessageTrackerUnitTest,
public testing::WithParamInterface<
- MessageTracker::MessageDestinationBackgroundType> {};
+ MessageTracker::MessageDestinationType> {};
// Tests that the tracker correctly records and reports metrics when an
// extension message finishes its delivery and stops being tracked.
@@ -95,7 +95,7 @@
base::UnguessableToken message_id = base::UnguessableToken::Create();
MessageTrackerTestObserver observer(message_id);
message_tracker()->SetMessageStaleTimeoutForTest(base::Seconds(1));
- MessageTracker::MessageDestinationBackgroundType destination_background_type =
+ MessageTracker::MessageDestinationType destination_background_type =
GetParam();
message_tracker()->NotifyStartTrackingMessageDelivery(
message_id, MessageTracker::MessageDeliveryStage::kUnknown,
@@ -112,43 +112,29 @@
EXPECT_EQ(message_tracker()->GetNumberOfTrackedMessagesForTest(), 0u);
switch (destination_background_type) {
- case MessageTracker::MessageDestinationBackgroundType::kNone:
+ case MessageTracker::MessageDestinationType::kUnknown:
histogram_tester().ExpectBucketCount(
- "Extensions.MessagePipeline.MessageCompleted.None", /*sample=*/true,
- /*expected_count=*/1);
- histogram_tester().ExpectTotalCount(
- "Extensions.MessagePipeline.MessageCompletedTime.None",
- /*expected_count=*/1);
- histogram_tester().ExpectTotalCount(
- "Extensions.MessagePipeline.MessageStaleAtStage.None",
- /*expected_count=*/0);
- break;
- case MessageTracker::MessageDestinationBackgroundType::kPersistentPage:
- histogram_tester().ExpectBucketCount(
- "Extensions.MessagePipeline.MessageCompleted."
- "PersistentBackgroundPage",
+ "Extensions.MessagePipeline.MessageCompleted.Unknown",
/*sample=*/true, /*expected_count=*/1);
histogram_tester().ExpectTotalCount(
- "Extensions.MessagePipeline.MessageCompletedTime."
- "PersistentBackgroundPage",
+ "Extensions.MessagePipeline.MessageCompletedTime.Unknown",
/*expected_count=*/1);
histogram_tester().ExpectTotalCount(
- "Extensions.MessagePipeline.MessageStaleAtStage."
- "PersistentBackgroundPage",
+ "Extensions.MessagePipeline.MessageStaleAtStage.Unknown",
/*expected_count=*/0);
break;
- case MessageTracker::MessageDestinationBackgroundType::kEventPage:
+ case MessageTracker::MessageDestinationType::kNonServiceWorker:
histogram_tester().ExpectBucketCount(
- "Extensions.MessagePipeline.MessageCompleted.EventPage",
- /*sample=*//*sample=*/true, /*expected_count=*/1);
+ "Extensions.MessagePipeline.MessageCompleted.NonServiceWorker",
+ /*sample=*/true, /*expected_count=*/1);
histogram_tester().ExpectTotalCount(
- "Extensions.MessagePipeline.MessageCompletedTime.EventPage",
+ "Extensions.MessagePipeline.MessageCompletedTime.NonServiceWorker",
/*expected_count=*/1);
histogram_tester().ExpectTotalCount(
- "Extensions.MessagePipeline.MessageStaleAtStage.EventPage",
+ "Extensions.MessagePipeline.MessageStaleAtStage.NonServiceWorker",
/*expected_count=*/0);
break;
- case MessageTracker::MessageDestinationBackgroundType::kServiceWorker:
+ case MessageTracker::MessageDestinationType::kServiceWorker:
histogram_tester().ExpectBucketCount(
"Extensions.MessagePipeline.MessageCompleted.ServiceWorker",
/*sample=*/true, /*expected_count=*/1);
@@ -165,11 +151,8 @@
INSTANTIATE_TEST_SUITE_P(
BackgroundDestination,
MessageTrackerMultiBackgroundDestinationUnitTest,
- testing::Values(
- MessageTracker::MessageDestinationBackgroundType::kNone,
- MessageTracker::MessageDestinationBackgroundType::kPersistentPage,
- MessageTracker::MessageDestinationBackgroundType::kEventPage,
- MessageTracker::MessageDestinationBackgroundType::kServiceWorker));
+ testing::Values(MessageTracker::MessageDestinationType::kNonServiceWorker,
+ MessageTracker::MessageDestinationType::kServiceWorker));
// Tests that the tracker correctly records and reports metrics when an
// extension message becomes stale.
@@ -179,7 +162,7 @@
message_tracker()->SetMessageStaleTimeoutForTest(base::Microseconds(1));
message_tracker()->NotifyStartTrackingMessageDelivery(
message_id, MessageTracker::MessageDeliveryStage::kUnknown,
- MessageTracker::MessageDestinationBackgroundType::kServiceWorker);
+ MessageTracker::MessageDestinationType::kServiceWorker);
{
SCOPED_TRACE(
@@ -205,7 +188,7 @@
message_tracker()->SetMessageStaleTimeoutForTest(base::Seconds(1));
message_tracker()->NotifyStartTrackingMessageDelivery(
message_id, MessageTracker::MessageDeliveryStage::kUnknown,
- MessageTracker::MessageDestinationBackgroundType::kServiceWorker);
+ MessageTracker::MessageDestinationType::kServiceWorker);
message_tracker()->NotifyUpdateMessageDelivery(
message_id,
MessageTracker::MessageDeliveryStage::kOpenChannelRequestReceived);
@@ -243,7 +226,7 @@
message_tracker()->SetMessageStaleTimeoutForTest(base::Microseconds(1));
message_tracker()->NotifyStartTrackingMessageDelivery(
message_id, MessageTracker::MessageDeliveryStage::kUnknown,
- MessageTracker::MessageDestinationBackgroundType::kServiceWorker);
+ MessageTracker::MessageDestinationType::kServiceWorker);
{
SCOPED_TRACE(
@@ -273,7 +256,7 @@
message_tracker()->SetMessageStaleTimeoutForTest(base::Microseconds(1));
message_tracker()->NotifyStartTrackingMessageDelivery(
message_id, MessageTracker::MessageDeliveryStage::kUnknown,
- MessageTracker::MessageDestinationBackgroundType::kServiceWorker);
+ MessageTracker::MessageDestinationType::kServiceWorker);
{
SCOPED_TRACE(
diff --git a/tools/metrics/histograms/metadata/extensions/histograms.xml b/tools/metrics/histograms/metadata/extensions/histograms.xml
index 077d7e7..91b5796 100644
--- a/tools/metrics/histograms/metadata/extensions/histograms.xml
+++ b/tools/metrics/histograms/metadata/extensions/histograms.xml
@@ -43,14 +43,6 @@
<variant name="DeveloperModeOn" summary="developer mode on"/>
</variants>
-<variants name="ExtensionBackgroundType">
- <variant name="EventPage" summary="event page"/>
- <variant name="None" summary="no background page"/>
- <variant name="PersistentBackgroundPage"
- summary="persistent background page"/>
- <variant name="ServiceWorker" summary="service worker"/>
-</variants>
-
<variants name="ExtensionFunctionExecutionTime">
<variant name=".1msTo5ms"
summary="Execution took between 1ms and 5ms (tolerable)."/>
@@ -118,6 +110,12 @@
kCommandLine and kUnpacked)"/>
</variants>
+<variants name="MessageDestinationType">
+ <variant name="NonServiceWorker" summary="not a service worker"/>
+ <variant name="ServiceWorker" summary="service worker"/>
+ <variant name="Unknown" summary="unknown type"/>
+</variants>
+
<!--
This is emitted for two scenarios when a worker may be unregistered:
@@ -3916,14 +3914,13 @@
</histogram>
<histogram
- name="Extensions.MessagePipeline.MessageCompleted.{ExtensionBackgroundType}"
+ name="Extensions.MessagePipeline.MessageCompleted.{MessageDestinationType}"
enum="Boolean" expires_after="2025-04-10">
<owner>jlulejian@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<summary>
Tracks the overall success/failure of messages as they progress through the
- messaging process to background page destination type
- {ExtensionBackgroundType}.
+ messaging process to destination type {MessageDestinationType}.
It is emitted either:
@@ -3933,18 +3930,17 @@
*) when a message has stayed in a given stage for ~30 seconds (false) and is
now considered "stale"
</summary>
- <token key="ExtensionBackgroundType" variants="ExtensionBackgroundType"/>
+ <token key="MessageDestinationType" variants="MessageDestinationType"/>
</histogram>
<histogram
- name="Extensions.MessagePipeline.MessageCompletedTime.{ExtensionBackgroundType}"
+ name="Extensions.MessagePipeline.MessageCompletedTime.{MessageDestinationType}"
units="microseconds" expires_after="2025-04-10">
<owner>jlulejian@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<summary>
Tracks how long it took for an extension message to successfully complete
- the messaging process to background page destination type
- {ExtensionBackgroundType}.
+ the messaging process to destination type {MessageDestinationType}.
It is emitted when
Extensions.ServiceWorkerBackground.Extensions.ServiceWorkerBackground.MessageStagesCompleted
@@ -3952,23 +3948,23 @@
This is only emitted for users with high resolution clocks.
</summary>
- <token key="ExtensionBackgroundType" variants="ExtensionBackgroundType"/>
+ <token key="MessageDestinationType" variants="MessageDestinationType"/>
</histogram>
<histogram
- name="Extensions.MessagePipeline.MessageStaleAtStage.{ExtensionBackgroundType}"
+ name="Extensions.MessagePipeline.MessageStaleAtStage.{MessageDestinationType}"
enum="MessageDeliveryStage" expires_after="2025-04-10">
<owner>jlulejian@chromium.org</owner>
<owner>extensions-core@chromium.org</owner>
<summary>
Tracks the stage where the an extension message to the background context
- became stale to background page destination type {ExtensionBackgroundType}.
+ became stale to destination type {MessageDestinationType}.
It is emitted when
Extensions.ServiceWorkerBackground.Extensions.ServiceWorkerBackground.MessageStagesCompleted
is false.
</summary>
- <token key="ExtensionBackgroundType" variants="ExtensionBackgroundType"/>
+ <token key="MessageDestinationType" variants="MessageDestinationType"/>
</histogram>
<histogram