Bug 835698 - 'Pre-open() and send the fd for app process's application.zip'. r=jduell.
authorBen Turner <bent.mozilla@gmail.com>
Thu, 07 Feb 2013 13:06:58 +0000
changeset 121156 f21da1055c468cc998cec3815d2ac9364b7ca762
parent 121155 6d5b6ce4a2ae90edd2d1206ef602dcebd84595bc
child 121157 62b7e0a68a11d906fb1b0e37984a735959cdbe54
push idunknown
push userunknown
push dateunknown
reviewersjduell
bugs835698
milestone21.0a1
Bug 835698 - 'Pre-open() and send the fd for app process's application.zip'. r=jduell.
netwerk/ipc/NeckoParent.cpp
netwerk/ipc/NeckoParent.h
netwerk/ipc/PNecko.ipdl
netwerk/ipc/PRemoteOpenFile.ipdl
netwerk/ipc/RemoteOpenFileChild.cpp
netwerk/ipc/RemoteOpenFileChild.h
netwerk/ipc/RemoteOpenFileParent.cpp
netwerk/ipc/RemoteOpenFileParent.h
--- a/netwerk/ipc/NeckoParent.cpp
+++ b/netwerk/ipc/NeckoParent.cpp
@@ -16,16 +16,17 @@
 #include "mozilla/dom/TabParent.h"
 #include "mozilla/dom/network/TCPSocketParent.h"
 #include "mozilla/ipc/URIUtils.h"
 #include "mozilla/LoadContext.h"
 #include "nsPrintfCString.h"
 #include "nsHTMLDNSPrefetch.h"
 #include "nsIAppsService.h"
 #include "nsEscape.h"
+#include "RemoteOpenFileParent.h"
 
 using mozilla::dom::TabParent;
 using mozilla::net::PTCPSocketParent;
 using mozilla::dom::TCPSocketParent;
 using IPC::SerializedLoadContext;
 
 namespace mozilla {
 namespace net {
@@ -399,16 +400,24 @@ NeckoParent::AllocPRemoteOpenFile(const 
     }
   }
 
   RemoteOpenFileParent* parent = new RemoteOpenFileParent(fileURL);
   return parent;
 }
 
 bool
+NeckoParent::RecvPRemoteOpenFileConstructor(PRemoteOpenFileParent* aActor,
+                                            const URIParams& aFileURI,
+                                            PBrowserParent* aBrowser)
+{
+  return static_cast<RemoteOpenFileParent*>(aActor)->OpenSendCloseDelete();
+}
+
+bool
 NeckoParent::DeallocPRemoteOpenFile(PRemoteOpenFileParent* actor)
 {
   delete actor;
   return true;
 }
 
 bool
 NeckoParent::RecvHTMLDNSPrefetch(const nsString& hostname,
@@ -423,9 +432,8 @@ NeckoParent::RecvCancelHTMLDNSPrefetch(c
                                  const uint16_t& flags,
                                  const nsresult& reason)
 {
   nsHTMLDNSPrefetch::CancelPrefetch(hostname, flags, reason);
   return true;
 }
 
 }} // mozilla::net
-
--- a/netwerk/ipc/NeckoParent.h
+++ b/netwerk/ipc/NeckoParent.h
@@ -64,20 +64,26 @@ protected:
   virtual PWebSocketParent* AllocPWebSocket(PBrowserParent* browser,
                                             const SerializedLoadContext& aSerialized);
   virtual bool DeallocPWebSocket(PWebSocketParent*);
   virtual PTCPSocketParent* AllocPTCPSocket(const nsString& aHost,
                                             const uint16_t& aPort,
                                             const bool& useSSL,
                                             const nsString& aBinaryType,
                                             PBrowserParent* aBrowser);
-  virtual PRemoteOpenFileParent* AllocPRemoteOpenFile(
-                                            const URIParams& fileuri,
-                                            PBrowserParent* browser);
-  virtual bool DeallocPRemoteOpenFile(PRemoteOpenFileParent* actor);
+
+  virtual PRemoteOpenFileParent* AllocPRemoteOpenFile(const URIParams& aFileURI,
+                                                      PBrowserParent* aBrowser)
+                                                      MOZ_OVERRIDE;
+  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,
                                          const uint16_t& aPort,
                                          const bool& useSSL,
                                          const nsString& aBinaryType,
                                          PBrowserParent* aBrowser);
   virtual bool DeallocPTCPSocket(PTCPSocketParent*);
