Backed out changeset 84db7473edfb (bug 835575)
authorEd Morley <emorley@mozilla.com>
Thu, 07 Feb 2013 12:52:50 +0000
changeset 131816 6d5b6ce4a2ae90edd2d1206ef602dcebd84595bc
parent 131815 84db7473edfb4a47f861b09594a576d9c4ea88b2
child 131817 f21da1055c468cc998cec3815d2ac9364b7ca762
push id317
push userbbajaj@mozilla.com
push dateTue, 07 May 2013 01:20:33 +0000
treeherdermozilla-release@159a10910249 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs835575
milestone21.0a1
backs out84db7473edfb4a47f861b09594a576d9c4ea88b2
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 84db7473edfb (bug 835575)
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_NOTREACHED("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