Backed out changeset f21da1055c46 (bug 835698) a=bustage_from_bug_839688

netwerk/ipc/NeckoParent.cpp

@@ -21,7 +21,6 @@
 #include "nsHTMLDNSPrefetch.h"
 #include "nsIAppsService.h"
 #include "nsEscape.h"
-#include "RemoteOpenFileParent.h"
 
 using mozilla::dom::TabParent;
 using mozilla::net::PTCPSocketParent;
@@ -404,14 +403,6 @@ NeckoParent::AllocPRemoteOpenFile(const URIParams& aURI,
   return parent;
 }
 
-bool
-NeckoParent::RecvPRemoteOpenFileConstructor(PRemoteOpenFileParent* aActor,
-                                            const URIParams& aFileURI,
-                                            PBrowserParent* aBrowser)
-{
-  return static_cast<RemoteOpenFileParent*>(aActor)->OpenSendCloseDelete();
-}
-
 bool
 NeckoParent::DeallocPRemoteOpenFile(PRemoteOpenFileParent* actor)
 {
@@ -437,3 +428,4 @@ NeckoParent::RecvCancelHTMLDNSPrefetch(const nsString& hostname,
 }
 
 }} // mozilla::net
+

netwerk/ipc/NeckoParent.h

@@ -69,16 +69,10 @@ protected:
                                             const bool& useSSL,
                                             const nsString& aBinaryType,
                                             PBrowserParent* aBrowser);
-
+  virtual PRemoteOpenFileParent* AllocPRemoteOpenFile(
-  virtual PRemoteOpenFileParent* AllocPRemoteOpenFile(const URIParams& aFileURI,
+                                            const URIParams& fileuri,
-                                                      PBrowserParent* aBrowser)
+                                            PBrowserParent* browser);
-                                                      MOZ_OVERRIDE;
+  virtual bool DeallocPRemoteOpenFile(PRemoteOpenFileParent* actor);
-  virtual bool RecvPRemoteOpenFileConstructor(PRemoteOpenFileParent* aActor,
-                                              const URIParams& aFileURI,
-                                              PBrowserParent* aBrowser)
-                                              MOZ_OVERRIDE;
-  virtual bool DeallocPRemoteOpenFile(PRemoteOpenFileParent* aActor)
-                                      MOZ_OVERRIDE;
 
   virtual bool RecvPTCPSocketConstructor(PTCPSocketParent*,
                                          const nsString& aHost,

netwerk/ipc/PNecko.ipdl

@@ -47,8 +47,6 @@ parent:
   PWebSocket(PBrowser browser, SerializedLoadContext loadContext);
   PTCPSocket(nsString host, uint16_t port, bool useSSL, nsString binaryType,
              nullable PBrowser browser);
-
-  // Request that the parent open a file.
   PRemoteOpenFile(URIParams fileuri, nullable PBrowser browser);
 
   HTMLDNSPrefetch(nsString hostname, uint16_t flags);

netwerk/ipc/PRemoteOpenFile.ipdl

@@ -18,9 +18,18 @@ protocol PRemoteOpenFile
 {
   manager PNecko;
 
+parent:
+  // Tell parent to open file. URI to open was passed and vetted for security in
+  // IPDL constructor: see NeckoParent::AllocPRemoteOpenFile()
+  AsyncOpenFile();
+
+  __delete__();
+
 child:
   // Your file handle is ready, Sir...
-  __delete__(FileDescriptor fd);
+  FileOpened(FileDescriptor fd);
+  // Trying to send invalid fd crashes, so we need separate method for failure
+  FileDidNotOpen();
 };
 
 

netwerk/ipc/RemoteOpenFileChild.cpp

@@ -186,7 +186,11 @@ RemoteOpenFileChild::AsyncRemoteFileOpen(int32_t aFlags,
 
   gNeckoChild->SendPRemoteOpenFileConstructor(this, uri, mTabChild);
 
-  // The chrome process now has a logical ref to us until it calls Send__delete.
+  // Can't seem to reply from within IPDL Parent constructor, so send open as
+  // separate message
+  SendAsyncOpenFile();
+
+  // The chrome process now has a logical ref to us until we call Send__delete
   AddIPDLReference();
 
   mListener = aListener;
@@ -212,13 +216,13 @@ RemoteOpenFileChild::OnCachedFileDescriptor(const nsAString& aPath,
   }
 #endif
 
-  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvDelete */ false);
+  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvFileOpened */ false);
 }
 
 void
 RemoteOpenFileChild::HandleFileDescriptorAndNotifyListener(
                                                       const FileDescriptor& aFD,
-                                                      bool aFromRecvDelete)
+                                                      bool aFromRecvFileOpened)
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
   MOZ_NOT_REACHED("OS X and Windows shouldn't be doing IPDL here");
