bug 517923: support serializing ns*Strings that represent NULL, use this mechanism in PluginInstanceParent/PluginModuleChild. also add basic crash-handling to *Channel code and some NS_OVERRIDE annotations.
authorChris Jones <jones.chris.g@gmail.com>
Mon, 21 Sep 2009 21:02:15 -0500
changeset 35940 1dad436eaa992dcd91b0e936c82b5781dadc1962
parent 35939 e812696516917d8e3d276935d7ff94afbb67d340
child 35941 8d05283ce5621bff8b974b8e4438971d57439580
push id10694
push userbsmedberg@mozilla.com
push dateMon, 14 Dec 2009 15:23:10 +0000
treeherdermozilla-central@683dfdc4adf0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs517923
milestone1.9.3a1pre
bug 517923: support serializing ns*Strings that represent NULL, use this mechanism in PluginInstanceParent/PluginModuleChild. also add basic crash-handling to *Channel code and some NS_OVERRIDE annotations.
dom/plugins/PluginInstanceParent.cpp
dom/plugins/PluginMessageUtils.h
dom/plugins/PluginModuleChild.cpp
ipc/glue/AsyncChannel.cpp
ipc/glue/AsyncChannel.h
ipc/glue/IPCMessageUtils.h
ipc/glue/RPCChannel.cpp
ipc/glue/RPCChannel.h
ipc/glue/SyncChannel.cpp
ipc/glue/SyncChannel.h
--- a/dom/plugins/PluginInstanceParent.cpp
+++ b/dom/plugins/PluginInstanceParent.cpp
@@ -152,17 +152,19 @@ PluginInstanceParent::AnswerNPN_GetValue
     return true;
 }
 
 bool
 PluginInstanceParent::AnswerNPN_GetURL(const nsCString& url,
                                        const nsCString& target,
                                        NPError* result)
 {
-    *result = mNPNIface->geturl(mNPP, url.get(), target.get());
+    *result = mNPNIface->geturl(mNPP,
+                                NullableStringGet(url),
+                                NullableStringGet(target));
     return true;
 }
 
 bool
 PluginInstanceParent::AnswerNPN_PostURL(const nsCString& url,
                                         const nsCString& target,
                                         const nsCString& buffer,
                                         const bool& file,
@@ -179,21 +181,25 @@ PluginInstanceParent::PStreamNotifyConst
                                                const bool& post,
                                                const nsCString& buffer,
                                                const bool& file,
                                                NPError* result)
 {
     StreamNotifyParent* notifyData = new StreamNotifyParent();
 
     if (!post) {
-        *result = mNPNIface->geturlnotify(mNPP, url.get(), target.get(),
+        *result = mNPNIface->geturlnotify(mNPP,
+                                          NullableStringGet(url),
+                                          NullableStringGet(target),
                                           notifyData);
     }
     else {
-        *result = mNPNIface->posturlnotify(mNPP, url.get(), target.get(),
+        *result = mNPNIface->posturlnotify(mNPP,
+                                           NullableStringGet(url),
+                                           NullableStringGet(target),
                                            buffer.Length(), buffer.get(),
                                            file, notifyData);
     }
     // TODO: what if this method fails?
     return notifyData;
 }
 
 bool
@@ -261,16 +267,17 @@ NPError
 PluginInstanceParent::NPP_NewStream(NPMIMEType type, NPStream* stream,
                                     NPBool seekable, uint16_t* stype)
 {
     _MOZ_LOG(__FUNCTION__);
         
     BrowserStreamParent* bs = new BrowserStreamParent(this, stream);
 
     NPError err;
+    // TODO are any of these strings nullable?
     if (!CallPBrowserStreamConstructor(bs,
                                        nsCString(stream->url),
                                        stream->end,
                                        stream->lastmodified,
                                        static_cast<PStreamNotifyParent*>(stream->notifyData),
                                        nsCString(stream->headers),
                                        nsCString(type), seekable, &err, stype))
         return NPERR_GENERIC_ERROR;
--- a/dom/plugins/PluginMessageUtils.h
+++ b/dom/plugins/PluginMessageUtils.h
@@ -79,16 +79,39 @@ struct IPCByteRange
 typedef std::vector<IPCByteRange> IPCByteRanges;
 
 typedef nsCString Buffer;
 
 } /* namespace plugins */
 
 } /* namespace mozilla */
 
+
+namespace {
+
+// in NPAPI, char* == NULL is sometimes meaningful.  the following is
+// helper code for dealing with nullable nsCString's
+nsCString
+NullableString(const char* aString)
+{
+    if (!aString) {
+        nsCString str;
+        str.SetIsVoid(PR_TRUE);
+        return str;
+    }
+    return nsCString(aString);
+}
+
+} // namespace <anon>
+
+// TODO is there any safe way for this to be a function?
+#define NullableStringGet(__string)                     \
+    ( __string.IsVoid() ? NULL : __string.get())
+
+
 namespace IPC {
 
 template <>
 struct ParamTraits<NPRect>
 {
   typedef NPRect paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
--- a/dom/plugins/PluginModuleChild.cpp
+++ b/dom/plugins/PluginModuleChild.cpp
@@ -54,19 +54,20 @@
 #include "mozilla/plugins/BrowserStreamChild.h"
 #include "mozilla/plugins/PluginStreamChild.h"
 
 using mozilla::ipc::NPRemoteIdentifier;
 
 using namespace mozilla::plugins;
 
 namespace {
-    PluginModuleChild* gInstance = nsnull;
+PluginModuleChild* gInstance = nsnull;
 }
 
+
 PluginModuleChild::PluginModuleChild() :
     mLibrary(0),
     mInitializeFunc(0),
     mShutdownFunc(0)
 #ifdef OS_WIN
   , mGetEntryPointsFunc(0)
 #endif
 //  ,mNextInstanceId(0)
@@ -446,21 +447,21 @@ NPError NP_CALLBACK
 NPError NP_CALLBACK
 _geturlnotify(NPP aNPP,
               const char* aRelativeURL,
               const char* aTarget,
               void* aNotifyData)
 {
     _MOZ_LOG(__FUNCTION__);
 
-    const nsDependentCString url(aRelativeURL);
+    nsCString url = NullableString(aRelativeURL);
     NPError err;
     InstCast(aNPP)->CallPStreamNotifyConstructor(
         new StreamNotifyChild(url, aNotifyData),
-        url, nsDependentCString(aTarget), false, nsCString(), false, &err);
+        url, NullableString(aTarget), false, nsCString(), false, &err);
     // TODO: what if this fails?
     return err;
 }
 
 NPError NP_CALLBACK
 _getvalue(NPP aNPP,
           NPNVariable aVariable,
           void* aValue)
@@ -481,54 +482,56 @@ NPError NP_CALLBACK
 
 NPError NP_CALLBACK
 _geturl(NPP aNPP,
         const char* aRelativeURL,
         const char* aTarget)
 {
     _MOZ_LOG(__FUNCTION__);
     NPError err;
-    InstCast(aNPP)->CallNPN_GetURL(nsDependentCString(aRelativeURL),
-                                   nsDependentCString(aTarget), &err);
+    InstCast(aNPP)->CallNPN_GetURL(NullableString(aRelativeURL),
+                                   NullableString(aTarget), &err);
     return err;
 }
 
 NPError NP_CALLBACK
 _posturlnotify(NPP aNPP,
                const char* aRelativeURL,
                const char* aTarget,
                uint32_t aLength,
                const char* aBuffer,
                NPBool aIsFile,
                void* aNotifyData)
 {
     _MOZ_LOG(__FUNCTION__);
 
-    const nsDependentCString url(aRelativeURL);
+    nsCString url = NullableString(aRelativeURL);
     NPError err;
+    // FIXME what should happen when |aBuffer| is null?
     InstCast(aNPP)->CallPStreamNotifyConstructor(
         new StreamNotifyChild(url, aNotifyData),
-        url, nsDependentCString(aTarget), true,
+        url, NullableString(aTarget), true,
         nsDependentCString(aBuffer, aLength), aIsFile, &err);
     // TODO: what if this fails?
     return err;
 }
 
 NPError NP_CALLBACK
 _posturl(NPP aNPP,
          const char* aRelativeURL,
          const char* aTarget,
          uint32_t aLength,
          const char* aBuffer,
          NPBool aIsFile)
 {
     _MOZ_LOG(__FUNCTION__);
     NPError err;
-    InstCast(aNPP)->CallNPN_PostURL(nsDependentCString(aRelativeURL),
-                                    nsDependentCString(aTarget),
+    // FIXME what should happen when |aBuffer| is null?
+    InstCast(aNPP)->CallNPN_PostURL(NullableString(aRelativeURL),
+                                    NullableString(aTarget),
                                     nsDependentCString(aBuffer, aLength),
                                     aIsFile, &err);
     return err;
 }
 
 NPError NP_CALLBACK
 _newstream(NPP aNPP,
            NPMIMEType aMIMEType,
@@ -1021,26 +1024,26 @@ PluginModuleChild::AnswerPPluginInstance
 
     nsAutoArrayPtr<char*> argn(new char*[1 + argc]);
     nsAutoArrayPtr<char*> argv(new char*[1 + argc]);
     argn[argc] = 0;
     argv[argc] = 0;
 
     printf ("(plugin args: ");
     for (int i = 0; i < argc; ++i) {
-        argn[i] = const_cast<char*>(aNames[i].get());
-        argv[i] = const_cast<char*>(aValues[i].get());
+        argn[i] = const_cast<char*>(NullableStringGet(aNames[i]));
+        argv[i] = const_cast<char*>(NullableStringGet(aValues[i]));
         printf("%s=%s, ", argn[i], argv[i]);
     }
     printf(")\n");
 
     NPP npp = childInstance->GetNPP();
 
     // FIXME/cjones: use SAFE_CALL stuff
-    *rv = mFunctions.newp((char*)aMimeType.get(),
+    *rv = mFunctions.newp((char*)NullableStringGet(aMimeType),
                           npp,
                           aMode,
                           argc,
                           argn,
                           argv,
                           0);
     if (NPERR_NO_ERROR != *rv) {
         return false;
--- a/ipc/glue/AsyncChannel.cpp
+++ b/ipc/glue/AsyncChannel.cpp
@@ -107,19 +107,21 @@ AsyncChannel::Close()
     // FIXME impl
 
     mChannelState = ChannelClosed;
 }
 
 bool
 AsyncChannel::Send(Message* msg)
 {
-    NS_ASSERTION(ChannelConnected == mChannelState,
-                 "trying to Send() to a channel not yet open");
-    NS_PRECONDITION(MSG_ROUTING_NONE != msg->routing_id(), "need a route");
+    NS_ABORT_IF_FALSE(MSG_ROUTING_NONE != msg->routing_id(), "need a route");
+
+    if (!Connected())
+        // trying to Send() to a closed or error'd channel
+        return false;
 
     mIOLoop->PostTask(FROM_HERE,
                       NewRunnableMethod(this, &AsyncChannel::OnSend, msg));
     return true;
 }
 
 void
 AsyncChannel::OnDispatchMessage(const Message& msg)
--- a/ipc/glue/AsyncChannel.h
+++ b/ipc/glue/AsyncChannel.h
@@ -115,21 +115,25 @@ public:
     // Close from the perspective of the RPC layer; leaves the
     // underlying transport channel open, however.
     void Close();
 
     // Asynchronously send a message to the other side of the channel
     bool Send(Message* msg);
 
     // Implement the IPC::Channel::Listener interface
-    virtual void OnMessageReceived(const Message& msg);
-    virtual void OnChannelConnected(int32 peer_pid);
-    virtual void OnChannelError();
+    NS_OVERRIDE virtual void OnMessageReceived(const Message& msg);
+    NS_OVERRIDE virtual void OnChannelConnected(int32 peer_pid);
+    NS_OVERRIDE virtual void OnChannelError();
 
 protected:
+    bool Connected() {
+        return ChannelConnected == mChannelState;
+    }
+
     // Additional methods that execute on the worker thread
     void OnDispatchMessage(const Message& aMsg);
 
     // Additional methods that execute on the IO thread
     void OnChannelOpened();
     void OnSend(Message* aMsg);
 
     Transport* mTransport;
--- a/ipc/glue/IPCMessageUtils.h
+++ b/ipc/glue/IPCMessageUtils.h
@@ -1,8 +1,10 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set sw=2 ts=8 et tw=80 : */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -51,76 +53,115 @@ namespace IPC {
 
 template <>
 struct ParamTraits<nsACString>
 {
   typedef nsACString paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
+    bool isVoid = aParam.IsVoid();
+    aMsg->WriteBool(isVoid);
+
+    if (isVoid)
+      // represents a NULL pointer
+      return;
+
     PRUint32 length = aParam.Length();
     WriteParam(aMsg, length);
     aMsg->WriteBytes(aParam.BeginReading(), length);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
+    bool isVoid;
+    if (!aMsg->ReadBool(aIter, &isVoid))
+      return false;
+
+    if (isVoid) {
+      aResult->SetIsVoid(PR_TRUE);
+      return true;
+    }
+
     PRUint32 length;
     if (ReadParam(aMsg, aIter, &length)) {
       const char* buf;
       if (aMsg->ReadBytes(aIter, &buf, length)) {
         aResult->Assign(buf, length);
         return true;
       }
     }
     return false;
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
-    aLog->append(UTF8ToWide(aParam.BeginReading()));
+    if (aParam.IsVoid())
+      aLog->append(L"(NULL)");
+    else
+      aLog->append(UTF8ToWide(aParam.BeginReading()));
   }
 };
 
 template <>
 struct ParamTraits<nsAString>
 {
   typedef nsAString paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
+    bool isVoid = aParam.IsVoid();
+    aMsg->WriteBool(isVoid);
+
+    if (isVoid)
+      // represents a NULL pointer
+      return;
+
     PRUint32 length = aParam.Length();
     WriteParam(aMsg, length);
     aMsg->WriteBytes(aParam.BeginReading(), length * sizeof(PRUnichar));
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
+    bool isVoid;
+    if (!aMsg->ReadBool(aIter, &isVoid))
+      return false;
+
+    if (isVoid) {
+      aResult->SetIsVoid(PR_TRUE);
+      return true;
+    }
+
     PRUint32 length;
     if (ReadParam(aMsg, aIter, &length)) {
       const PRUnichar* buf;
       if (aMsg->ReadBytes(aIter, reinterpret_cast<const char**>(&buf),
                        length * sizeof(PRUnichar))) {
         aResult->Assign(buf, length);
         return true;
       }
     }
     return false;
   }
 
   static void Log(const paramType& aParam, std::wstring* aLog)
   {
+    if (aParam.IsVoid())
+      aLog->append(L"(NULL)");
+    else {
 #ifdef WCHAR_T_IS_UTF16
-    aLog->append(reinterpret_cast<const wchar_t*>(aParam.BeginReading()));
+      aLog->append(reinterpret_cast<const wchar_t*>(aParam.BeginReading()));
 #else
-    PRUint32 length = aParam.Length();
-    for (PRUint32 index = 0; index < length; index++) {
-      aLog->push_back(std::wstring::value_type(aParam[index]));
+      PRUint32 length = aParam.Length();
+      for (PRUint32 index = 0; index < length; index++) {
+        aLog->push_back(std::wstring::value_type(aParam[index]));
+      }
+#endif
     }
-#endif
   }
 };
 
 template <>
 struct ParamTraits<nsCString> : ParamTraits<nsACString>
 {
   typedef nsCString paramType;
 };
--- a/ipc/glue/RPCChannel.cpp
+++ b/ipc/glue/RPCChannel.cpp
@@ -55,37 +55,43 @@ struct RunnableMethodTraits<mozilla::ipc
 namespace mozilla {
 namespace ipc {
 
 bool
 RPCChannel::Call(Message* msg, Message* reply)
 {
     NS_ABORT_IF_FALSE(!ProcessingSyncMessage(),
                       "violation of sync handler invariant");
-    NS_ASSERTION(ChannelConnected == mChannelState,
-                 "trying to Send() to a channel not yet open");
-    NS_PRECONDITION(msg->is_rpc(),
-                    "can only Call() RPC messages here");
+    NS_ABORT_IF_FALSE(msg->is_rpc(),
+                      "can only Call() RPC messages here");
+
+    if (!Connected())
+        // trying to Send() to a closed or error'd channel
+        return false;
 
     MutexAutoLock lock(mMutex);
 
     msg->set_rpc_remote_stack_depth(mRemoteStackDepth);
     mStack.push(*msg);
 
     // bypass |SyncChannel::Send| b/c RPCChannel implements its own
     // waiting semantics
     AsyncChannel::Send(msg);
 
     while (1) {
         // here we're waiting for something to happen. see long
         // comment about the queue in RPCChannel.h
-        while (mPending.empty()) {
+        while (Connected() && mPending.empty()) {
             mCvar.Wait();
         }
 
+        if (!Connected())
+            // FIXME more sophisticated error handling
+            return false;
+
         Message recvd = mPending.front();
         mPending.pop();
 
         // async message.  process it, go back to waiting
         if (!recvd.is_sync() && !recvd.is_rpc()) {
             MutexAutoUnlock unlock(mMutex);
 
             AsyncChannel::OnDispatchMessage(recvd);
@@ -358,10 +364,30 @@ RPCChannel::OnMessageReceived(const Mess
         // were awaiting.  Dispatch to the worker, where invariants
         // are checked and the message processed.
         mPending.push(msg);
         mCvar.Notify();
     }
 }
 
 
+void
+RPCChannel::OnChannelError()
+{
+    {
+        MutexAutoLock lock(mMutex);
+
+        mChannelState = ChannelError;
+
+        if (AwaitingSyncReply()
+            || 0 < StackDepth()) {
+            mCvar.Notify();
+        }
+    }
+
+    // skip SyncChannel::OnError(); we subsume its duties
+
+    return AsyncChannel::OnChannelError();
+}
+
+
 } // namespace ipc
 } // namespace mozilla
--- a/ipc/glue/RPCChannel.h
+++ b/ipc/glue/RPCChannel.h
@@ -75,17 +75,18 @@ public:
     {
         // FIXME/cjones: impl
     }
 
     // Make an RPC to the other side of the channel
     bool Call(Message* msg, Message* reply);
 
     // Override the SyncChannel handler so we can dispatch RPC messages
-    virtual void OnMessageReceived(const Message& msg);
+    NS_OVERRIDE virtual void OnMessageReceived(const Message& msg);
+    NS_OVERRIDE virtual void OnChannelError();
 
 protected:
     // Only exists because we can't schedule SyncChannel::OnDispatchMessage
     // or AsyncChannel::OnDispatchMessage from within Call() when we flush
     // the pending queue
     void OnDelegate(const Message& msg);
 
 private:
--- a/ipc/glue/SyncChannel.cpp
+++ b/ipc/glue/SyncChannel.cpp
@@ -54,28 +54,36 @@ struct RunnableMethodTraits<mozilla::ipc
 namespace mozilla {
 namespace ipc {
 
 bool
 SyncChannel::Send(Message* msg, Message* reply)
 {
     NS_ABORT_IF_FALSE(!ProcessingSyncMessage(),
                       "violation of sync handler invariant");
-    NS_ASSERTION(ChannelConnected == mChannelState,
-                 "trying to Send() to a channel not yet open");
-    NS_PRECONDITION(msg->is_sync(), "can only Send() sync messages here");
+    NS_ABORT_IF_FALSE(msg->is_sync(), "can only Send() sync messages here");
+
+    if (!Connected())
+        // trying to Send() to a closed or error'd channel
+        return false;
 
     MutexAutoLock lock(mMutex);
 
     mPendingReply = msg->type() + 1;
-    /*assert*/AsyncChannel::Send(msg);
+    if (!AsyncChannel::Send(msg))
+        // FIXME more sophisticated error handling
+        return false;
 
     // wait for the next sync message to arrive
     mCvar.Wait();
 
+    if (!Connected())
+        // FIXME more sophisticated error handling
+        return false;
+
     // we just received a synchronous message from the other side.
     // If it's not the reply we were awaiting, there's a serious
     // error: either a mistimed/malformed message or a sync in-message
     // that raced with our sync out-message.
     // (NB: IPDL prevents the latter from occuring in actor code)
 
     // FIXME/cjones: real error handling
     NS_ABORT_IF_FALSE(mRecvd.is_sync() && mRecvd.is_reply() &&
@@ -152,16 +160,32 @@ SyncChannel::OnMessageReceived(const Mes
     else {
         // let the worker know a new sync message has arrived
         mRecvd = msg;
         mCvar.Notify();
     }
 }
 
 void
+SyncChannel::OnChannelError()
+{
+    {
+        MutexAutoLock lock(mMutex);
+
+        mChannelState = ChannelError;
+
+        if (AwaitingSyncReply()) {
+            mCvar.Notify();
+        }
+    }
+
+    return AsyncChannel::OnChannelError();
+}
+
+void
 SyncChannel::OnSendReply(Message* aReply)
 {
     mTransport->Send(aReply);
 }
 
 
 } // namespace ipc
 } // namespace mozilla
--- a/ipc/glue/SyncChannel.h
+++ b/ipc/glue/SyncChannel.h
@@ -79,17 +79,18 @@ public:
     bool Send(Message* msg) {
         return AsyncChannel::Send(msg);
     }
 
     // Synchronously send |msg| (i.e., wait for |reply|)
     bool Send(Message* msg, Message* reply);
 
     // Override the AsyncChannel handler so we can dispatch sync messages
-    virtual void OnMessageReceived(const Message& msg);
+    NS_OVERRIDE virtual void OnMessageReceived(const Message& msg);
+    NS_OVERRIDE virtual void OnChannelError();
 
 protected:
     // Executed on the worker thread
     bool ProcessingSyncMessage() {
         return mProcessingSyncMessage;
     }
 
     void OnDispatchMessage(const Message& aMsg);