Bug 1394494 - Use a custom Frame::Kind for special BHR frame types, r=froydnj

toolkit/components/backgroundhangmonitor/HangDetails.cpp

@@ -70,6 +70,30 @@ nsHangDetails::GetAnnotations(JSContext* aCx, JS::MutableHandleValue aVal)
   return NS_OK;
 }
 
+namespace  {
+
+nsresult
+StringFrame(JSContext* aCx,
+            JS::RootedObject& aTarget,
+            size_t aIndex,
+            const char* aString)
+{
+  JSString* jsString = JS_NewStringCopyZ(aCx, aString);
+  if (!jsString) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  JS::RootedString string(aCx, jsString);
+  if (!string) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  if (!JS_DefineElement(aCx, aTarget, aIndex, string, JSPROP_ENUMERATE)) {
+    return NS_ERROR_OUT_OF_MEMORY;
+  }
+  return NS_OK;
+}
+
+} // anonymous namespace
+
 NS_IMETHODIMP
 nsHangDetails::GetStack(JSContext* aCx, JS::MutableHandle<JS::Value> aVal)
 {
@@ -81,17 +105,8 @@ nsHangDetails::GetStack(JSContext* aCx, JS::MutableHandle<JS::Value> aVal)
     const HangStack::Frame& frame = mDetails.mStack[i];
     switch (frame.GetKind()) {
       case HangStack::Frame::Kind::STRING: {
-        JSString* jsString = JS_NewStringCopyZ(aCx, frame.AsString());
-        if (!jsString) {
-          return NS_ERROR_OUT_OF_MEMORY;
-        }
-        JS::RootedString string(aCx, jsString);
-        if (!string) {
-          return NS_ERROR_OUT_OF_MEMORY;
-        }
-        if (!JS_DefineElement(aCx, ret, i, string, JSPROP_ENUMERATE)) {
-          return NS_ERROR_OUT_OF_MEMORY;
-        }
+        nsresult rv = StringFrame(aCx, ret, i, frame.AsString());
+        NS_ENSURE_SUCCESS(rv, rv);
         break;
       }
 
@@ -138,6 +153,31 @@ nsHangDetails::GetStack(JSContext* aCx, JS::MutableHandle<JS::Value> aVal)
         }
         break;
       }
+
+      case HangStack::Frame::Kind::CONTENT: {
+        nsresult rv = StringFrame(aCx, ret, i, "(content script)");
+        NS_ENSURE_SUCCESS(rv, rv);
+        break;
+      }
+
+      case HangStack::Frame::Kind::JIT: {
+        nsresult rv = StringFrame(aCx, ret, i, "(jit frame)");
+        NS_ENSURE_SUCCESS(rv, rv);
+        break;
+      }
+
+      case HangStack::Frame::Kind::WASM: {
+        nsresult rv = StringFrame(aCx, ret, i, "(wasm)");
+        NS_ENSURE_SUCCESS(rv, rv);
+        break;
+      }
+
+      case HangStack::Frame::Kind::SUPPRESSED: {
+        nsresult rv = StringFrame(aCx, ret, i, "(profiling suppressed)");
+        NS_ENSURE_SUCCESS(rv, rv);
+        break;
+      }
+
       default:
         MOZ_ASSERT_UNREACHABLE("Invalid variant");
         break;

toolkit/components/backgroundhangmonitor/HangStack.cpp

@@ -227,6 +227,13 @@ ParamTraits<mozilla::HangStack>::Write(Message* aMsg, const mozilla::HangStack&
         WriteParam(aMsg, frame.AsPC());
         break;
       }
+      case Frame::Kind::CONTENT:
+      case Frame::Kind::WASM:
+      case Frame::Kind::JIT:
+      case Frame::Kind::SUPPRESSED: {
+        // NOTE: no associated data.
+        break;
+      }
       default: {
         MOZ_RELEASE_ASSERT(false, "Invalid kind for HangStack Frame");
         break;
@@ -285,6 +292,18 @@ ParamTraits<mozilla::HangStack>::Read(const Message* aMsg,
         aResult->infallibleAppend(Frame(pc));
         break;
       }
+      case Frame::Kind::CONTENT:
+        aResult->infallibleAppend(Frame::Content());
+        break;
+      case Frame::Kind::WASM:
+        aResult->infallibleAppend(Frame::Wasm());
+        break;
+      case Frame::Kind::JIT:
+        aResult->infallibleAppend(Frame::Jit());
+        break;
+      case Frame::Kind::SUPPRESSED:
+        aResult->infallibleAppend(Frame::Suppressed());
+        break;
       default:
         // We can't deserialize other kinds!
         return false;

toolkit/components/backgroundhangmonitor/HangStack.h

@@ -44,6 +44,10 @@ public:
   // * Kind::STRING(const char*) : A string representing a pseudostack or chrome JS stack frame.
   // * Kind::MODOFFSET(ModOffset) : A module index and offset into that module.
   // * Kind::PC(uintptr_t) : A raw program counter which has not been mapped to a module.
+  // * Kind::CONTENT: A hidden "(content script)" frame.
+  // * Kind::JIT : An unprocessed  "(jit frame)".
+  // * Kind::WASM : An unprocessed "(wasm)" frame.
+  // * Kind::SUPPRESSED : A JS frame while profiling was suppressed.
   //
   // NOTE: A manually rolled tagged enum is used instead of mozilla::Variant
   // here because we cannot use mozilla::Variant's IPC serialization directly.
@@ -57,6 +61,10 @@ public:
       STRING,
       MODOFFSET,
       PC,
+      CONTENT,
+      JIT,
+      WASM,
+      SUPPRESSED,
       END // Marker
     };
 
