Backed out changeset f21da1055c46 (bug 835698) a=bustage_from_bug_839688
authorJason Duell <jduell.mcbugs@gmail.com>
Fri, 08 Feb 2013 17:18:18 -0800
changeset 127450 31d07b2d8234e0a5f005c817036edd5d69ee4b18
parent 127449 c1ee454506f6b9aca27b59b101c47246b8a3034d
child 127451 876652a0c8ef92c1bcd766be84c000a603670908
push idunknown
push userunknown
push dateunknown
reviewersbustage_from_bug_839688
bugs835698
milestone21.0a1
backs outf21da1055c468cc998cec3815d2ac9364b7ca762
Backed out changeset f21da1055c46 (bug 835698) a=bustage_from_bug_839688
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,17 +16,16 @@
 #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 {
@@ -400,24 +399,16 @@ 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,
@@ -432,8 +423,9 @@ 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,26 +64,20 @@ 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& 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 PRemoteOpenFileParent* AllocPRemoteOpenFile(
+                                            const URIParams& fileuri,
+                                            PBrowserParent* browser);
+  virtual bool DeallocPRemoteOpenFile(PRemoteOpenFileParent* actor);
 
   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,18 +42,16 @@ 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,16 +13,25 @@ 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...
-  __delete__(FileDescriptor fd);
+  FileOpened(FileDescriptor fd);
+  // Trying to send invalid fd crashes, so we need separate method for failure
+  FileDidNotOpen();
 };
 
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/ipc/RemoteOpenFileChild.cpp
+++ b/netwerk/ipc/RemoteOpenFileChild.cpp
@@ -181,17 +181,21 @@ RemoteOpenFileChild::AsyncRemoteFileOpen
     }
   }
 
   URIParams uri;
   SerializeURI(mURI, uri);
 
   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;
   mAsyncOpenCalled = true;
   return NS_OK;
 #endif
 }
 
@@ -207,23 +211,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, /* 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");
 #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.
@@ -234,21 +238,19 @@ RemoteOpenFileChild::HandleFileDescripto
     return;
   }
 
   MOZ_ASSERT(!mNSPRFileDesc);
 
   nsRefPtr<TabChild> tabChild;
   mTabChild.swap(tabChild);
 
-  // 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) {
+  // If there is a pending callback and we're being called from IPDL then we
+  // need to cancel it.
+  if (tabChild && aFromRecvFileOpened) {
     nsString path;
     if (NS_FAILED(mFile->GetPath(path))) {
       MOZ_NOT_REACHED("Couldn't get path from file!");
     }
 
     tabChild->CancelCachedFileDescriptorCallback(path, this);
   }
 
@@ -278,22 +280,43 @@ RemoteOpenFileChild::NotifyListener(nsre
   }
 }
 
 //-----------------------------------------------------------------------------
 // RemoteOpenFileChild::PRemoteOpenFileChild
 //-----------------------------------------------------------------------------
 
 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;
 }
 
 //-----------------------------------------------------------------------------
 // RemoteOpenFileChild::nsIFile functions that we override logic for
 //-----------------------------------------------------------------------------
--- a/netwerk/ipc/RemoteOpenFileChild.h
+++ b/netwerk/ipc/RemoteOpenFileChild.h
@@ -2,17 +2,16 @@
 /* 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 {
 
@@ -82,23 +81,24 @@ private:
   RemoteOpenFileChild(const RemoteOpenFileChild& other);
 
 protected:
   void AddIPDLReference()
   {
     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);
 
   // 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,51 +12,60 @@
 #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::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) {
-      printf_stderr("RemoteOpenFileParent: file '%s' was not found!\n",
-                    path.get());
-    } else {
-      fileDescriptor = FileDescriptor(fd);
+    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;
     }
   }
 
-  // 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());
-  }
-
+  // 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();
 #endif // OS_TYPE
 
   return true;
 }
 
 } // namespace net
 } // namespace mozilla
--- a/netwerk/ipc/RemoteOpenFileParent.h
+++ b/netwerk/ipc/RemoteOpenFileParent.h
@@ -13,22 +13,26 @@
 #include "nsIFileURL.h"
 
 namespace mozilla {
 namespace net {
 
 class RemoteOpenFileParent : public PRemoteOpenFileParent
 {
 public:
-  RemoteOpenFileParent(nsIFileURL* aURI)
-  : mURI(aURI)
-  {}
+  RemoteOpenFileParent(nsIFileURL* aURI);
 
-  bool OpenSendCloseDelete();
+ ~RemoteOpenFileParent();
+
+  virtual bool RecvAsyncOpenFile();
 
 private:
   nsCOMPtr<nsIFileURL> mURI;
+
+#if !defined(XP_WIN) && !defined(MOZ_WIDGET_COCOA)
+  int mFd;
+#endif
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // mozilla_net_RemoteOpenFileParent_h