--- a/netwerk/ipc/PNecko.ipdl
+++ b/netwerk/ipc/PNecko.ipdl
@@ -42,16 +42,18 @@ parent:
   PCookieService();
   PHttpChannel(nullable PBrowser browser,
                SerializedLoadContext loadContext);
   PWyciwygChannel();
   PFTPChannel(PBrowser browser, SerializedLoadContext loadContext);
   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);
   CancelHTMLDNSPrefetch(nsString hostname, uint16_t flags, nsresult reason);
 
 };
 
 
--- a/netwerk/ipc/PRemoteOpenFile.ipdl
+++ b/netwerk/ipc/PRemoteOpenFile.ipdl
@@ -13,25 +13,16 @@ namespace net {
 /**
  * Protocol to support RemoteOpenFile, an nsIFile that opens it's file handle on
  * the parent instead of the child (since child lacks permission to do so).
  */
 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...
-  FileOpened(FileDescriptor fd);
-  // Trying to send invalid fd crashes, so we need separate method for failure
-  FileDidNotOpen();
+  __delete__(FileDescriptor fd);
 };
 
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/ipc/RemoteOpenFileChild.cpp
+++ b/netwerk/ipc/RemoteOpenFileChild.cpp
@@ -181,21 +181,17 @@ RemoteOpenFileChild::AsyncRemoteFileOpen
     }
   }
 
   URIParams uri;
   SerializeURI(mURI, uri);
 
   gNeckoChild->SendPRemoteOpenFileConstructor(this, uri, mTabChild);
 
-  // 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
+  // The chrome process now has a logical ref to us until it calls Send__delete.
   AddIPDLReference();
 
   mListener = aListener;
   mAsyncOpenCalled = true;
   return NS_OK;
 #endif
 }
 
@@ -211,23 +207,23 @@ RemoteOpenFileChild::OnCachedFileDescrip
     if (NS_FAILED(mFile->GetPath(path))) {
       MOZ_NOT_REACHED("Couldn't get path from file!");
     }
 
     MOZ_ASSERT(path == aPath, "Paths don't match!");
   }
 #endif
 
-  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvFileOpened */ false);
+  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvDelete */ false);
 }
 
 void
 RemoteOpenFileChild::HandleFileDescriptorAndNotifyListener(
                                                       const FileDescriptor& aFD,
-                                                      bool aFromRecvFileOpened)
+                                                      bool aFromRecvDelete)
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
   MOZ_NOT_REACHED("OS X and Windows shouldn't be doing IPDL here");
 #else
   if (!mListener) {
     // We already notified our listener (either in response to a cached file
     // descriptor callback or through the normal messaging mechanism). Close the
     // file descriptor if it is valid.
@@ -238,19 +234,21 @@ RemoteOpenFileChild::HandleFileDescripto
     return;
   }
 
   MOZ_ASSERT(!mNSPRFileDesc);
 
   nsRefPtr<TabChild> tabChild;
   mTabChild.swap(tabChild);
 
-  // If there is a pending callback and we're being called from IPDL then we
-  // need to cancel it.
-  if (tabChild && aFromRecvFileOpened) {
+  // If RemoteOpenFile reply (Recv__delete__) for app's application.zip comes
+  // back sooner than the parent-pushed fd (TabChild::RecvCacheFileDescriptor())
+  // have TabChild cancel running callbacks, since we'll call them in
+  // NotifyListener.
+  if (tabChild && aFromRecvDelete) {
     nsString path;
     if (NS_FAILED(mFile->GetPath(path))) {
       MOZ_NOT_REACHED("Couldn't get path from file!");
     }
 
     tabChild->CancelCachedFileDescriptorCallback(path, this);
   }
 
@@ -280,43 +278,22 @@ RemoteOpenFileChild::NotifyListener(nsre
   }
 }
 
 //-----------------------------------------------------------------------------
 // RemoteOpenFileChild::PRemoteOpenFileChild
 //-----------------------------------------------------------------------------
 
 bool
-RemoteOpenFileChild::RecvFileOpened(const FileDescriptor& aFD)
+RemoteOpenFileChild::Recv__delete__(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, /* 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);
+  HandleFileDescriptorAndNotifyListener(aFD, /* aFromRecvDelete */ true);
 #endif
 
   return true;
 }
 
 //-----------------------------------------------------------------------------
 // RemoteOpenFileChild::nsIFile functions that we override logic for
 //-----------------------------------------------------------------------------
