From 855f08b5d6f1299386964507a8fd9d5b7203c21b Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Fri, 24 Mar 2017 10:34:44 -0700 Subject: [PATCH] Bug 1350435 - Compute snapshot ID in the parent process. r=fitzgen,smaug The parent and content processes can have different temp directories when sandboxing is enabled, so the process that creates the file for a heap snapshot must also determine the snapshot ID. MozReview-Commit-ID: 2UuncT54NXc --HG-- extra : rebase_source : 350e49bf7c570abfdde457a89ee8922f8cdb8b7d --- devtools/server/performance/memory.js | 3 +- devtools/shared/heapsnapshot/HeapSnapshot.cpp | 57 ++++++++++++++++--- devtools/shared/heapsnapshot/HeapSnapshot.h | 3 +- .../heapsnapshot/HeapSnapshotFileUtils.js | 12 ---- .../HeapSnapshotTempFileHelperParent.cpp | 6 +- .../PHeapSnapshotTempFileHelper.ipdl | 1 + dom/base/ChromeUtils.h | 14 +++++ dom/webidl/ThreadSafeChromeUtils.webidl | 10 ++++ 8 files changed, 80 insertions(+), 26 deletions(-) diff --git a/devtools/server/performance/memory.js b/devtools/server/performance/memory.js index b69160c7d817..89ca919931e9 100644 --- a/devtools/server/performance/memory.js +++ b/devtools/server/performance/memory.js @@ -155,8 +155,7 @@ exports.Memory = Class({ boundaries = { debugger: this.dbg }; } } - const path = ThreadSafeChromeUtils.saveHeapSnapshot(boundaries); - return HeapSnapshotFileUtils.getSnapshotIdFromPath(path); + return ThreadSafeChromeUtils.saveHeapSnapshotGetId(boundaries); }, "saveHeapSnapshot"), /** diff --git a/devtools/shared/heapsnapshot/HeapSnapshot.cpp b/devtools/shared/heapsnapshot/HeapSnapshot.cpp index e9b1e1166482..82c6b4d92935 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp +++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp @@ -1444,13 +1444,19 @@ msSinceProcessCreation(const TimeStamp& now) /* static */ already_AddRefed HeapSnapshot::CreateUniqueCoreDumpFile(ErrorResult& rv, const TimeStamp& now, - nsAString& outFilePath) + nsAString& outFilePath, + nsAString& outSnapshotId) { nsCOMPtr file; rv = NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(file)); if (NS_WARN_IF(rv.Failed())) return nullptr; + nsAutoString tempPath; + rv = file->GetPath(tempPath); + if (NS_WARN_IF(rv.Failed())) + return nullptr; + auto ms = msSinceProcessCreation(now); rv = file->AppendNative(nsPrintfCString("%lu.fxsnapshot", ms)); if (NS_WARN_IF(rv.Failed())) @@ -1462,7 +1468,12 @@ HeapSnapshot::CreateUniqueCoreDumpFile(ErrorResult& rv, rv = file->GetPath(outFilePath); if (NS_WARN_IF(rv.Failed())) - return nullptr; + return nullptr; + + // The snapshot ID must be computed in the process that created the + // temp file, because TmpD may not be the same in all processes. + outSnapshotId.Assign(Substring(outFilePath, tempPath.Length() + 1, + outFilePath.Length() - tempPath.Length() - sizeof(".fxsnapshot"))); return file.forget(); } @@ -1489,14 +1500,18 @@ using UniqueHeapSnapshotTempFileHelperChild = UniquePtr -getCoreDumpOutputStream(ErrorResult& rv, TimeStamp& start, nsAString& outFilePath) +getCoreDumpOutputStream(ErrorResult& rv, + TimeStamp& start, + nsAString& outFilePath, + nsAString& outSnapshotId) { if (XRE_IsParentProcess()) { // Create the file and open the output stream directly. nsCOMPtr file = HeapSnapshot::CreateUniqueCoreDumpFile(rv, start, - outFilePath); + outFilePath, + outSnapshotId); if (NS_WARN_IF(rv.Failed())) return nullptr; @@ -1535,6 +1550,7 @@ getCoreDumpOutputStream(ErrorResult& rv, TimeStamp& start, nsAString& outFilePat auto opened = response.get_OpenedFile(); outFilePath = opened.path(); + outSnapshotId = opened.snapshotId(); nsCOMPtr outputStream = FileDescriptorOutputStream::Create(opened.descriptor()); if (NS_WARN_IF(!outputStream)) { @@ -1553,10 +1569,11 @@ using namespace JS; using namespace devtools; /* static */ void -ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, - const HeapSnapshotBoundaries& boundaries, - nsAString& outFilePath, - ErrorResult& rv) +ThreadSafeChromeUtils::SaveHeapSnapshotShared(GlobalObject& global, + const HeapSnapshotBoundaries& boundaries, + nsAString& outFilePath, + nsAString& outSnapshotId, + ErrorResult& rv) { auto start = TimeStamp::Now(); @@ -1565,7 +1582,9 @@ ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, uint32_t nodeCount = 0; uint32_t edgeCount = 0; - nsCOMPtr outputStream = getCoreDumpOutputStream(rv, start, outFilePath); + nsCOMPtr outputStream = getCoreDumpOutputStream(rv, start, + outFilePath, + outSnapshotId); if (NS_WARN_IF(rv.Failed())) return; @@ -1618,6 +1637,26 @@ ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, edgeCount); } +/* static */ void +ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, + const HeapSnapshotBoundaries& boundaries, + nsAString& outFilePath, + ErrorResult& rv) +{ + nsAutoString snapshotId; + SaveHeapSnapshotShared(global, boundaries, outFilePath, snapshotId, rv); +} + +/* static */ void +ThreadSafeChromeUtils::SaveHeapSnapshotGetId(GlobalObject& global, + const HeapSnapshotBoundaries& boundaries, + nsAString& outSnapshotId, + ErrorResult& rv) +{ + nsAutoString filePath; + SaveHeapSnapshotShared(global, boundaries, filePath, outSnapshotId, rv); +} + /* static */ already_AddRefed ThreadSafeChromeUtils::ReadHeapSnapshot(GlobalObject& global, const nsAString& filePath, diff --git a/devtools/shared/heapsnapshot/HeapSnapshot.h b/devtools/shared/heapsnapshot/HeapSnapshot.h index 0428033f626a..5e179ab0df61 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshot.h +++ b/devtools/shared/heapsnapshot/HeapSnapshot.h @@ -126,7 +126,8 @@ public: // snapshots are serialized into. static already_AddRefed CreateUniqueCoreDumpFile(ErrorResult& rv, const TimeStamp& now, - nsAString& outFilePath); + nsAString& outFilePath, + nsAString& outSnapshotId); NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(HeapSnapshot) diff --git a/devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js b/devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js index abd44fc30934..421176df916e 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js +++ b/devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js @@ -81,15 +81,3 @@ exports.haveHeapSnapshotTempFile = function (snapshotId) { return OS.File.stat(path).then(() => true, () => false); }; - -/** - * Given a heap snapshot's file path, extricate the snapshot id. - * - * @param {String} path - * - * @returns String - */ -exports.getSnapshotIdFromPath = function (path) { - return path.slice(OS.Constants.Path.tmpDir.length + 1, - path.length - ".fxsnapshot".length); -}; diff --git a/devtools/shared/heapsnapshot/HeapSnapshotTempFileHelperParent.cpp b/devtools/shared/heapsnapshot/HeapSnapshotTempFileHelperParent.cpp index 83f70b558a85..0c804984c754 100644 --- a/devtools/shared/heapsnapshot/HeapSnapshotTempFileHelperParent.cpp +++ b/devtools/shared/heapsnapshot/HeapSnapshotTempFileHelperParent.cpp @@ -31,9 +31,11 @@ HeapSnapshotTempFileHelperParent::RecvOpenHeapSnapshotTempFile( auto start = TimeStamp::Now(); ErrorResult rv; nsAutoString filePath; + nsAutoString snapshotId; nsCOMPtr file = HeapSnapshot::CreateUniqueCoreDumpFile(rv, start, - filePath); + filePath, + snapshotId); if (NS_WARN_IF(rv.Failed())) { if (!openFileFailure(rv, outResponse)) { return IPC_FAIL_NO_REASON(this); @@ -53,7 +55,7 @@ HeapSnapshotTempFileHelperParent::RecvOpenHeapSnapshotTempFile( FileDescriptor::PlatformHandleType handle = FileDescriptor::PlatformHandleType(PR_FileDesc2NativeHandle(prfd)); FileDescriptor fd(handle); - *outResponse = OpenedFile(filePath, fd); + *outResponse = OpenedFile(filePath, snapshotId, fd); return IPC_OK(); } diff --git a/devtools/shared/heapsnapshot/PHeapSnapshotTempFileHelper.ipdl b/devtools/shared/heapsnapshot/PHeapSnapshotTempFileHelper.ipdl index 2576470e255f..6a2a8c9933a1 100644 --- a/devtools/shared/heapsnapshot/PHeapSnapshotTempFileHelper.ipdl +++ b/devtools/shared/heapsnapshot/PHeapSnapshotTempFileHelper.ipdl @@ -12,6 +12,7 @@ namespace devtools { struct OpenedFile { nsString path; + nsString snapshotId; FileDescriptor descriptor; }; diff --git a/dom/base/ChromeUtils.h b/dom/base/ChromeUtils.h index 543beadbf40b..77223638bd1e 100644 --- a/dom/base/ChromeUtils.h +++ b/dom/base/ChromeUtils.h @@ -26,6 +26,14 @@ class Promise; class ThreadSafeChromeUtils { +private: + // Implemented in devtools/shared/heapsnapshot/HeapSnapshot.cpp + static void SaveHeapSnapshotShared(GlobalObject& global, + const HeapSnapshotBoundaries& boundaries, + nsAString& filePath, + nsAString& snapshotId, + ErrorResult& rv); + public: // Implemented in devtools/shared/heapsnapshot/HeapSnapshot.cpp static void SaveHeapSnapshot(GlobalObject& global, @@ -33,6 +41,12 @@ public: nsAString& filePath, ErrorResult& rv); + // Implemented in devtools/shared/heapsnapshot/HeapSnapshot.cpp + static void SaveHeapSnapshotGetId(GlobalObject& global, + const HeapSnapshotBoundaries& boundaries, + nsAString& snapshotId, + ErrorResult& rv); + // Implemented in devtools/shared/heapsnapshot/HeapSnapshot.cpp static already_AddRefed ReadHeapSnapshot(GlobalObject& global, const nsAString& filePath, diff --git a/dom/webidl/ThreadSafeChromeUtils.webidl b/dom/webidl/ThreadSafeChromeUtils.webidl index 8d0f21734c0e..80484a90e3be 100644 --- a/dom/webidl/ThreadSafeChromeUtils.webidl +++ b/dom/webidl/ThreadSafeChromeUtils.webidl @@ -24,6 +24,16 @@ interface ThreadSafeChromeUtils { [Throws] static DOMString saveHeapSnapshot(optional HeapSnapshotBoundaries boundaries); + /** + * This is the same as saveHeapSnapshot, but with a different return value. + * + * @returns The snapshot ID of the file. This is the file name + * without the temp directory or the trailing + * `.fxsnapshot`. + */ + [Throws] + static DOMString saveHeapSnapshotGetId(optional HeapSnapshotBoundaries boundaries); + /** * Deserialize a core dump into a HeapSnapshot. *