@@ -104,7 +112,32 @@ public:
       return mPC;
     }
 
+    // Public constant frames copies of each of the data-less frames.
+    static Frame Content() {
+      return Frame(Kind::CONTENT);
+    }
+    static Frame Jit() {
+      return Frame(Kind::JIT);
+    }
+    static Frame Wasm() {
+      return Frame(Kind::WASM);
+    }
+    static Frame Suppressed() {
+      return Frame(Kind::SUPPRESSED);
+    }
+
   private:
+    explicit Frame(Kind aKind)
+      : mKind(aKind)
+    {
+      MOZ_ASSERT(aKind == Kind::CONTENT ||
+                 aKind == Kind::JIT ||
+                 aKind == Kind::WASM ||
+                 aKind == Kind::SUPPRESSED,
+                 "Kind must only be one of CONTENT, JIT, WASM or SUPPRESSED "
+                 "for the data-free constructor.");
+    }
+
     Kind mKind;
     union {
       const char* mString;

toolkit/components/backgroundhangmonitor/ThreadStackHelper.cpp

@@ -165,6 +165,20 @@ ThreadStackHelper::TryAppendFrame(HangStack::Frame aFrame)
 {
   MOZ_RELEASE_ASSERT(mStackToFill);
 
+  // We deduplicate identical frames of kind CONTENT, JIT, WASM, and SUPPRESSED.
+  switch (aFrame.GetKind()) {
+    case HangStack::Frame::Kind::CONTENT:
+    case HangStack::Frame::Kind::JIT:
+    case HangStack::Frame::Kind::WASM:
+    case HangStack::Frame::Kind::SUPPRESSED:
+      if (!mStackToFill->empty() && mStackToFill->back().GetKind() == aFrame.GetKind()) {
+        return;
+      }
+      break;
+    default:
+      break;
+  }
+
   // Record that we _want_ to use another frame entry. If this exceeds
   // mMaxStackSize, we'll allocate more room on the next hang.
   mDesiredStackSize += 1;
@@ -186,7 +200,7 @@ void
 ThreadStackHelper::CollectJitReturnAddr(void* aAddr)
 {
   MOZ_RELEASE_ASSERT(mStackToFill);
-  TryAppendFrame(HangStack::Frame("(jit frame)"));
+  TryAppendFrame(HangStack::Frame::Jit());
 }
 
 void
@@ -195,7 +209,7 @@ ThreadStackHelper::CollectWasmFrame(const char* aLabel)
   MOZ_RELEASE_ASSERT(mStackToFill);
   // We don't want to collect WASM frames, as they are probably for content, so
   // we just add a "(content wasm)" frame.
-  TryAppendFrame(HangStack::Frame("(wasm)"));
+  TryAppendFrame(HangStack::Frame::Wasm());
 }
 
 namespace {
@@ -247,10 +261,6 @@ GetPathAfterComponent(const char* filename, const char (&component)[LEN]) {
 void
 ThreadStackHelper::CollectPseudoEntry(const js::ProfileEntry& aEntry)
 {
-  HangStack::Frame* backFrame = mStackToFill->empty()
-    ? nullptr
-    : &mStackToFill->back();
-
   // For non-js frames we just include the raw label.
   if (!aEntry.isJs()) {
     const char* label = aEntry.label();
@@ -259,18 +269,12 @@ ThreadStackHelper::CollectPseudoEntry(const js::ProfileEntry& aEntry)
   }
 
   if (!aEntry.script()) {
-    TryAppendFrame(HangStack::Frame("(profiling suppressed)"));
+    TryAppendFrame(HangStack::Frame::Suppressed());
     return;
   }
 
   if (!IsChromeJSScript(aEntry.script())) {
-    // NOTE: Deduplicate consecutive (content script) frames.
-    if (backFrame &&
-        backFrame->GetKind() == HangStack::Frame::Kind::STRING &&
-        !strcmp(backFrame->AsString(), "(content script)")) {
-      return;
-    }
-    TryAppendFrame(HangStack::Frame("(content script)"));
+    TryAppendFrame(HangStack::Frame::Content());
     return;
   }