Bug 1161169: Clean up usage of mContentParent and clearly identify it as specifically for async plugin init; r=billm,jimm

mContentParent is really just to be used while handling a synchronous
ContentParent::RecvLoadPlugin call when async plugin init turned on.

In any other context, using it will be unsafe.

This patch adds comments and assertions to ensure that this value isn't set
otherwise, and converts the one use of mContentParent outside of async plugin
init to use an alternative mechanism for identifying the content process.

MozReview-Commit-ID: Esgt1kj0MCt

--HG--
extra : rebase_source : 166b913b401582c6948e4b61efc102dc1b9c8d2f
This commit is contained in:
Aaron Klotz 2016-03-03 00:24:36 -07:00
parent 4f0cfa3b70
commit 094a7b0970
6 changed files with 41 additions and 14 deletions

View File

@ -5,6 +5,7 @@
* 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/. */
using base::ProcessId from "base/process.h";
using mozilla::dom::TabId from "mozilla/dom/ipc/IdType.h";
namespace mozilla {
@ -19,6 +20,7 @@ struct SlowScriptData
struct PluginHangData
{
uint32_t pluginId;
ProcessId contentProcessId;
};
union HangData

View File

@ -428,7 +428,8 @@ HangMonitorChild::NotifyPluginHangAsync(uint32_t aPluginId)
// bounce back to parent on background thread
if (mIPCOpen) {
Unused << SendHangEvidence(PluginHangData(aPluginId));
Unused << SendHangEvidence(PluginHangData(aPluginId,
base::GetCurrentProcId()));
}
}
@ -819,7 +820,8 @@ HangMonitoredProcess::TerminatePlugin()
// generates a crash report that includes a browser report taken here
// earlier, the content process, and any plugin process(es).
uint32_t id = mHangData.get_PluginHangData().pluginId();
plugins::TerminatePlugin(id, NS_LITERAL_CSTRING("HangMonitor"),
base::ProcessId contentPid = mHangData.get_PluginHangData().contentProcessId();
plugins::TerminatePlugin(id, contentPid, NS_LITERAL_CSTRING("HangMonitor"),
mBrowserDumpId);
if (mActor) {

View File

@ -7,6 +7,8 @@
#ifndef mozilla_plugins_PluginBridge_h
#define mozilla_plugins_PluginBridge_h
#include "base/process.h"
namespace mozilla {
namespace dom {
@ -26,6 +28,7 @@ FindPluginsForContent(uint32_t aPluginEpoch,
void
TerminatePlugin(uint32_t aPluginId,
base::ProcessId aContentProcessId,
const nsCString& aMonitorDescription,
const nsAString& aBrowserDumpId);

View File

@ -9,6 +9,7 @@
#include "PluginHangUIParent.h"
#include "mozilla/Telemetry.h"
#include "mozilla/ipc/ProtocolUtils.h"
#include "mozilla/plugins/PluginModuleParent.h"
#include "nsContentUtils.h"
@ -354,6 +355,7 @@ PluginHangUIParent::RecvUserResponse(const unsigned int& aResponse)
if (aResponse & HANGUI_USER_RESPONSE_STOP) {
// User clicked Stop
mModule->TerminateChildProcess(mMainThreadMessageLoop,
mozilla::ipc::kInvalidProcessId,
NS_LITERAL_CSTRING("ModalHangUI"),
EmptyString());
responseCode = 1;

View File

@ -17,6 +17,7 @@
#include "mozilla/dom/PCrashReporterParent.h"
#include "mozilla/ipc/GeckoChildProcessHost.h"
#include "mozilla/ipc/MessageChannel.h"
#include "mozilla/ipc/ProtocolUtils.h"
#include "mozilla/plugins/BrowserStreamParent.h"
#include "mozilla/plugins/PluginAsyncSurrogate.h"
#include "mozilla/plugins/PluginBridge.h"
@ -127,7 +128,9 @@ mozilla::plugins::SetupBridge(uint32_t aPluginId,
if (NS_FAILED(*rv)) {
return true;
}
chromeParent->SetContentParent(aContentParent);
if (chromeParent->IsStartingAsync()) {
chromeParent->SetContentParent(aContentParent);
}
if (!aForceBridgeNow && chromeParent->IsStartingAsync() &&
PluginModuleChromeParent::DidInstantiate()) {
// We'll handle the bridging asynchronously
@ -358,6 +361,7 @@ bool PluginModuleMapping::sIsLoadModuleOnStack = false;
void
mozilla::plugins::TerminatePlugin(uint32_t aPluginId,
base::ProcessId aContentProcessId,
const nsCString& aMonitorDescription,
const nsAString& aBrowserDumpId)
{
@ -372,6 +376,7 @@ mozilla::plugins::TerminatePlugin(uint32_t aPluginId,
PluginModuleChromeParent* chromeParent =
static_cast<PluginModuleChromeParent*>(plugin->GetLibrary());
chromeParent->TerminateChildProcess(MessageLoop::current(),
aContentProcessId,
aMonitorDescription,
aBrowserDumpId);
}
@ -473,7 +478,8 @@ PluginModuleContentParent::OnLoadPluginResult(const uint32_t& aPluginId,
void
PluginModuleChromeParent::SetContentParent(dom::ContentParent* aContentParent)
{
MOZ_ASSERT(aContentParent);
// mContentParent is to be used ONLY during async plugin init!
MOZ_ASSERT(aContentParent && mIsStartingAsync);
mContentParent = aContentParent;
}
@ -713,7 +719,7 @@ PluginModuleChromeParent::PluginModuleChromeParent(const char* aFilePath,
, mPluginId(aPluginId)
, mChromeTaskFactory(this)
, mHangAnnotationFlags(0)
, mHangAnnotatorMutex("PluginModuleChromeParent::mHangAnnotatorMutex")
, mProtocolCallStackMutex("PluginModuleChromeParent::mProtocolCallStackMutex")
#ifdef XP_WIN
, mPluginCpuUsageOnHang()
, mHangUIParent(nullptr)
@ -985,14 +991,14 @@ PluginModuleChromeParent::OnEnteredCall()
{
mozilla::ipc::IProtocol* protocol = GetInvokingProtocol();
MOZ_ASSERT(protocol);
mozilla::MutexAutoLock lock(mHangAnnotatorMutex);
mozilla::MutexAutoLock lock(mProtocolCallStackMutex);
mProtocolCallStack.AppendElement(protocol);
}
void
PluginModuleChromeParent::OnExitedCall()
{
mozilla::MutexAutoLock lock(mHangAnnotatorMutex);
mozilla::MutexAutoLock lock(mProtocolCallStackMutex);
MOZ_ASSERT(!mProtocolCallStack.IsEmpty());
mProtocolCallStack.RemoveElementAt(mProtocolCallStack.Length() - 1);
}
@ -1002,14 +1008,14 @@ PluginModuleChromeParent::OnEnteredSyncSend()
{
mozilla::ipc::IProtocol* protocol = GetInvokingProtocol();
MOZ_ASSERT(protocol);
mozilla::MutexAutoLock lock(mHangAnnotatorMutex);
mozilla::MutexAutoLock lock(mProtocolCallStackMutex);
mProtocolCallStack.AppendElement(protocol);
}
void
PluginModuleChromeParent::OnExitedSyncSend()
{
mozilla::MutexAutoLock lock(mHangAnnotatorMutex);
mozilla::MutexAutoLock lock(mProtocolCallStackMutex);
MOZ_ASSERT(!mProtocolCallStack.IsEmpty());
mProtocolCallStack.RemoveElementAt(mProtocolCallStack.Length() - 1);
}
@ -1171,6 +1177,7 @@ PluginModuleChromeParent::ShouldContinueFromReplyTimeout()
FinishHangUI();
#endif // XP_WIN
TerminateChildProcess(MessageLoop::current(),
mozilla::ipc::kInvalidProcessId,
NS_LITERAL_CSTRING("ModalHangUI"),
EmptyString());
GetIPCChannel()->CloseWithTimeout();
@ -1196,6 +1203,7 @@ PluginModuleContentParent::OnExitedSyncSend()
void
PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
base::ProcessId aContentPid,
const nsCString& aMonitorDescription,
const nsAString& aBrowserDumpId)
{
@ -1281,9 +1289,9 @@ PluginModuleChromeParent::TerminateChildProcess(MessageLoop* aMsgLoop,
additionalDumps.AppendLiteral(",flash2");
}
#endif
if (mContentParent) {
if (aContentPid != mozilla::ipc::kInvalidProcessId) {
// Include the content process minidump
if (CreatePluginMinidump(mContentParent->OtherPid(), 0,
if (CreatePluginMinidump(aContentPid, 0,
pluginDumpFile,
NS_LITERAL_CSTRING("content"))) {
additionalDumps.AppendLiteral(",content");
@ -2239,7 +2247,9 @@ PluginModuleChromeParent::RecvNP_InitializeResult(const NPError& aError)
}
}
mNPInitialized = initOk;
return mContentParent->SendLoadPluginResult(mPluginId, initOk);
bool result = mContentParent->SendLoadPluginResult(mPluginId, initOk);
mContentParent = nullptr;
return result;
}
#else
@ -2346,9 +2356,10 @@ PluginModuleChromeParent::RecvNP_InitializeResult(const NPError& aError)
if ((ok = SendAssociatePluginId())) {
ok = mContentParent->SendLoadPluginResult(mPluginId,
aError == NPERR_NO_ERROR);
mContentParent = nullptr;
}
} else if (aError == NPERR_NO_ERROR) {
// Initialization steps when e10s is disabled
// Initialization steps for (e10s && !asyncInit) || !e10s
#if defined XP_WIN
if (mIsStartingAsync) {
SetPluginFuncs(mNPPIface);

View File

@ -418,6 +418,8 @@ class PluginModuleChromeParent
*
* @param aMsgLoop the main message pump associated with the module
* protocol.
* @param aContentPid PID of the e10s content process from which a hang was
* reported. May be kInvalidProcessId if not applicable.
* @param aMonitorDescription a string describing the hang monitor that
* is making this call. This string is added to the crash reporter
* annotations for the plugin process.
@ -427,6 +429,7 @@ class PluginModuleChromeParent
* dump will be taken at the time of this call.
*/
void TerminateChildProcess(MessageLoop* aMsgLoop,
base::ProcessId aContentPid,
const nsCString& aMonitorDescription,
const nsAString& aBrowserDumpId);
@ -547,7 +550,7 @@ private:
kHangUIDontShow = (1u << 3)
};
Atomic<uint32_t> mHangAnnotationFlags;
mozilla::Mutex mHangAnnotatorMutex;
mozilla::Mutex mProtocolCallStackMutex;
InfallibleTArray<mozilla::ipc::IProtocol*> mProtocolCallStack;
#ifdef XP_WIN
InfallibleTArray<float> mPluginCpuUsageOnHang;
@ -625,6 +628,10 @@ private:
bool mInitOnAsyncConnect;
nsresult mAsyncInitRv;
NPError mAsyncInitError;
// mContentParent is to be used ONLY during the IPC dance that occurs
// when ContentParent::RecvLoadPlugin is called under async plugin init!
// In other contexts it is *unsafe*, as there might be multiple content
// processes in existence!
dom::ContentParent* mContentParent;
nsCOMPtr<nsIObserver> mOfflineObserver;
#ifdef MOZ_ENABLE_PROFILER_SPS