@@ -239,11 +243,9 @@ RemoteOpenFileChild::HandleFileDescriptorAndNotifyListener(
   nsRefPtr<TabChild> tabChild;
   mTabChild.swap(tabChild);
 
-  // If RemoteOpenFile reply (Recv__delete__) for app's application.zip comes
+  // If there is a pending callback and we're being called from IPDL then we
-  // back sooner than the parent-pushed fd (TabChild::RecvCacheFileDescriptor())
+  // need to cancel it.
-  // have TabChild cancel running callbacks, since we'll call them in
+  if (tabChild && aFromRecvFileOpened) {
-  // NotifyListener.
-  if (tabChild && aFromRecvDelete) {
     nsString path;
     if (NS_FAILED(mFile->GetPath(path))) {
       MOZ_NOT_REACHED("Couldn't get path from file!");
@@ -283,12 +285,33 @@ RemoteOpenFileChild::NotifyListener(nsresult aResult)
 //-----------------------------------------------------------------------------
 
 bool
-RemoteOpenFileChild::Recv__delete__(const FileDescriptor& aFD)
+RemoteOpenFileChild::RecvFileOpened(const FileDescriptor& aFD)
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
   NS_NOTREACHED("OS X and Windows shouldn't be doing IPDL here");
 #else
-  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvDelete */ true);
+  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvFileOpened */ true);
+
+  // This calls NeckoChild::DeallocPRemoteOpenFile(), which deletes |this| if
+  // IPDL holds the last reference.  Don't rely on |this| existing after here!
+  Send__delete__(this);
+#endif
+
+  return true;
+}
+
+bool
+RemoteOpenFileChild::RecvFileDidNotOpen()
+{
+#if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
+  NS_NOTREACHED("OS X and Windows shouldn't be doing IPDL here");
+#else
+  HandleFileDescriptorAndNotifyListener(FileDescriptor(),
+                                        /* aFromRecvFileOpened */ true);
+
+  // This calls NeckoChild::DeallocPRemoteOpenFile(), which deletes |this| if
+  // IPDL holds the last reference.  Don't rely on |this| existing after here!
+  Send__delete__(this);
 #endif
 
   return true;

netwerk/ipc/RemoteOpenFileChild.h

@@ -7,7 +7,6 @@
 #ifndef _RemoteOpenFileChild_h
 #define _RemoteOpenFileChild_h
 
-#include "mozilla/Attributes.h"
 #include "mozilla/dom/TabChild.h"
 #include "mozilla/net/PRemoteOpenFileChild.h"
 #include "nsICachedFileDescriptorListener.h"
@@ -87,13 +86,14 @@ protected:
     AddRef();
   }
 
-  virtual bool Recv__delete__(const FileDescriptor&) MOZ_OVERRIDE;
+  virtual bool RecvFileOpened(const FileDescriptor&);
+  virtual bool RecvFileDidNotOpen();
 
   virtual void OnCachedFileDescriptor(const nsAString& aPath,
                                       const FileDescriptor& aFD) MOZ_OVERRIDE;
 
   void HandleFileDescriptorAndNotifyListener(const FileDescriptor&,
-                                             bool aFromRecvDelete);
+                                             bool aFromRecvFileOpened);
 
   void NotifyListener(nsresult aResult);
 

netwerk/ipc/RemoteOpenFileParent.cpp

@@ -17,42 +17,51 @@
 namespace mozilla {
 namespace net {
 
+RemoteOpenFileParent::RemoteOpenFileParent(nsIFileURL *aURI)
+  : mURI(aURI)
+#if !defined(XP_WIN) && !defined(MOZ_WIDGET_COCOA)
+  , mFd(-1)
+#endif
+{}
+
+RemoteOpenFileParent::~RemoteOpenFileParent()
+{
+#if !defined(XP_WIN) && !defined(MOZ_WIDGET_COCOA)
+  if (mFd != -1) {
+    // close file handle now that other process has it open, else we'll leak
+    // file handles in parent process
+    close(mFd);
+  }
+#endif
+}
+
 bool
-RemoteOpenFileParent::OpenSendCloseDelete()
+RemoteOpenFileParent::RecvAsyncOpenFile()
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
-  MOZ_NOT_REACHED("OS X and Windows shouldn't be doing IPDL here");
+  NS_NOTREACHED("osX and Windows shouldn't be doing IPDL here");
 #else
 
   // TODO: make this async!
 
-  FileDescriptor fileDescriptor;
-
   nsAutoCString path;
   nsresult rv = mURI->GetFilePath(path);
-  NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "GetFilePath failed!");
-
   NS_UnescapeURL(path);
-
   if (NS_SUCCEEDED(rv)) {
     int fd = open(path.get(), O_RDONLY);
-    if (fd == -1) {
+    if (fd != -1) {
-      printf_stderr("RemoteOpenFileParent: file '%s' was not found!\n",
+      unused << SendFileOpened(FileDescriptor(fd));
-                    path.get());
+      // file handle needs to stay open until it's shared with child (and IPDL
-    } else {
+      // is async, so hasn't happened yet). Close in destructor.
-      fileDescriptor = FileDescriptor(fd);
+      mFd = fd;
+      return true;
     }
   }
 
-  // Sending a potentially invalid file descriptor is just fine.
+  // Note: sending an invalid file descriptor currently kills the child process:
-  unused << Send__delete__(this, fileDescriptor);
+  // but that's ok for our use case (failing to open application.jar).
-
+  printf_stderr("RemoteOpenFileParent: file '%s' was not found!\n", path.get());
-  if (fileDescriptor.IsValid()) {
+  unused << SendFileDidNotOpen();
-    // close file now that other process has it open, else we'll leak fds in the
-    // parent process.
-    close(fileDescriptor.PlatformHandle());
-  }
-
 #endif // OS_TYPE
 
   return true;

netwerk/ipc/RemoteOpenFileParent.h

@@ -18,14 +18,18 @@ namespace net {
 class RemoteOpenFileParent : public PRemoteOpenFileParent
 {
 public:
-  RemoteOpenFileParent(nsIFileURL* aURI)
+  RemoteOpenFileParent(nsIFileURL* aURI);
-  : mURI(aURI)
-  {}
 
-  bool OpenSendCloseDelete();
+ ~RemoteOpenFileParent();
+
+  virtual bool RecvAsyncOpenFile();
 
 private:
   nsCOMPtr<nsIFileURL> mURI;
+
+#if !defined(XP_WIN) && !defined(MOZ_WIDGET_COCOA)
+  int mFd;
+#endif
 };
 
 } // namespace net