--- a/netwerk/ipc/RemoteOpenFileChild.h
+++ b/netwerk/ipc/RemoteOpenFileChild.h
@@ -2,16 +2,17 @@
 /* vim: set sw=2 ts=8 et tw=80 : */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 
 #ifndef _RemoteOpenFileChild_h
 #define _RemoteOpenFileChild_h
 
+#include "mozilla/Attributes.h"
 #include "mozilla/dom/TabChild.h"
 #include "mozilla/net/PRemoteOpenFileChild.h"
 #include "nsICachedFileDescriptorListener.h"
 #include "nsILocalFile.h"
 #include "nsIRemoteOpenFileListener.h"
 
 namespace mozilla {
 
@@ -81,24 +82,23 @@ private:
   RemoteOpenFileChild(const RemoteOpenFileChild& other);
 
 protected:
   void AddIPDLReference()
   {
     AddRef();
   }
 
-  virtual bool RecvFileOpened(const FileDescriptor&);
-  virtual bool RecvFileDidNotOpen();
+  virtual bool Recv__delete__(const FileDescriptor&) MOZ_OVERRIDE;
 
   virtual void OnCachedFileDescriptor(const nsAString& aPath,
                                       const FileDescriptor& aFD) MOZ_OVERRIDE;
 
   void HandleFileDescriptorAndNotifyListener(const FileDescriptor&,
-                                             bool aFromRecvFileOpened);
+                                             bool aFromRecvDelete);
 
   void NotifyListener(nsresult aResult);
 
   // regular nsIFile object, that we forward most calls to.
   nsCOMPtr<nsIFile> mFile;
   nsCOMPtr<nsIURI> mURI;
   nsCOMPtr<nsIRemoteOpenFileListener> mListener;
   nsRefPtr<TabChild> mTabChild;
--- a/netwerk/ipc/RemoteOpenFileParent.cpp
+++ b/netwerk/ipc/RemoteOpenFileParent.cpp
@@ -12,60 +12,51 @@
 #if !defined(XP_WIN) && !defined(MOZ_WIDGET_COCOA)
 #include <fcntl.h>
 #include <unistd.h>
 #endif
 
 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::RecvAsyncOpenFile()
+RemoteOpenFileParent::OpenSendCloseDelete()
 {
 #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
-  NS_NOTREACHED("osX and Windows shouldn't be doing IPDL here");
+  MOZ_NOT_REACHED("OS X 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) {
-      unused << SendFileOpened(FileDescriptor(fd));
-      // file handle needs to stay open until it's shared with child (and IPDL
-      // is async, so hasn't happened yet). Close in destructor.
-      mFd = fd;
-      return true;
+    if (fd == -1) {
+      printf_stderr("RemoteOpenFileParent: file '%s' was not found!\n",
+                    path.get());
+    } else {
+      fileDescriptor = FileDescriptor(fd);
     }
   }
 
-  // Note: sending an invalid file descriptor currently kills the child process:
-  // but that's ok for our use case (failing to open application.jar).
-  printf_stderr("RemoteOpenFileParent: file '%s' was not found!\n", path.get());
-  unused << SendFileDidNotOpen();
+  // Sending a potentially invalid file descriptor is just fine.
+  unused << Send__delete__(this, fileDescriptor);
+
+  if (fileDescriptor.IsValid()) {
+    // 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;
 }
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/ipc/RemoteOpenFileParent.h
+++ b/netwerk/ipc/RemoteOpenFileParent.h
@@ -13,26 +13,22 @@
 #include "nsIFileURL.h"
 
 namespace mozilla {
 namespace net {
 
 class RemoteOpenFileParent : public PRemoteOpenFileParent
 {
 public:
-  RemoteOpenFileParent(nsIFileURL* aURI);
+  RemoteOpenFileParent(nsIFileURL* aURI)
+  : mURI(aURI)
+  {}
 
- ~RemoteOpenFileParent();
-
-  virtual bool RecvAsyncOpenFile();
+  bool OpenSendCloseDelete();
 
 private:
   nsCOMPtr<nsIFileURL> mURI;
-
-#if !defined(XP_WIN) && !defined(MOZ_WIDGET_COCOA)
-  int mFd;
-#endif
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // mozilla_net_RemoteOpenFileParent_h