Bug 1477129: Part 2 - Fix SharedMap blob handling, and add tests. r?erahm
We were previously failing to send blobs to new content processes, which was a
problem for those processes. But we were also attempting to extract blobs for
new entries that we were serializing after we'd extracted their structured
clone data, and their blob array had been thrown away (which was a problem for
all processes).
This patch fixes both problems.
MozReview-Commit-ID: 3qbAmUTA85g
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -588,33 +588,28 @@ NS_INTERFACE_MAP_BEGIN(ContentChild)
NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIContentChild)
NS_INTERFACE_MAP_END
mozilla::ipc::IPCResult
ContentChild::RecvSetXPCOMProcessAttributes(const XPCOMInitData& aXPCOMInit,
const StructuredCloneData& aInitialData,
nsTArray<LookAndFeelInt>&& aLookAndFeelIntCache,
- nsTArray<SystemFontListEntry>&& aFontList,
- const FileDescriptor& aSharedDataMapFile,
- const uint32_t& aSharedDataMapSize)
+ nsTArray<SystemFontListEntry>&& aFontList)
{
if (!sShutdownCanary) {
return IPC_OK();
}
mLookAndFeelCache = std::move(aLookAndFeelIntCache);
mFontList = std::move(aFontList);
gfx::gfxVars::SetValuesForInitialize(aXPCOMInit.gfxNonDefaultVarUpdates());
InitXPCOM(aXPCOMInit, aInitialData);
InitGraphicsDeviceData(aXPCOMInit.contentDeviceData());
- mSharedData = new SharedMap(ProcessGlobal::Get(), aSharedDataMapFile,
- aSharedDataMapSize);
-
return IPC_OK();
}
bool
ContentChild::Init(MessageLoop* aIOLoop,
base::ProcessId aParentPid,
const char* aParentBuildID,
IPC::Channel* aChannel,
@@ -2568,25 +2563,28 @@ ContentChild::RecvRegisterStringBundles(
}
mozilla::ipc::IPCResult
ContentChild::RecvUpdateSharedData(const FileDescriptor& aMapFile,
const uint32_t& aMapSize,
nsTArray<IPCBlob>&& aBlobs,
nsTArray<nsCString>&& aChangedKeys)
{
+ nsTArray<RefPtr<BlobImpl>> blobImpls(aBlobs.Length());
+ for (auto& ipcBlob : aBlobs) {
+ blobImpls.AppendElement(IPCBlobUtils::Deserialize(ipcBlob));
+ }
+
if (mSharedData) {
- nsTArray<RefPtr<BlobImpl>> blobImpls(aBlobs.Length());
- for (auto& ipcBlob : aBlobs) {
- blobImpls.AppendElement(IPCBlobUtils::Deserialize(ipcBlob));
- }
-
mSharedData->Update(aMapFile, aMapSize,
std::move(blobImpls),
std::move(aChangedKeys));
+ } else {
+ mSharedData = new SharedMap(ProcessGlobal::Get(), aMapFile,
+ aMapSize, std::move(blobImpls));
}
return IPC_OK();
}
mozilla::ipc::IPCResult
ContentChild::RecvGeolocationUpdate(nsIDOMGeoPosition* aPosition)
{
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@@ -621,19 +621,17 @@ public:
const bool& anonymize,
const bool& minimizeMemoryUsage,
const MaybeFileDesc& DMDFile) override;
virtual mozilla::ipc::IPCResult
RecvSetXPCOMProcessAttributes(const XPCOMInitData& aXPCOMInit,
const StructuredCloneData& aInitialData,
nsTArray<LookAndFeelInt>&& aLookAndFeelIntCache,
- nsTArray<SystemFontListEntry>&& aFontList,
- const FileDescriptor& aSharedDataMapFile,
- const uint32_t& aSharedDataMapSize) override;
+ nsTArray<SystemFontListEntry>&& aFontList) override;
virtual mozilla::ipc::IPCResult
RecvProvideAnonymousTemporaryFile(const uint64_t& aID, const FileDescOrError& aFD) override;
mozilla::ipc::IPCResult
RecvSetPermissionsWithKey(const nsCString& aPermissionKey,
nsTArray<IPC::Permission>&& aPerms) override;
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -2334,22 +2334,22 @@ ContentParent::InitInternal(ProcessPrior
// Send the dynamic scalar definitions to the new process.
TelemetryIPC::GetDynamicScalarDefinitions(xpcomInit.dynamicScalarDefs());
// Must send screen info before send initialData
ScreenManager& screenManager = ScreenManager::GetSingleton();
screenManager.CopyScreensToRemote(this);
+ Unused << SendSetXPCOMProcessAttributes(xpcomInit, initialData, lnfCache,
+ fontList);
+
ipc::WritableSharedMap* sharedData = nsFrameMessageManager::sParentProcessManager->SharedData();
sharedData->Flush();
-
- Unused << SendSetXPCOMProcessAttributes(xpcomInit, initialData, lnfCache,
- fontList, sharedData->CloneMapFile(),
- sharedData->MapSize());
+ sharedData->SendTo(this);
nsCOMPtr<nsIChromeRegistry> registrySvc = nsChromeRegistry::GetService();
nsChromeRegistryChrome* chromeRegistry =
static_cast<nsChromeRegistryChrome*>(registrySvc.get());
chromeRegistry->SendRegisteredChrome(this);
nsCOMPtr<nsIStringBundleService> stringBundleService =
services::GetStringBundleService();
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -512,19 +512,17 @@ child:
* Send BlobURLRegistrationData to child process.
*/
async InitBlobURLs(BlobURLRegistrationData[] registrations);
async SetXPCOMProcessAttributes(XPCOMInitData xpcomInit,
StructuredCloneData initialData,
LookAndFeelInt[] lookAndFeelIntCache,
/* used on MacOSX and Linux only: */
- SystemFontListEntry[] systemFontList,
- FileDescriptor sharedDataMapFile,
- uint32_t sharedDataMapSize);
+ SystemFontListEntry[] systemFontList);
// Notify child that last-pb-context-exited notification was observed
async LastPrivateDocShellDestroyed();
async NotifyProcessPriorityChanged(ProcessPriority priority);
async MinimizeMemoryUsage();
/**
--- a/dom/ipc/SharedMap.cpp
+++ b/dom/ipc/SharedMap.cpp
@@ -38,18 +38,19 @@ AlignTo(size_t* aOffset, size_t aAlign)
}
SharedMap::SharedMap()
: DOMEventTargetHelper()
{}
SharedMap::SharedMap(nsIGlobalObject* aGlobal, const FileDescriptor& aMapFile,
- size_t aMapSize)
+ size_t aMapSize, nsTArray<RefPtr<BlobImpl>>&& aBlobs)
: DOMEventTargetHelper(aGlobal)
+ , mBlobImpls(std::move(aBlobs))
{
mMapFile.reset(new FileDescriptor(aMapFile));
mMapSize = aMapSize;
}
bool
SharedMap::Has(const nsACString& aName)
@@ -102,17 +103,17 @@ SharedMap::Entry::Read(JSContext* aCx,
}
if (mBlobCount) {
holder.BlobImpls().AppendElements(Blobs());
}
holder.Read(aCx, aRetVal, aRv);
}
FileDescriptor
-SharedMap::CloneMapFile()
+SharedMap::CloneMapFile() const
{
if (mMap.initialized()) {
return mMap.cloneHandle();
}
return *mMapFile;
}
void
@@ -278,18 +279,19 @@ WritableSharedMap::WritableSharedMap()
Unused << Serialize();
MOZ_RELEASE_ASSERT(mMap.initialized());
}
SharedMap*
WritableSharedMap::GetReadOnly()
{
if (!mReadOnly) {
+ nsTArray<RefPtr<BlobImpl>> blobs(mBlobImpls);
mReadOnly = new SharedMap(ProcessGlobal::Get(), CloneMapFile(),
- MapSize());
+ MapSize(), std::move(blobs));
}
return mReadOnly;
}
Result<Ok, nsresult>
WritableSharedMap::Serialize()
{
// Serializes a new snapshot of the map, initializes a new read-only shared
@@ -344,24 +346,25 @@ WritableSharedMap::Serialize()
// We need to build the new array of blobs before we overwrite the existing
// one, since previously-serialized entries will store their blob references
// as indexes into our blobs array.
nsTArray<RefPtr<BlobImpl>> blobImpls(blobCount);
for (auto& entry : IterHash(mEntries)) {
AlignTo(&offset, kStructuredCloneAlign);
- entry->ExtractData(&ptr[offset], offset, blobImpls.Length());
+ size_t blobOffset = blobImpls.Length();
+ if (entry->BlobCount()) {
+ blobImpls.AppendElements(entry->Blobs());
+ }
+
+ entry->ExtractData(&ptr[offset], offset, blobOffset);
entry->Code(header);
offset += entry->Size();
-
- if (entry->BlobCount()) {
- mBlobImpls.AppendElements(entry->Blobs());
- }
}
mBlobImpls = std::move(blobImpls);
// FIXME: We should create a separate OutputBuffer class which can encode to
// a static memory region rather than dynamically allocating and then
// copying.
MOZ_ASSERT(header.cursor() == headerSize);
@@ -370,41 +373,47 @@ WritableSharedMap::Serialize()
// We've already updated offsets at this point. We need this to succeed.
mMap.reset();
MOZ_RELEASE_ASSERT(mem.Finalize(mMap).isOk());
return Ok();
}
void
+WritableSharedMap::SendTo(ContentParent* aParent) const
+{
+ nsTArray<IPCBlob> blobs(mBlobImpls.Length());
+
+ for (auto& blobImpl : mBlobImpls) {
+ nsresult rv = IPCBlobUtils::Serialize(blobImpl, aParent,
+ *blobs.AppendElement());
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ continue;
+ }
+ }
+
+ Unused << aParent->SendUpdateSharedData(CloneMapFile(), mMap.size(),
+ blobs, mChangedKeys);
+}
+
+void
WritableSharedMap::BroadcastChanges()
{
if (mChangedKeys.IsEmpty()) {
return;
}
if (!Serialize().isOk()) {
return;
}
nsTArray<ContentParent*> parents;
ContentParent::GetAll(parents);
for (auto& parent : parents) {
- nsTArray<IPCBlob> blobs(mBlobImpls.Length());
-
- for (auto& blobImpl : mBlobImpls) {
- nsresult rv = IPCBlobUtils::Serialize(blobImpl, parent,
- *blobs.AppendElement());
- if (NS_WARN_IF(NS_FAILED(rv))) {
- continue;
- }
- }
-
- Unused << parent->SendUpdateSharedData(CloneMapFile(), mMap.size(),
- blobs, mChangedKeys);
+ SendTo(parent);
}
if (mReadOnly) {
nsTArray<RefPtr<BlobImpl>> blobImpls(mBlobImpls);
mReadOnly->Update(CloneMapFile(), mMap.size(),
std::move(blobImpls),
std::move(mChangedKeys));
}
--- a/dom/ipc/SharedMap.h
+++ b/dom/ipc/SharedMap.h
@@ -17,16 +17,19 @@
#include "mozilla/Variant.h"
#include "nsClassHashtable.h"
#include "nsTArray.h"
class nsIGlobalObject;
namespace mozilla {
namespace dom {
+
+class ContentParent;
+
namespace ipc {
/**
* Together, the SharedMap and WritableSharedMap classes allow sharing a
* dynamically-updated, shared-memory key-value store across processes.
*
* The maps may only ever be updated in the parent process, via
* WritableSharedMap instances. When that map changes, its entire contents are
@@ -53,17 +56,18 @@ namespace ipc {
class SharedMap : public DOMEventTargetHelper
{
using FileDescriptor = mozilla::ipc::FileDescriptor;
public:
SharedMap();
- SharedMap(nsIGlobalObject* aGlobal, const FileDescriptor&, size_t);
+ SharedMap(nsIGlobalObject* aGlobal, const FileDescriptor&, size_t,
+ nsTArray<RefPtr<BlobImpl>>&& aBlobs);
// Returns true if the map contains the given (UTF-8) key.
bool Has(const nsACString& name);
// If the map contains the given (UTF-8) key, decodes and returns a new copy
// of its value. Otherwise returns null.
void Get(JSContext* cx, const nsACString& name, JS::MutableHandleValue aRetVal,
ErrorResult& aRv);
@@ -100,17 +104,17 @@ public:
JS::MutableHandle<JS::Value> aResult) const;
/**
* Returns a copy of the read-only file descriptor which backs the shared
* memory region for this map. The file descriptor may be passed between
* processes, and used to update corresponding instances in child processes.
*/
- FileDescriptor CloneMapFile();
+ FileDescriptor CloneMapFile() const;
/**
* Returns the size of the memory mapped region that backs this map. Must be
* passed to the SharedMap() constructor or Update() method along with the
* descriptor returned by CloneMapFile() in order to initialize or update a
* child SharedMap.
*/
size_t MapSize() const { return mMap.size(); }
@@ -341,16 +345,20 @@ public:
}
// Flushes any queued changes to a new snapshot, and broadcasts it to all
// child SharedMap instances.
void Flush();
+ // Sends the current set of shared map data to the given content process.
+ void SendTo(ContentParent* aContentParent) const;
+
+
/**
* Returns the read-only SharedMap instance corresponding to this
* WritableSharedMap for use in the parent process.
*/
SharedMap* GetReadOnly();
JSObject* WrapObject(JSContext* aCx, JS::HandleObject aGivenProto) override;
--- a/dom/ipc/tests/test_sharedMap.js
+++ b/dom/ipc/tests/test_sharedMap.js
@@ -1,20 +1,36 @@
"use strict";
ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
ChromeUtils.import("resource://gre/modules/Services.jsm");
+ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm");
ChromeUtils.import("resource://testing-common/ExtensionXPCShellUtils.jsm");
+const PROCESS_COUNT_PREF = "dom.ipc.processCount";
+
const remote = AppConstants.platform !== "android";
ExtensionTestUtils.init(this);
let contentPage;
+Cu.importGlobalProperties(["Blob", "FileReader"]);
+
+async function readBlob(key, sharedData = Services.cpmm.sharedData) {
+ let reader = new FileReader();
+ reader.readAsText(sharedData.get(key));
+ await ExtensionUtils.promiseEvent(reader, "loadend");
+ return reader.result;
+}
+
+function getKey(key, sharedData = Services.cpmm.sharedData) {
+ return sharedData.get(key);
+}
+
function getContents(sharedMap = Services.cpmm.sharedData) {
return {
keys: Array.from(sharedMap.keys()),
values: Array.from(sharedMap.values()),
entries: Array.from(sharedMap.entries()),
getValues: Array.from(sharedMap.keys(),
key => sharedMap.get(key)),
};
@@ -55,19 +71,33 @@ async function checkContentMaps(expected
if (!parentOnly) {
info("Checking out-of-process content map");
let contents = await contentPage.spawn(undefined, getContents);
checkMap(contents, expected);
}
}
+async function loadContentPage() {
+ let page = await ExtensionTestUtils.loadContentPage("about:blank", {remote});
+ registerCleanupFunction(() => page.close());
+
+ page.addFrameScriptHelper(`
+ ChromeUtils.import("resource://gre/modules/ExtensionUtils.jsm");
+ Cu.importGlobalProperties(["FileReader"]);
+ `);
+ return page;
+}
+
add_task(async function setup() {
- contentPage = await ExtensionTestUtils.loadContentPage("about:blank", {remote});
- registerCleanupFunction(() => contentPage.close());
+ // Start with one content process so that we can increase the number
+ // later and test the behavior of a fresh content process.
+ Services.prefs.setIntPref(PROCESS_COUNT_PREF, 1);
+
+ contentPage = await loadContentPage();
});
add_task(async function test_sharedMap() {
let {sharedData} = Services.ppmm;
info("Check that parent and child maps are both initially empty");
checkParentMap([]);
@@ -155,8 +185,66 @@ add_task(async function test_sharedMap()
info("Wait for an idle timeout. Check that changes are now visible in all children");
await new Promise(resolve => ChromeUtils.idleDispatch(resolve));
checkParentMap(expected);
await checkContentMaps(expected);
});
+
+add_task(async function test_blobs() {
+ let {sharedData} = Services.ppmm;
+
+ let text = [
+ "The quick brown fox jumps over the lazy dog",
+ "Lorem ipsum dolor sit amet, consectetur adipiscing elit",
+ "sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.",
+ ];
+ let blobs = text.map(str => new Blob([str]));
+
+ let data = {foo: {bar: "baz"}};
+
+ sharedData.set("blob0", blobs[0]);
+ sharedData.set("blob1", blobs[1]);
+ sharedData.set("data", data);
+
+ equal(await readBlob("blob0", sharedData), text[0], "Expected text for blob0 in parent ppmm");
+
+ sharedData.flush();
+
+ equal(await readBlob("blob0", sharedData), text[0], "Expected text for blob0 in parent ppmm");
+ equal(await readBlob("blob1", sharedData), text[1], "Expected text for blob1 in parent ppmm");
+
+ equal(await readBlob("blob0"), text[0], "Expected text for blob0 in parent cpmm");
+ equal(await readBlob("blob1"), text[1], "Expected text for blob1 in parent cpmm");
+
+ equal(await contentPage.spawn("blob0", readBlob), text[0], "Expected text for blob0 in child 1 cpmm");
+ equal(await contentPage.spawn("blob1", readBlob), text[1], "Expected text for blob1 in child 1 cpmm");
+
+ // Start a second child process
+ Services.prefs.setIntPref(PROCESS_COUNT_PREF, 2);
+
+ let page2 = await loadContentPage();
+
+ equal(await page2.spawn("blob0", readBlob), text[0], "Expected text for blob0 in child 2 cpmm");
+ equal(await page2.spawn("blob1", readBlob), text[1], "Expected text for blob1 in child 2 cpmm");
+
+ sharedData.set("blob0", blobs[2]);
+
+ equal(await readBlob("blob0", sharedData), text[2], "Expected text for blob0 in parent ppmm");
+
+ sharedData.flush();
+
+ equal(await readBlob("blob0", sharedData), text[2], "Expected text for blob0 in parent ppmm");
+ equal(await readBlob("blob1", sharedData), text[1], "Expected text for blob1 in parent ppmm");
+
+ equal(await readBlob("blob0"), text[2], "Expected text for blob0 in parent cpmm");
+ equal(await readBlob("blob1"), text[1], "Expected text for blob1 in parent cpmm");
+
+ equal(await contentPage.spawn("blob0", readBlob), text[2], "Expected text for blob0 in child 1 cpmm");
+ equal(await contentPage.spawn("blob1", readBlob), text[1], "Expected text for blob1 in child 1 cpmm");
+
+ equal(await page2.spawn("blob0", readBlob), text[2], "Expected text for blob0 in child 2 cpmm");
+ equal(await page2.spawn("blob1", readBlob), text[1], "Expected text for blob1 in child 2 cpmm");
+
+ deepEqual(await page2.spawn("data", getKey), data, "Expected data for data key in child 2 cpmm");
+});
--- a/js/xpconnect/loader/AutoMemMap.h
+++ b/js/xpconnect/loader/AutoMemMap.h
@@ -46,17 +46,17 @@ class AutoMemMap
// Windows, the FileDescriptor must be a handle for a file mapping,
// rather than a file descriptor.
Result<Ok, nsresult>
initWithHandle(const ipc::FileDescriptor& file, size_t size,
PRFileMapProtect prot = PR_PROT_READONLY);
void reset();
- bool initialized() { return addr; }
+ bool initialized() const { return addr; }
uint32_t size() const { return size_; }
template<typename T = void>
RangedPtr<T> get()
{
MOZ_ASSERT(addr);
return { static_cast<T*>(addr), size_ };