Bug 1614933 - Gather content processes' crash annotations at exception time instead of using IPC; r=froydnj

Crash annotations in content processes are currently sent over IPC via
shared memory buffers. To pave the way for the Rust rewrite of the exception
handler we are removing this code and gathering all the crash annotations
within the content processes themselves. This patch causes annotations to be
stored in the global table of each content process. They are then streamed
out to the parent process by the exception handler together with the
exception-time annotations.

This has a number of benefits:

* we have one less channel to exchange data between content processes and
  the parent process
* we save memory because we don't need to allocate the shared memory buffers
* annotations are faster because we don't stream them all out every time one
  changes
* we won't truncate annotations anymore if we run out of space in the shared
  segment.
* we don't need delayed annotations anymore, so we can get rid of the
  associated machinery

As I refactored the code I tried to adjust all the obsolete comments,
consolidate shared code and remove the redundant steps that were sometimes
present. In many places we had two entire crash annotation tables we merged to
change just a couple; that comes from the fact that historically we loaded
them from disk. Now it doesn't matter anymore and we can just go ahead and
change the ones we care about.

Differential Revision: https://phabricator.services.mozilla.com/D62586

--HG--
extra : moz-landing-system : lando

dom/ipc/ContentParent.cpp

@@ -3455,9 +3455,9 @@ void ContentParent::FriendlyName(nsAString& aName, bool aAnonymize) {
 }
 
 mozilla::ipc::IPCResult ContentParent::RecvInitCrashReporter(
-    Shmem&& aShmem, const NativeThreadId& aThreadId) {
-  mCrashReporter = MakeUnique<CrashReporterHost>(GeckoProcessType_Content,
-                                                 aShmem, aThreadId);
+    const NativeThreadId& aThreadId) {
+  mCrashReporter =
+      MakeUnique<CrashReporterHost>(GeckoProcessType_Content, aThreadId);
 
   return IPC_OK();
 }

dom/ipc/ContentParent.h

@@ -429,7 +429,7 @@ class ContentParent final
   virtual void OnChannelError() override;
 
   mozilla::ipc::IPCResult RecvInitCrashReporter(
-      Shmem&& aShmem, const NativeThreadId& aThreadId);
+      const NativeThreadId& aThreadId);
 
   PNeckoParent* AllocPNeckoParent();
 

dom/ipc/PContent.ipdl

@@ -889,7 +889,7 @@ parent:
 
     async PRemoteSpellcheckEngine();
 
-    async InitCrashReporter(Shmem shmem, NativeThreadId tid);
+    async InitCrashReporter(NativeThreadId tid);
 
     sync IsSecureURI(uint32_t aType, URIParams aURI, uint32_t aFlags,
                      OriginAttributes aOriginAttributes)

dom/media/gmp/PGMP.ipdl

@@ -18,7 +18,7 @@ intr protocol PGMP
   manages PGMPStorage;
 
 parent:
-  async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
+  async InitCrashReporter(NativeThreadId threadId);
   async PGMPTimer();
   async PGMPStorage();
 

dom/media/ipc/PRDD.ipdl

@@ -49,7 +49,7 @@ parent:
 
 child:
 
-  async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
+  async InitCrashReporter(NativeThreadId threadId);
 
   async AddMemoryReport(MemoryReport aReport);
   async FinishMemoryReport(uint32_t aGeneration);

dom/plugins/ipc/PPluginModule.ipdl

@@ -88,7 +88,7 @@ child:
 
   async SetParentHangTimeout(uint32_t seconds);
 
-  intr InitCrashReporter(Shmem shmem)
+  intr InitCrashReporter()
     returns (NativeThreadId tid);
 
   async SettingChanged(PluginSettings settings);

dom/plugins/ipc/PluginModuleChild.cpp

