From b3a7ad66b1438c319577715698d55d8855bed8da Mon Sep 17 00:00:00 2001 From: Noemi Erli Date: Tue, 25 Feb 2020 20:10:35 +0200 Subject: [PATCH] Backed out 4 changesets (bug 1589493) for causing lint failure in test_process_error_oom.xhtml Backed out changeset 9dbe0bdd321b (bug 1589493) Backed out changeset 900ec6b447c9 (bug 1589493) Backed out changeset 3f7c15d29416 (bug 1589493) Backed out changeset c1bc7695e720 (bug 1589493) --- dom/ipc/ContentParent.cpp | 3 -- dom/ipc/tests/chrome.ini | 5 --- dom/ipc/tests/process_error.xhtml | 10 +---- dom/ipc/tests/test_process_error_oom.xhtml | 22 ----------- ipc/glue/CrashReporterHost.cpp | 17 -------- ipc/glue/CrashReporterHost.h | 8 ---- .../BrowserTestUtils/BrowserTestUtils.jsm | 11 +----- .../BrowserTestUtilsChild.jsm | 39 +++---------------- 8 files changed, 8 insertions(+), 107 deletions(-) delete mode 100644 dom/ipc/tests/test_process_error_oom.xhtml diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 9be887d0fe6a..b028f28958bc 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1786,9 +1786,6 @@ void ContentParent::ActorDestroy(ActorDestroyReason why) { if (!dumpID.IsEmpty()) { props->SetPropertyAsAString(NS_LITERAL_STRING("dumpID"), dumpID); } - // Propagate `isLikelyOOM`. - Unused << props->SetPropertyAsBool(NS_LITERAL_STRING("isLikelyOOM"), - mCrashReporter->IsLikelyOOM()); } nsAutoString cpId; cpId.AppendInt(static_cast(this->ChildID())); diff --git a/dom/ipc/tests/chrome.ini b/dom/ipc/tests/chrome.ini index f6aa63a078b4..1f0bc2995445 100644 --- a/dom/ipc/tests/chrome.ini +++ b/dom/ipc/tests/chrome.ini @@ -5,8 +5,3 @@ support-files = [test_process_error.xhtml] skip-if = !crashreporter - - -[test_process_error_oom.xhtml] -skip-if = !crashreporter - diff --git a/dom/ipc/tests/process_error.xhtml b/dom/ipc/tests/process_error.xhtml index a017508b9482..a074f33b6def 100644 --- a/dom/ipc/tests/process_error.xhtml +++ b/dom/ipc/tests/process_error.xhtml @@ -14,10 +14,6 @@ const done = window.arguments[0].done; const SimpleTest = window.arguments[0].SimpleTest; - // Parse test options. - const url = new URL(document.location); - const crashType = url.searchParams.get("crashType"); - // Allow the browser to get connected before using the messageManager to cause // a crash: addEventListener("DOMContentLoaded", () => { @@ -33,10 +29,6 @@ if ('nsICrashReporter' in Ci) { dumpID = subject.getPropertyAsAString('dumpID'); ok(dumpID, "dumpID is present and not an empty string"); - - // Let's check whether we have correctly reported OOM. - var isLikelyOOM = subject.getPropertyAsBool('isLikelyOOM'); - is(isLikelyOOM, crashType == 'CRASH_OOM', 'isLikelyOOM is correct'); } Services.obs.removeObserver(crashObserver, 'ipc:content-shutdown'); @@ -54,7 +46,7 @@ "Expected the right browsing context id on the oop-browser-crashed event."); }) - BrowserTestUtils.crashFrame(browser, true, false, /* Default browsing context */ null, { crashType }); + BrowserTestUtils.crashFrame(browser, true, false); Promise.all([observerPromise, eventPromise]).then(done); }); diff --git a/dom/ipc/tests/test_process_error_oom.xhtml b/dom/ipc/tests/test_process_error_oom.xhtml deleted file mode 100644 index 0a04565f0416..000000000000 --- a/dom/ipc/tests/test_process_error_oom.xhtml +++ /dev/null @@ -1,22 +0,0 @@ - - - - - - - - - diff --git a/ipc/glue/CrashReporterHost.cpp b/ipc/glue/CrashReporterHost.cpp index da44d3cb0917..d37cfc73d605 100644 --- a/ipc/glue/CrashReporterHost.cpp +++ b/ipc/glue/CrashReporterHost.cpp @@ -8,7 +8,6 @@ #include "CrashReporterMetadataShmem.h" #include "mozilla/dom/Promise.h" #include "mozilla/recordreplay/ParentIPC.h" -#include "mozilla/EnumeratedRange.h" #include "mozilla/Sprintf.h" #include "mozilla/SyncRunnable.h" #include "mozilla/Telemetry.h" @@ -242,21 +241,5 @@ void CrashReporterHost::AddAnnotation(CrashReporter::Annotation aKey, mExtraAnnotations[aKey] = aValue; } -bool CrashReporterHost::IsLikelyOOM() { - // The data is only populated during the call to `FinalizeCrashReport()`. - MOZ_ASSERT(mFinalized); - if (mExtraAnnotations[CrashReporter::Annotation::OOMAllocationSize].Length() > - 0) { - // If `OOMAllocationSize` was set, we know that the crash happened - // because an allocation failed (`malloc` returned `nullptr`). - // - // As Unix systems generally allow `malloc` to return a non-null value - // even when no virtual memory is available, this doesn't cover all - // cases of OOM under Unix (far from it). - return true; - } - return false; -} - } // namespace ipc } // namespace mozilla diff --git a/ipc/glue/CrashReporterHost.h b/ipc/glue/CrashReporterHost.h index dc08cb1a984a..e271adfef31a 100644 --- a/ipc/glue/CrashReporterHost.h +++ b/ipc/glue/CrashReporterHost.h @@ -94,14 +94,6 @@ class CrashReporterHost { return mExtraAnnotations[CrashReporter::Annotation::additional_minidumps]; } - // Return `true` if this crash reporter has been identified as a likely OOM. - // - // At the time of this writing, OOMs detection is considered reliable under - // Windows but other platforms quite often return false negatives. - // - // `CrashReporterHost::FinalizeCrashReport()` MUST have been called already. - bool IsLikelyOOM(); - // This is a static helper function to notify the crash service that a // crash has occurred and record the crash with telemetry. This can be called // from any thread, and if not called from the main thread, will post a diff --git a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm index 006366b9ef39..313067949695 100644 --- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm +++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm @@ -1628,10 +1628,6 @@ var BrowserTestUtils = { * @param (BrowsingContext) browsingContext * The context where the frame leaves. Default to * top level context if not supplied. - * @param (object?) options - * An object with any of the following fields: - * crashType: "CRASH_INVALID_POINTER_DEREF" | "CRASH_OOM" - * The type of crash. If unspecified, default to "CRASH_INVALID_POINTER_DEREF" * * @returns (Promise) * @resolves An Object with key-value pairs representing the data from the @@ -1641,8 +1637,7 @@ var BrowserTestUtils = { browser, shouldShowTabCrashPage = true, shouldClearMinidumps = true, - browsingContext, - options = {} + browsingContext ) { let extra = {}; @@ -1777,9 +1772,7 @@ var BrowserTestUtils = { this.sendAsyncMessage( browsingContext || browser.browsingContext, "BrowserTestUtils:CrashFrame", - { - crashType: options.crashType || "", - } + {} ); await Promise.all(expectedPromises); diff --git a/testing/mochitest/BrowserTestUtils/BrowserTestUtilsChild.jsm b/testing/mochitest/BrowserTestUtils/BrowserTestUtilsChild.jsm index 1c0b4017bdd2..6e307e0fb350 100644 --- a/testing/mochitest/BrowserTestUtils/BrowserTestUtilsChild.jsm +++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtilsChild.jsm @@ -210,8 +210,8 @@ class BrowserTestUtilsChild extends JSWindowActorChild { case "BrowserTestUtils:CrashFrame": { // This is to intentionally crash the frame. - // We crash by using js-ctypes. The crash - // should happen immediately + // We crash by using js-ctypes and dereferencing + // a bad pointer. The crash should happen immediately // upon loading this frame script. const { ctypes } = ChromeUtils.import( @@ -220,38 +220,9 @@ class BrowserTestUtilsChild extends JSWindowActorChild { let dies = function() { ChromeUtils.privateNoteIntentionalCrash(); - - switch (aMessage.data.crashType) { - case "CRASH_OOM": { - // Allocate waaaaaay too much memory to encourage the system - // to crash with an OOM. - const OS = ChromeUtils.import( - "resource://gre/modules/osfile/osfile_shared_allthreads.jsm" - ); - let libxul = ctypes.open(OS.Constants.Path.libxul); - let moz_xmalloc = libxul.declare( - "moz_xmalloc", - ctypes.default_abi, - /* return type */ ctypes.voidptr_t, - /* size */ ctypes.size_t - ); - let max_value = ctypes.cast(ctypes.ssize_t(-1), ctypes.size_t); - moz_xmalloc(max_value); - moz_xmalloc(max_value); - moz_xmalloc(max_value); - break; - } - case "CRASH_INVALID_POINTER_DEREF": // Fallthrough - default: { - // Dereference a bad pointer. - let zero = new ctypes.intptr_t(8); - let badptr = ctypes.cast( - zero, - ctypes.PointerType(ctypes.int32_t) - ); - badptr.contents; - } - } + let zero = new ctypes.intptr_t(8); + let badptr = ctypes.cast(zero, ctypes.PointerType(ctypes.int32_t)); + badptr.contents; }; dump("\nEt tu, Brute?\n");