@@ -715,8 +715,8 @@ mozilla::ipc::IPCResult PluginModuleChild::RecvInitPluginFunctionBroker(
 }
 
 mozilla::ipc::IPCResult PluginModuleChild::AnswerInitCrashReporter(
-    Shmem&& aShmem, mozilla::dom::NativeThreadId* aOutId) {
-  CrashReporterClient::InitSingletonWithShmem(aShmem);
+    mozilla::dom::NativeThreadId* aOutId) {
+  CrashReporterClient::InitSingleton();
   *aOutId = CrashReporter::CurrentThreadId();
 
   return IPC_OK();

dom/plugins/ipc/PluginModuleChild.h

@@ -97,7 +97,7 @@ class PluginModuleChild : public PPluginModuleChild {
   mozilla::ipc::IPCResult RecvSetParentHangTimeout(const uint32_t& aSeconds);
 
   mozilla::ipc::IPCResult AnswerInitCrashReporter(
-      Shmem&& aShmem, mozilla::dom::NativeThreadId* aId);
+      mozilla::dom::NativeThreadId* aId);
 
   virtual void ActorDestroy(ActorDestroyReason why) override;
 

dom/plugins/ipc/PluginModuleParent.cpp

@@ -527,20 +527,15 @@ void PluginModuleChromeParent::OnProcessLaunched(const bool aSucceeded) {
 }
 
 bool PluginModuleChromeParent::InitCrashReporter() {
-  ipc::Shmem shmem;
-  if (!ipc::CrashReporterClient::AllocShmem(this, &shmem)) {
-    return false;
-  }
-
   NativeThreadId threadId;
-  if (!CallInitCrashReporter(std::move(shmem), &threadId)) {
+  if (!CallInitCrashReporter(&threadId)) {
     return false;
   }
 
   {
     mozilla::MutexAutoLock lock(mCrashReporterMutex);
-    mCrashReporter = MakeUnique<ipc::CrashReporterHost>(GeckoProcessType_Plugin,
-                                                        shmem, threadId);
+    mCrashReporter =
+        MakeUnique<ipc::CrashReporterHost>(GeckoProcessType_Plugin, threadId);
   }
 
   return true;

gfx/ipc/PGPU.ipdl

@@ -120,7 +120,7 @@ child:
   // Graphics errors, analogous to PContent::GraphicsError
   async GraphicsError(nsCString aError);
 
-  async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
+  async InitCrashReporter(NativeThreadId threadId);
 
   async CreateVRProcess();
   async ShutdownVRProcess();

gfx/vr/ipc/PVR.ipdl

@@ -39,7 +39,7 @@ child:
   async InitComplete();
   async OpenVRControllerActionPathToParent(nsCString aPath);
   async OpenVRControllerManifestPathToParent(VRControllerType aType, nsCString aPath);
-  async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
+  async InitCrashReporter(NativeThreadId threadId);
   async AddMemoryReport(MemoryReport aReport);
   async FinishMemoryReport(uint32_t aGeneration);
 };

ipc/glue/CrashReporterClient.cpp

@@ -5,7 +5,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CrashReporterClient.h"
-#include "CrashReporterMetadataShmem.h"
 #include "nsISupportsImpl.h"
 
 namespace mozilla {
@@ -14,8 +13,7 @@ namespace ipc {
 StaticMutex CrashReporterClient::sLock;
 StaticRefPtr<CrashReporterClient> CrashReporterClient::sClientSingleton;
 
-CrashReporterClient::CrashReporterClient(const Shmem& aShmem)
-    : mMetadata(new CrashReporterMetadataShmem(aShmem)) {
+CrashReporterClient::CrashReporterClient() {
   MOZ_COUNT_CTOR(CrashReporterClient);
 }
 
@@ -23,27 +21,14 @@ CrashReporterClient::~CrashReporterClient() {
   MOZ_COUNT_DTOR(CrashReporterClient);
 }
 
-void CrashReporterClient::AnnotateCrashReport(CrashReporter::Annotation aKey,
-                                              const nsACString& aData) {
-  StaticMutexAutoLock lock(sLock);
-  mMetadata->AnnotateCrashReport(aKey, aData);
-}
-
-void CrashReporterClient::AppendAppNotes(const nsACString& aData) {
-  StaticMutexAutoLock lock(sLock);
-  mMetadata->AppendAppNotes(aData);
-}
-
 /* static */
-void CrashReporterClient::InitSingletonWithShmem(const Shmem& aShmem) {
+void CrashReporterClient::InitSingleton() {
   {
     StaticMutexAutoLock lock(sLock);
 
     MOZ_ASSERT(!sClientSingleton);
-    sClientSingleton = new CrashReporterClient(aShmem);
+    sClientSingleton = new CrashReporterClient();
   }
-
-  CrashReporter::NotifyCrashReporterClientCreated();
 }
 
 /* static */

ipc/glue/CrashReporterClient.h

@@ -11,66 +11,36 @@
 #include "mozilla/StaticMutex.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/Unused.h"
-#include "mozilla/ipc/Shmem.h"
 
 namespace mozilla {
 namespace ipc {
 
-class CrashReporterMetadataShmem;
-
 class CrashReporterClient {
  public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CrashReporterClient);
 
-  // |aTopLevelProtocol| must be a top-level protocol instance, as sub-actors
-  // do not have AllocUnsafeShmem. It must also have a child-to-parent message:
+  // |aTopLevelProtocol| must have a child-to-parent message:
   //
-  //   async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
-  //
-  // The parent-side receive function of this message should save the shmem
-  // somewhere, and when the top-level actor's ActorDestroy runs (or when the
-  // crash reporter needs metadata), the shmem should be parsed.
+  //   async InitCrashReporter(NativeThreadId threadId);
   template <typename T>
   static void InitSingleton(T* aToplevelProtocol) {
-    Shmem shmem;
-    if (!AllocShmem(aToplevelProtocol, &shmem)) {
-      MOZ_DIAGNOSTIC_ASSERT(false, "failed to allocate crash reporter shmem");
-      return;
-    }
-
-    InitSingletonWithShmem(shmem);
+    InitSingleton();
     Unused << aToplevelProtocol->SendInitCrashReporter(
-        std::move(shmem), CrashReporter::CurrentThreadId());
+        CrashReporter::CurrentThreadId());
   }
 
-  template <typename T>
-  static bool AllocShmem(T* aToplevelProtocol, Shmem* aOutShmem) {
-    // 16KB should be enough for most metadata - see bug 1278717 comment #11.
-    static const size_t kShmemSize = 16 * 1024;
-
-    return aToplevelProtocol->AllocUnsafeShmem(
-        kShmemSize, SharedMemory::TYPE_BASIC, aOutShmem);
-  }
-
-  static void InitSingletonWithShmem(const Shmem& aShmem);
+  static void InitSingleton();
 
   static void DestroySingleton();
   static RefPtr<CrashReporterClient> GetSingleton();
 
-  void AnnotateCrashReport(CrashReporter::Annotation aKey,
-                           const nsACString& aData);
-  void AppendAppNotes(const nsACString& aData);
-
  private:
-  explicit CrashReporterClient(const Shmem& aShmem);
+  explicit CrashReporterClient();
   ~CrashReporterClient();
 
  private:
   static StaticMutex sLock;
   static StaticRefPtr<CrashReporterClient> sClientSingleton;
-
- private:
-  UniquePtr<CrashReporterMetadataShmem> mMetadata;
 };
 
 }  // namespace ipc

ipc/glue/CrashReporterHelper.h

@@ -6,7 +6,6 @@
 #define mozilla_ipc_CrashReporterHelper_h
 
 #include "CrashReporterHost.h"
-#include "mozilla/ipc/Shmem.h"
 #include "mozilla/UniquePtr.h"
 #include "nsExceptionHandler.h"
 #include "nsICrashService.h"
@@ -19,7 +18,7 @@ namespace ipc {
  * toplevel protocols representing processes. To use this class, you should:
  *
  * 1. Declare a method to initialize the crash reporter in your IPDL:
- *    `async InitCrashReporter(Shmem shmem, NativeThreadId threadId)`
+ *    `async InitCrashReporter(NativeThreadId threadId)`
  *
  * 2. Inherit from this class, providing the appropriate `GeckoProcessType`
  *    enum value for the template parameter PT.
@@ -33,9 +32,8 @@ template <GeckoProcessType PT>
 class CrashReporterHelper {
  public:
   CrashReporterHelper() : mCrashReporter(nullptr) {}
-  IPCResult RecvInitCrashReporter(Shmem&& aShmem,
-                                  const CrashReporter::ThreadId& aThreadId) {
-    mCrashReporter = MakeUnique<ipc::CrashReporterHost>(PT, aShmem, aThreadId);
+  IPCResult RecvInitCrashReporter(const CrashReporter::ThreadId& aThreadId) {
+    mCrashReporter = MakeUnique<ipc::CrashReporterHost>(PT, aThreadId);
     return IPC_OK();
   }
 

ipc/glue/CrashReporterHost.cpp

@@ -5,7 +5,6 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CrashReporterHost.h"
-#include "CrashReporterMetadataShmem.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/SyncRunnable.h"
@@ -58,10 +57,8 @@ namespace mozilla {
 namespace ipc {
 
 CrashReporterHost::CrashReporterHost(GeckoProcessType aProcessType,
-                                     const Shmem& aShmem,
                                      CrashReporter::ThreadId aThreadId)
     : mProcessType(aProcessType),
-      mShmem(aShmem),
       mThreadId(aThreadId),
       mStartTime(::time(nullptr)),
       mFinalized(false) {}
@@ -112,22 +109,14 @@ bool CrashReporterHost::FinalizeCrashReport() {
   MOZ_ASSERT(!mFinalized);
   MOZ_ASSERT(HasMinidump());
 
-  CrashReporter::AnnotationTable annotations;
-
-  annotations[CrashReporter::Annotation::ProcessType] =
+  mExtraAnnotations[CrashReporter::Annotation::ProcessType] =
       XRE_ChildProcessTypeToAnnotation(mProcessType);
 
   char startTime[32];
   SprintfLiteral(startTime, "%lld", static_cast<long long>(mStartTime));
-  annotations[CrashReporter::Annotation::StartupTime] =
+  mExtraAnnotations[CrashReporter::Annotation::StartupTime] =
       nsDependentCString(startTime);
 
-  // We might not have shmem (for example, when running crashreporter tests).
-  if (mShmem.IsReadable()) {
-    CrashReporterMetadataShmem::ReadAppNotes(mShmem, annotations);
-  }
-
-  MergeCrashAnnotations(mExtraAnnotations, annotations);
   CrashReporter::WriteExtraFile(mDumpID, mExtraAnnotations);
 
   RecordCrash(mProcessType, GetCrashType(), mDumpID);

ipc/glue/CrashReporterHost.h

@@ -10,7 +10,6 @@
 #include <functional>
 
 #include "mozilla/UniquePtr.h"
-#include "mozilla/ipc/Shmem.h"
 #include "base/process.h"
 #include "nsExceptionHandler.h"
 #include "nsThreadUtils.h"
@@ -20,16 +19,14 @@ namespace mozilla {
 namespace ipc {
 
 // This is the newer replacement for CrashReporterParent. It is created in
-// response to a InitCrashReporter message on a top-level actor, and simply
-// holds the metadata shmem alive until the process ends. When the process
-// terminates abnormally, the top-level should call GenerateCrashReport to
-// automatically integrate metadata.
+// response to a InitCrashReporter message on a top-level actor. When the
+// process terminates abnormally, the top-level should call GenerateCrashReport
+// to automatically integrate metadata.
 class CrashReporterHost {
-  typedef mozilla::ipc::Shmem Shmem;
   typedef CrashReporter::AnnotationTable AnnotationTable;
 
  public:
-  CrashReporterHost(GeckoProcessType aProcessType, const Shmem& aShmem,
+  CrashReporterHost(GeckoProcessType aProcessType,
                     CrashReporter::ThreadId aThreadId);
 
   // Helper function for generating a crash report for a process that probably
@@ -121,7 +118,6 @@ class CrashReporterHost {
 
  private:
   GeckoProcessType mProcessType;
-  Shmem mShmem;
   CrashReporter::ThreadId mThreadId;
   time_t mStartTime;
   AnnotationTable mExtraAnnotations;

ipc/glue/CrashReporterMetadataShmem.cpp

@@ -1,225 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "CrashReporterMetadataShmem.h"
-#include "mozilla/Attributes.h"
-#include "mozilla/EnumeratedRange.h"
-#include "nsISupportsImpl.h"
-
-namespace mozilla {
-namespace ipc {
-
-using CrashReporter::Annotation;
-
-enum class EntryType : uint8_t {
-  None,
-  Annotation,
-};
-
-CrashReporterMetadataShmem::CrashReporterMetadataShmem(const Shmem& aShmem)
-    : mShmem(aShmem) {
-  MOZ_COUNT_CTOR(CrashReporterMetadataShmem);
-}
-
-CrashReporterMetadataShmem::~CrashReporterMetadataShmem() {
-  MOZ_COUNT_DTOR(CrashReporterMetadataShmem);
-}
-
-void CrashReporterMetadataShmem::AnnotateCrashReport(Annotation aKey,
-                                                     const nsACString& aData) {
-  mAnnotations[aKey] = aData;
-  SyncNotesToShmem();
-}
-
-void CrashReporterMetadataShmem::AppendAppNotes(const nsACString& aData) {
-  mAppNotes.Append(aData);
-  mAnnotations[Annotation::Notes] = mAppNotes;
-  SyncNotesToShmem();
-}
-
-class MOZ_STACK_CLASS MetadataShmemWriter {
- public:
-  explicit MetadataShmemWriter(const Shmem& aShmem)
-      : mCursor(aShmem.get<uint8_t>()), mEnd(mCursor + aShmem.Size<uint8_t>()) {
-    *mCursor = uint8_t(EntryType::None);
-  }
-
-  MOZ_MUST_USE bool WriteAnnotation(Annotation aKey, const nsCString& aValue) {
-    // This shouldn't happen because Commit() guarantees mCursor < mEnd. But
-    // we might as well be safe.
-    if (mCursor >= mEnd) {
-      return false;
-    }
-
-    // Save the current position so we can write the entry type if the entire
-    // entry fits.
-    uint8_t* start = mCursor++;
-    if (!Write(aKey) || !Write(aValue)) {
-      return false;
-    }
-    return Commit(start, EntryType::Annotation);
-  }
-
- private:
-  // On success, append a new terminal byte. On failure, rollback the cursor.
-  MOZ_MUST_USE bool Commit(uint8_t* aStart, EntryType aType) {
-    MOZ_ASSERT(aStart < mEnd);
-    MOZ_ASSERT(EntryType(*aStart) == EntryType::None);
-
-    if (mCursor >= mEnd) {
-      // No room for a terminating byte - rollback.
-      mCursor = aStart;
-      return false;
-    }
-
-    // Commit the entry and write a new terminal byte.
-    *aStart = uint8_t(aType);
-    *mCursor = uint8_t(EntryType::None);
-    return true;
-  }
-
-  MOZ_MUST_USE bool Write(const nsCString& aString) {
-    // 32-bit length is okay since our shmems are very small (16K),
-    // a huge write would fail anyway.
-    return Write(static_cast<uint32_t>(aString.Length())) &&
-           Write(aString.get(), aString.Length());
-  }
-
-  template <typename T>
-  MOZ_MUST_USE bool Write(const T& aT) {
-    return Write(&aT, sizeof(T));
-  }
-
-  MOZ_MUST_USE bool Write(const void* aData, size_t aLength) {
-    if (size_t(mEnd - mCursor) < aLength) {
-      return false;
-    }
-    memcpy(mCursor, aData, aLength);
-    mCursor += aLength;
-    return true;
-  }
-
- private:
-  // The cursor (beginning at start) always points to a single byte
-  // representing the next EntryType. An EntryType is either None,
-  // indicating there are no more entries, or Annotation, meaning
-  // two strings follow.
-  //
-  // Strings are written as a 32-bit length and byte sequence. After each new
-  // entry, a None entry is always appended, and a subsequent entry will
-  // overwrite this byte.
-  uint8_t* mCursor;
-  uint8_t* mEnd;
-};
-
-void CrashReporterMetadataShmem::SyncNotesToShmem() {
-  MetadataShmemWriter writer(mShmem);
-
-  for (auto key : MakeEnumeratedRange(Annotation::Count)) {
-    if (!mAnnotations[key].IsEmpty()) {
-      if (!writer.WriteAnnotation(key, mAnnotations[key])) {
-        return;
-      }
-    }
-  }
-}
-
-// Helper class to iterate over metadata entries encoded in shmem.
-class MOZ_STACK_CLASS MetadataShmemReader {
- public:
-  explicit MetadataShmemReader(const Shmem& aShmem)
-      : mEntryType(EntryType::None) {
-    mCursor = aShmem.get<uint8_t>();
-    mEnd = mCursor + aShmem.Size<uint8_t>();
-
-    // Advance to the first item, if any.
-    Next();
-  }
-
-  bool Done() const { return mCursor >= mEnd || Type() == EntryType::None; }
-  EntryType Type() const { return mEntryType; }
-  void Next() {
-    if (mCursor < mEnd) {
-      mEntryType = EntryType(*mCursor++);
-    } else {
-      mEntryType = EntryType::None;
-    }
-  }
-
-  template <typename T>
-  bool Read(T* aOut) {
-    return Read(aOut, sizeof(T));
-  }
-
-  bool Read(nsCString& aOut) {
-    uint32_t length = 0;
-    if (!Read(&length)) {
-      return false;
-    }
-
-    const uint8_t* src = Read(length);
-    if (!src) {
-      return false;
-    }
-
-    aOut.Assign((const char*)src, length);
-    return true;
-  }
-
- private:
-  bool Read(void* aOut, size_t aLength) {
-    const uint8_t* src = Read(aLength);
-    if (!src) {
-      return false;
-    }
-    memcpy(aOut, src, aLength);
-    return true;
-  }
-
-  // If buffer has |aLength| bytes, return cursor and then advance it.
-  // Otherwise, return null.
-  const uint8_t* Read(size_t aLength) {
-    if (size_t(mEnd - mCursor) < aLength) {
-      return nullptr;
-    }
-    const uint8_t* result = mCursor;
-    mCursor += aLength;
-    return result;
-  }
-
- private:
-  const uint8_t* mCursor;
-  const uint8_t* mEnd;
-  EntryType mEntryType;
-};
-
-void CrashReporterMetadataShmem::ReadAppNotes(const Shmem& aShmem,
-                                              AnnotationTable& aNotes) {
-  for (MetadataShmemReader reader(aShmem); !reader.Done(); reader.Next()) {
-    switch (reader.Type()) {
-      case EntryType::Annotation: {
-        Annotation key;
-        nsCString value;
-        if (!reader.Read(&key) || !reader.Read(value)) {
-          return;
-        }
-
-        if (key >= Annotation::Count) {
-          return;
-        }
-
-        aNotes[key] = value;
-        break;
-      }
-      default:
-        NS_ASSERTION(false, "Unknown metadata entry type");
-        break;
-    }
-  }
-}
-
-}  // namespace ipc
-}  // namespace mozilla

ipc/glue/CrashReporterMetadataShmem.h

@@ -1,47 +0,0 @@
-/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* vim: set ts=8 sts=2 et sw=2 tw=80: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#ifndef mozilla_ipc_CrashReporterMetadataShmem_h
-#define mozilla_ipc_CrashReporterMetadataShmem_h
-
-#include <stdint.h>
-#include "mozilla/ipc/Shmem.h"
-#include "nsExceptionHandler.h"
-#include "nsString.h"
-
-namespace mozilla {
-namespace ipc {
-
-class CrashReporterMetadataShmem {
-  typedef mozilla::ipc::Shmem Shmem;
-  typedef CrashReporter::AnnotationTable AnnotationTable;
-
- public:
-  explicit CrashReporterMetadataShmem(const Shmem& aShmem);
-  ~CrashReporterMetadataShmem();
-
-  // Metadata writers. These must only be called in child processes.
-  void AnnotateCrashReport(CrashReporter::Annotation aKey,
-                           const nsACString& aData);
-  void AppendAppNotes(const nsACString& aData);
-
-  static void ReadAppNotes(const Shmem& aShmem,
-                           CrashReporter::AnnotationTable& aNotes);
-
- private:
-  void SyncNotesToShmem();
-
- private:
-  Shmem mShmem;
-
-  AnnotationTable mAnnotations;
-  nsCString mAppNotes;
-};
-
-}  // namespace ipc
-}  // namespace mozilla
-
-#endif  // mozilla_ipc_CrashReporterMetadataShmem_h

ipc/glue/IdleSchedulerChild.cpp

@@ -6,6 +6,7 @@
 
 #include "mozilla/ipc/IdleSchedulerChild.h"
 #include "mozilla/ipc/IdleSchedulerParent.h"
+#include "mozilla/ipc/PBackgroundChild.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/IdlePeriodState.h"
 #include "BackgroundChild.h"

ipc/glue/moz.build

@@ -19,7 +19,6 @@ EXPORTS.mozilla.ipc += [
     'CrashReporterClient.h',
     'CrashReporterHelper.h',
     'CrashReporterHost.h',
-    'CrashReporterMetadataShmem.h',
     'CrossProcessMutex.h',
     'CrossProcessSemaphore.h',
     'EnvironmentMap.h',
@@ -152,7 +151,6 @@ UNIFIED_SOURCES += [
     'BrowserProcessSubThread.cpp',
     'CrashReporterClient.cpp',
     'CrashReporterHost.cpp',
-    'CrashReporterMetadataShmem.cpp',
     'FileDescriptor.cpp',
     'FileDescriptorUtils.cpp',
     'IdleSchedulerChild.cpp',

netwerk/ipc/PSocketProcess.ipdl

@@ -50,7 +50,7 @@ protocol PSocketProcess
   manages PAltSvcTransaction;
 
 parent:
-  async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
+  async InitCrashReporter(NativeThreadId threadId);
   async AddMemoryReport(MemoryReport aReport);
   async FinishMemoryReport(uint32_t aGeneration);
   // Messages for sending telemetry to parent process.

netwerk/ipc/SocketProcessParent.h

@@ -19,10 +19,6 @@ class MemoryReport;
 class MemoryReportRequestHost;
 }  // namespace dom
 
-namespace ipc {
-class CrashReporterHost;
-}  // namespace ipc
-
 namespace net {
 
 class SocketProcessHost;

security/sandbox/win/src/remotesandboxbroker/PRemoteSandboxBroker.ipdl

@@ -26,7 +26,7 @@ struct LaunchParameters {
 intr protocol PRemoteSandboxBroker
 {
 parent:
-  async InitCrashReporter(Shmem shmem, NativeThreadId threadId);
+  async InitCrashReporter(NativeThreadId threadId);
 child:
   intr LaunchApp(LaunchParameters params)
     returns (bool ok, uint64_t handle);

toolkit/crashreporter/nsDummyExceptionHandler.cpp

@@ -39,8 +39,6 @@ nsresult SetupExtraData(nsIFile* aAppDataDirectory,
 
 nsresult UnsetExceptionHandler() { return NS_ERROR_NOT_IMPLEMENTED; }
 
-void NotifyCrashReporterClientCreated() {}
-
 nsresult AnnotateCrashReport(Annotation key, bool data) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }

toolkit/crashreporter/nsExceptionHandler.cpp

@@ -24,7 +24,6 @@
 #include "mozilla/StaticMutex.h"
 #include "mozilla/SyncRunnable.h"
 #include "mozilla/TimeStamp.h"
-#include "mozilla/ipc/CrashReporterClient.h"
 
 #include "nsThreadUtils.h"
 #include "nsThread.h"
@@ -122,7 +121,6 @@ using google_breakpad::kDefaultBuildIdSize;
 using google_breakpad::PageAllocator;
 #endif
 using namespace mozilla;
-using mozilla::ipc::CrashReporterClient;
 
 namespace CrashReporter {
 
@@ -312,11 +310,6 @@ class ReportInjectedCrash : public Runnable {
 };
 #endif  // MOZ_CRASHREPORTER_INJECTOR
 
-// If annotations are attempted before the crash reporter is enabled,
-// they queue up here.
-class DelayedNote;
-nsTArray<UniquePtr<DelayedNote> >* gDelayedAnnotations;
-
 #if defined(XP_WIN)
 // the following are used to prevent other DLLs reverting the last chance
 // exception handler to the windows default. Any attempt to change the
@@ -1303,16 +1296,21 @@ static void WriteMozCrashReason(AnnotationWriter& aWriter) {
   }
 }
 
-static void WriteAnnotationsForMainProcessCrash(PlatformWriter& pw,
-                                                const phc::AddrInfo* addrInfo,
-                                                time_t crashTime) {
-  JSONAnnotationWriter writer(pw);
+static void WriteAnnotations(AnnotationWriter& writer,
+                             const AnnotationTable& aAnnotations) {
   for (auto key : MakeEnumeratedRange(Annotation::Count)) {
-    const nsCString& value = crashReporterAPIData_Table[key];
+    const nsCString& value = aAnnotations[key];
     if (!value.IsEmpty()) {
       writer.Write(key, value.get(), value.Length());
     }
   }
+}
+
+static void WriteAnnotationsForMainProcessCrash(PlatformWriter& pw,
+                                                const phc::AddrInfo* addrInfo,
+                                                time_t crashTime) {
+  JSONAnnotationWriter writer(pw);
+  WriteAnnotations(writer, crashReporterAPIData_Table);
 
   char crashTimeString[32];
   XP_TTOA(crashTime, crashTimeString);
@@ -1677,6 +1675,8 @@ static void PrepareChildExceptionTimeAnnotations(
     }
   };
   GetFlatThreadAnnotation(getThreadAnnotationCB, true);
+
+  WriteAnnotations(writer, crashReporterAPIData_Table);
 }
 
 #ifdef XP_WIN
@@ -1830,6 +1830,29 @@ static nsresult LocateExecutable(nsIFile* aXREDirectory,
 
 #endif  // !defined(MOZ_WIDGET_ANDROID)
 
+static void InitializeAnnotationFacilities() {
+  crashReporterAPILock = new Mutex("crashReporterAPILock");
+  notesFieldLock = new Mutex("notesFieldLock");
+  notesField = new nsCString();
+  InitThreadAnnotation();
+}
+
+static void TeardownAnnotationFacilities() {
+  std::fill(crashReporterAPIData_Table.begin(),
+            crashReporterAPIData_Table.end(), EmptyCString());
+
+  delete crashReporterAPILock;
+  crashReporterAPILock = nullptr;
+
+  delete notesFieldLock;
+  notesFieldLock = nullptr;
+
+  delete notesField;
+  notesField = nullptr;
+
+  ShutdownThreadAnnotation();
+}
+
 nsresult SetExceptionHandler(nsIFile* aXREDirectory, bool force /*=false*/) {
   if (gExceptionHandler) return NS_ERROR_ALREADY_INITIALIZED;
 
@@ -1849,13 +1872,7 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory, bool force /*=false*/) {
   // the crash reporter client
   doReport = ShouldReport();
 
-  NS_ASSERTION(!crashReporterAPILock, "Shouldn't have a lock yet");
-  crashReporterAPILock = new Mutex("crashReporterAPILock");
-  NS_ASSERTION(!notesFieldLock, "Shouldn't have a lock yet");
-  notesFieldLock = new Mutex("notesFieldLock");
-
-  notesField = new nsCString();
-  NS_ENSURE_TRUE(notesField, NS_ERROR_OUT_OF_MEMORY);
+  InitializeAnnotationFacilities();
 
 #if !defined(MOZ_WIDGET_ANDROID)
   // Locate the crash reporter executable
@@ -2035,8 +2052,6 @@ nsresult SetExceptionHandler(nsIFile* aXREDirectory, bool force /*=false*/) {
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
-  InitThreadAnnotation();
-
   return NS_OK;
 }
 
@@ -2282,19 +2297,7 @@ nsresult UnsetExceptionHandler() {
 
   delete gExceptionHandler;
 
-  // do this here in the unlikely case that we succeeded in allocating
-  // our strings but failed to allocate gExceptionHandler.
-  std::fill(crashReporterAPIData_Table.begin(),
-            crashReporterAPIData_Table.end(), EmptyCString());
-
-  delete crashReporterAPILock;
-  crashReporterAPILock = nullptr;
-
-  delete notesFieldLock;
-  notesFieldLock = nullptr;
-
-  delete notesField;
-  notesField = nullptr;
+  TeardownAnnotationFacilities();
 
   if (pendingDirectory) {
     free(pendingDirectory);
@@ -2323,8 +2326,6 @@ nsresult UnsetExceptionHandler() {
     memoryReportPath = nullptr;
   }
 
-  ShutdownThreadAnnotation();
-
   if (!gExceptionHandler) return NS_ERROR_NOT_INITIALIZED;
 
   gExceptionHandler = nullptr;
@@ -2339,45 +2340,6 @@ nsresult UnsetExceptionHandler() {
   return NS_OK;
 }
 
-class DelayedNote {
- public:
-  DelayedNote(Annotation aKey, const nsACString& aData)
-      : mKey(aKey), mData(aData), mType(CrashAnnotation) {}
-
-  explicit DelayedNote(const nsACString& aData)
-      : mData(aData), mType(AppNote) {}
-
-  void Run() {
-    if (mType == CrashAnnotation) {
-      AnnotateCrashReport(mKey, mData);
-    } else {
-      AppendAppNotesToCrashReport(mData);
-    }
-  }
-
- private:
-  Annotation mKey;
-  nsCString mData;
-  enum AnnotationType { CrashAnnotation, AppNote } mType;
-};
-
-static void EnqueueDelayedNote(DelayedNote* aNote) {
-  if (!gDelayedAnnotations) {
-    gDelayedAnnotations = new nsTArray<UniquePtr<DelayedNote> >();
-  }
-  gDelayedAnnotations->AppendElement(WrapUnique(aNote));
-}
-
-void NotifyCrashReporterClientCreated() {
-  if (gDelayedAnnotations) {
-    for (const auto& note : *gDelayedAnnotations) {
-      note->Run();
-    }
-    delete gDelayedAnnotations;
-    gDelayedAnnotations = nullptr;
-  }
-}
-
 nsresult AnnotateCrashReport(Annotation key, bool data) {
   return AnnotateCrashReport(
       key, data ? NS_LITERAL_CSTRING("1") : NS_LITERAL_CSTRING("0"));
@@ -2400,21 +2362,6 @@ nsresult AnnotateCrashReport(Annotation key, unsigned int data) {
 nsresult AnnotateCrashReport(Annotation key, const nsACString& data) {
   if (!GetEnabled()) return NS_ERROR_NOT_INITIALIZED;
 
-  if (!XRE_IsParentProcess()) {
-    // The newer CrashReporterClient can be used from any thread.
-    if (RefPtr<CrashReporterClient> client =
-            CrashReporterClient::GetSingleton()) {
-      client->AnnotateCrashReport(key, data);
-      return NS_OK;
-    }
-
-    // EnqueueDelayedNote() can only be called on the main thread.
-    MOZ_RELEASE_ASSERT(NS_IsMainThread());
-
-    EnqueueDelayedNote(new DelayedNote(key, data));
-    return NS_OK;
-  }
-
   MutexAutoLock lock(*crashReporterAPILock);
   crashReporterAPIData_Table[key] = data;
 
@@ -2436,10 +2383,10 @@ void MergeCrashAnnotations(AnnotationTable& aDst, const AnnotationTable& aSrc) {
   }
 }
 
-static void MergeContentCrashAnnotations(AnnotationTable& aDst,
-                                         const AnnotationTable& aSrc) {
+static void MergeContentCrashAnnotations(AnnotationTable& aDst) {
+  MutexAutoLock lock(*crashReporterAPILock);
   for (auto key : MakeEnumeratedRange(Annotation::Count)) {
-    const nsCString& value = aSrc[key];
+    const nsCString& value = crashReporterAPIData_Table[key];
     if (value.IsEmpty() || IsAnnotationBlacklistedForContent(key)) {
       continue;
     }
@@ -2485,20 +2432,6 @@ void SetMinidumpAnalysisAllThreads() {
 nsresult AppendAppNotesToCrashReport(const nsACString& data) {
   if (!GetEnabled()) return NS_ERROR_NOT_INITIALIZED;
 
-  if (!XRE_IsParentProcess()) {
-    if (RefPtr<CrashReporterClient> client =
-            CrashReporterClient::GetSingleton()) {
-      client->AppendAppNotes(data);
-      return NS_OK;
-    }
-
-    // EnqueueDelayedNote can only be called on the main thread.
-    MOZ_RELEASE_ASSERT(NS_IsMainThread());
-
-    EnqueueDelayedNote(new DelayedNote(data));
-    return NS_OK;
-  }
-
   MutexAutoLock lock(*notesFieldLock);
 
   notesField->Append(data);
@@ -3156,12 +3089,7 @@ static bool WriteExtraFile(PlatformWriter pw,
   }
 
   JSONAnnotationWriter writer(pw);
-  for (auto key : MakeEnumeratedRange(Annotation::Count)) {
-    const nsCString& value = aAnnotations[key];
-    if (!value.IsEmpty()) {
-      writer.Write(key, value.get(), value.Length());
-    }
-  }
+  WriteAnnotations(writer, aAnnotations);
 
   return true;
 }
@@ -3191,19 +3119,14 @@ static void ReadExceptionTimeAnnotations(AnnotationTable& aAnnotations,
   if (aPid && processToCrashFd.count(aPid)) {
     PRFileDesc* prFd = processToCrashFd[aPid];
     processToCrashFd.erase(aPid);
-    AnnotationTable exceptionTimeAnnotations;
-    ReadAndValidateExceptionTimeAnnotations(prFd, exceptionTimeAnnotations);
-    MergeCrashAnnotations(aAnnotations, exceptionTimeAnnotations);
+    ReadAndValidateExceptionTimeAnnotations(prFd, aAnnotations);
     PR_Close(prFd);
   }
 }
 
 static void PopulateContentProcessAnnotations(AnnotationTable& aAnnotations,
                                               uint32_t aPid) {
-  {
-    MutexAutoLock lock(*crashReporterAPILock);
-    MergeContentCrashAnnotations(aAnnotations, crashReporterAPIData_Table);
-  }
+  MergeContentCrashAnnotations(aAnnotations);
   AddCommonAnnotations(aAnnotations);
   ReadExceptionTimeAnnotations(aAnnotations, aPid);
 }
@@ -3512,6 +3435,8 @@ bool SetRemoteExceptionHandler(const char* aCrashPipe,
                                uintptr_t aCrashTimeAnnotationFile) {
   MOZ_ASSERT(!gExceptionHandler, "crash client already init'd");
 
+  InitializeAnnotationFacilities();
+
 #if defined(XP_WIN)
   gExceptionHandler = new google_breakpad::ExceptionHandler(
       L"", ChildFPEFilter,
@@ -3547,8 +3472,6 @@ bool SetRemoteExceptionHandler(const char* aCrashPipe,
 
   oldTerminateHandler = std::set_terminate(&TerminateHandler);
 
-  InitThreadAnnotation();
-
   // we either do remote or nothing, no fallback to regular crash reporting
   return gExceptionHandler->IsOutOfProcess();
 }
@@ -3822,12 +3745,7 @@ bool CreateMinidumpsAndPair(ProcessHandle aTargetPid,
   DllBlocklist_Shutdown();
 #endif
 
-  {
-    MutexAutoLock lock(*crashReporterAPILock);
-    MergeContentCrashAnnotations(aTargetAnnotations,
-                                 crashReporterAPIData_Table);
-  }
-
+  MergeContentCrashAnnotations(aTargetAnnotations);
   AddCommonAnnotations(aTargetAnnotations);
 
   targetMinidump.forget(aMainDumpOut);
@@ -3881,7 +3799,8 @@ bool UnsetRemoteExceptionHandler() {
   delete gExceptionHandler;
   gExceptionHandler = nullptr;
 #endif
-  ShutdownThreadAnnotation();
+  TeardownAnnotationFacilities();
+
   return true;
 }
 

toolkit/crashreporter/nsExceptionHandler.h

@@ -97,10 +97,6 @@ nsresult AnnotateCrashReport(Annotation key, const nsACString& data);
 nsresult RemoveCrashReportAnnotation(Annotation key);
 nsresult AppendAppNotesToCrashReport(const nsACString& data);
 
-// Called after the crash reporter client has been created in a content
-// process, allowing annotations to be processed.
-void NotifyCrashReporterClientCreated();
-
 void AnnotateOOMAllocationSize(size_t size);
 void AnnotateTexturesSize(size_t size);
 nsresult SetGarbageCollecting(bool collecting);