Bug 1412824: Refactor MaybeCloseWindow and allow to reuse the window close code from within nsExternalHelperAppService as well as nsDSURIContentListener. r=smaug
authorChristoph Kerschbaumer <ckerschb@christophkerschbaumer.com>
Fri, 03 Nov 2017 13:23:25 +0100
changeset 443316 9eb29f6c93a36d7357ae72c03a6fc0ab57ca5baf
parent 443315 0c4ecb84046395afd7c5ed4a250b64991c9b0da7
child 443317 4acac146285e05979767dbab0296aad8e1ea0ddd
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1412824
milestone58.0a1
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
Bug 1412824: Refactor MaybeCloseWindow and allow to reuse the window close code from within nsExternalHelperAppService as well as nsDSURIContentListener. r=smaug
docshell/base/nsDSURIContentListener.cpp
docshell/base/nsDSURIContentListener.h
uriloader/exthandler/moz.build
uriloader/exthandler/nsExternalHelperAppService.cpp
uriloader/exthandler/nsExternalHelperAppService.h
--- a/docshell/base/nsDSURIContentListener.cpp
+++ b/docshell/base/nsDSURIContentListener.cpp
@@ -11,20 +11,82 @@
 #include "nsDocShellCID.h"
 #include "nsIWebNavigationInfo.h"
 #include "nsIDocument.h"
 #include "nsIDOMWindow.h"
 #include "nsIHttpChannel.h"
 #include "nsError.h"
 #include "nsContentSecurityManager.h"
 #include "nsDocShellLoadTypes.h"
+#include "nsIInterfaceRequestor.h"
 #include "nsIMultiPartChannel.h"
 
 using namespace mozilla;
 
+NS_IMPL_ADDREF(MaybeCloseWindowHelper)
+NS_IMPL_RELEASE(MaybeCloseWindowHelper)
+
+NS_INTERFACE_MAP_BEGIN(MaybeCloseWindowHelper)
+  NS_INTERFACE_MAP_ENTRY(nsISupports)
+NS_INTERFACE_MAP_END
+
+MaybeCloseWindowHelper::MaybeCloseWindowHelper(nsIInterfaceRequestor* aContentContext)
+  : mContentContext(aContentContext)
+  , mWindowToClose(nullptr)
+  , mTimer(nullptr)
+  , mShouldCloseWindow(false)
+{
+}
+
+MaybeCloseWindowHelper::~MaybeCloseWindowHelper()
+{
+}
+
+void
+MaybeCloseWindowHelper::SetShouldCloseWindow(bool aShouldCloseWindow)
+{
+  mShouldCloseWindow = aShouldCloseWindow;
+}
+
+nsIInterfaceRequestor*
+MaybeCloseWindowHelper::MaybeCloseWindow()
+{
+  nsCOMPtr<nsPIDOMWindowOuter> window = do_GetInterface(mContentContext);
+  NS_ENSURE_TRUE(window, mContentContext);
+
+  if (mShouldCloseWindow) {
+    // Reset the window context to the opener window so that the dependent
+    // dialogs have a parent
+    nsCOMPtr<nsPIDOMWindowOuter> opener = window->GetOpener();
+
+    if (opener && !opener->Closed()) {
+      mContentContext = do_GetInterface(opener);
+
+      // Now close the old window.  Do it on a timer so that we don't run
+      // into issues trying to close the window before it has fully opened.
+      NS_ASSERTION(!mTimer, "mTimer was already initialized once!");
+      NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, 0, nsITimer::TYPE_ONE_SHOT);
+      mWindowToClose = window;
+    }
+  }
+  return mContentContext;
+}
+
+NS_IMETHODIMP
+MaybeCloseWindowHelper::Notify(nsITimer* timer)
+{
+  NS_ASSERTION(mWindowToClose, "No window to close after timer fired");
+
+  mWindowToClose->Close();
+  mWindowToClose = nullptr;
+  mTimer = nullptr;
+
+  return NS_OK;
+}
+
 nsDSURIContentListener::nsDSURIContentListener(nsDocShell* aDocShell)
   : mDocShell(aDocShell)
   , mExistingJPEGRequest(nullptr)
   , mParentContentListener(nullptr)
 {
 }
 
 nsDSURIContentListener::~nsDSURIContentListener()
@@ -88,16 +150,27 @@ nsDSURIContentListener::DoContent(const 
   if (aOpenedChannel) {
     aOpenedChannel->GetLoadFlags(&loadFlags);
 
     // block top-level data URI navigations if triggered by the web
     if (!nsContentSecurityManager::AllowTopLevelNavigationToDataURI(aOpenedChannel)) {
       // logging to console happens within AllowTopLevelNavigationToDataURI
       aRequest->Cancel(NS_ERROR_DOM_BAD_URI);
       *aAbortProcess = true;
+      // close the window since the navigation to a data URI was blocked
+      if (mDocShell) {
+        nsCOMPtr<nsIInterfaceRequestor> contentContext =
+          do_QueryInterface(mDocShell->GetWindow());
+        if (contentContext) {
+          RefPtr<MaybeCloseWindowHelper> maybeCloseWindowHelper =
+            new MaybeCloseWindowHelper(contentContext);
+          maybeCloseWindowHelper->SetShouldCloseWindow(true);
+          maybeCloseWindowHelper->MaybeCloseWindow();
+        }
+      }
       return NS_OK; 
     }
   }
 
   if (loadFlags & nsIChannel::LOAD_RETARGETED_DOCUMENT_URI) {
     // XXX: Why does this not stop the content too?
     mDocShell->Stop(nsIWebNavigation::STOP_NETWORK);
 
--- a/docshell/base/nsDSURIContentListener.h
+++ b/docshell/base/nsDSURIContentListener.h
@@ -5,19 +5,64 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef nsDSURIContentListener_h__
 #define nsDSURIContentListener_h__
 
 #include "nsCOMPtr.h"
 #include "nsIURIContentListener.h"
 #include "nsWeakReference.h"
+#include "nsITimer.h"
 
 class nsDocShell;
+class nsIInterfaceRequestor;
 class nsIWebNavigationInfo;
+class nsPIDOMWindowOuter;
+
+// Helper Class to eventually close an already openend window
+class MaybeCloseWindowHelper final
+  : public nsITimerCallback
+{
+public:
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSITIMERCALLBACK
+
+  explicit MaybeCloseWindowHelper(nsIInterfaceRequestor* aContentContext);
+
+  /**
+   * Closes the provided window async (if mShouldCloseWindow is true)
+   * and returns its opener if the window was just openend.
+   */
+  nsIInterfaceRequestor* MaybeCloseWindow();
+
+  void SetShouldCloseWindow(bool aShouldCloseWindow);
+
+protected:
+  ~MaybeCloseWindowHelper();
+
+private:
+  /**
+   * The dom window associated to handle content.
+   */
+  nsCOMPtr<nsIInterfaceRequestor> mContentContext;
+
+  /**
+   * Used to close the window on a timer, to avoid any exceptions that are
+   * thrown if we try to close the window before it's fully loaded.
+   */
+  nsCOMPtr<nsPIDOMWindowOuter> mWindowToClose;
+  nsCOMPtr<nsITimer> mTimer;
+
+  /**
+   * This is set based on whether the channel indicates that a new window
+   * was opened, e.g. for a download, or was blocked. If so, then we
+   * close it.
+   */
+  bool mShouldCloseWindow;
+};
 
 class nsDSURIContentListener final
   : public nsIURIContentListener
   , public nsSupportsWeakReference
 {
   friend class nsDocShell;
 
 public:
--- a/uriloader/exthandler/moz.build
+++ b/uriloader/exthandler/moz.build
@@ -124,16 +124,17 @@ IPDL_SOURCES += [
     'PHandlerService.ipdl',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
 FINAL_LIBRARY = 'xul'
 
 LOCAL_INCLUDES += [
+    '/docshell/base',
     '/dom/base',
     '/dom/ipc',
     '/netwerk/base',
     '/netwerk/protocol/http',
 ]
 
 if CONFIG['MOZ_ENABLE_DBUS']:
     CXXFLAGS += CONFIG['TK_CFLAGS']
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -51,16 +51,17 @@
 #include "nsIDocumentLoader.h" // XXX needed to get orig. channel and assoc. refresh uri
 #include "nsIHelperAppLauncherDialog.h"
 #include "nsIContentDispatchChooser.h"
 #include "nsNetUtil.h"
 #include "nsIPrivateBrowsingChannel.h"
 #include "nsIIOService.h"
 #include "nsNetCID.h"
 
+#include "nsDSURIContentListener.h"
 #include "nsMimeTypes.h"
 // used for header disposition information.
 #include "nsIHttpChannel.h"
 #include "nsIHttpChannelInternal.h"
 #include "nsIEncodedChannel.h"
 #include "nsIMultiPartChannel.h"
 #include "nsIFileChannel.h"
 #include "nsIObserverService.h" // so we can be a profile change observer
@@ -1188,36 +1189,33 @@ NS_IMPL_ADDREF(nsExternalAppHandler)
 NS_IMPL_RELEASE(nsExternalAppHandler)
 
 NS_INTERFACE_MAP_BEGIN(nsExternalAppHandler)
    NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStreamListener)
    NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
    NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
    NS_INTERFACE_MAP_ENTRY(nsIHelperAppLauncher)
    NS_INTERFACE_MAP_ENTRY(nsICancelable)
-   NS_INTERFACE_MAP_ENTRY(nsITimerCallback)
    NS_INTERFACE_MAP_ENTRY(nsIBackgroundFileSaverObserver)
    NS_INTERFACE_MAP_ENTRY(nsINamed)
 NS_INTERFACE_MAP_END_THREADSAFE
 
 nsExternalAppHandler::nsExternalAppHandler(nsIMIMEInfo * aMIMEInfo,
                                            const nsACString& aTempFileExtension,
                                            nsIInterfaceRequestor* aContentContext,
                                            nsIInterfaceRequestor* aWindowContext,
                                            nsExternalHelperAppService *aExtProtSvc,
                                            const nsAString& aSuggestedFilename,
                                            uint32_t aReason, bool aForceSave)
 : mMimeInfo(aMIMEInfo)
 , mContentContext(aContentContext)
 , mWindowContext(aWindowContext)
-, mWindowToClose(nullptr)
 , mSuggestedFileName(aSuggestedFilename)
 , mForceSave(aForceSave)
 , mCanceled(false)
-, mShouldCloseWindow(false)
 , mStopRequestIssued(false)
 , mReason(aReason)
 , mContentLength(-1)
 , mProgress(0)
 , mSaver(nullptr)
 , mDialogProgressListener(nullptr)
 , mTransfer(nullptr)
 , mRequest(nullptr)
@@ -1583,23 +1581,25 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
     mIsFileChannel = parent && parent->WasFileChannel();
   }
 
   // Get content length
   if (aChannel) {
     aChannel->GetContentLength(&mContentLength);
   }
 
+  mMaybeCloseWindowHelper = new MaybeCloseWindowHelper(mContentContext);
+
   nsCOMPtr<nsIPropertyBag2> props(do_QueryInterface(request, &rv));
   // Determine whether a new window was opened specifically for this request
   if (props) {
     bool tmp = false;
     props->GetPropertyAsBool(NS_LITERAL_STRING("docshell.newWindowTarget"),
                              &tmp);
-    mShouldCloseWindow = tmp;
+    mMaybeCloseWindowHelper->SetShouldCloseWindow(tmp);
   }
 
   // Now get the URI
   if (aChannel) {
     aChannel->GetURI(getter_AddRefs(mSourceUrl));
   }
 
   // retarget all load notifications to our docloader instead of the original window's docloader...
@@ -1608,24 +1608,24 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
   // Check to see if there is a refresh header on the original channel.
   if (mOriginalChannel) {
     nsCOMPtr<nsIHttpChannel> httpChannel(do_QueryInterface(mOriginalChannel));
     if (httpChannel) {
       nsAutoCString refreshHeader;
       Unused << httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("refresh"),
                                                refreshHeader);
       if (!refreshHeader.IsEmpty()) {
-        mShouldCloseWindow = false;
+        mMaybeCloseWindowHelper->SetShouldCloseWindow(false);
       }
     }
   }
 
   // Close the underlying DOMWindow if there is no refresh header
   // and it was opened specifically for the download
-  MaybeCloseWindow();
+  mContentContext = mMaybeCloseWindowHelper->MaybeCloseWindow();
 
   // In an IPC setting, we're allowing the child process, here, to make
   // decisions about decoding the channel (e.g. decompression).  It will
   // still forward the decoded (uncompressed) data back to the parent.
   // Con: Uncompressed data means more IPC overhead.
   // Pros: ExternalHelperAppParent doesn't need to implement nsIEncodedChannel.
   //       Parent process doesn't need to expect CPU time on decompression.
   MaybeApplyDecodingForExtension(aChannel);
@@ -2518,52 +2518,16 @@ bool nsExternalAppHandler::GetNeverAskFl
   NS_UnescapeURL(prefCString);
   nsACString::const_iterator start, end;
   prefCString.BeginReading(start);
   prefCString.EndReading(end);
   return !CaseInsensitiveFindInReadable(nsDependentCString(aContentType),
                                         start, end);
 }
 
-nsresult nsExternalAppHandler::MaybeCloseWindow()
-{
-  nsCOMPtr<nsPIDOMWindowOuter> window = do_GetInterface(mContentContext);
-  NS_ENSURE_STATE(window);
-
-  if (mShouldCloseWindow) {
-    // Reset the window context to the opener window so that the dependent
-    // dialogs have a parent
-    nsCOMPtr<nsPIDOMWindowOuter> opener = window->GetOpener();
-
-    if (opener && !opener->Closed()) {
-      mContentContext = do_GetInterface(opener);
-
-      // Now close the old window.  Do it on a timer so that we don't run
-      // into issues trying to close the window before it has fully opened.
-      NS_ASSERTION(!mTimer, "mTimer was already initialized once!");
-      MOZ_TRY_VAR(mTimer, NS_NewTimerWithCallback(this, 0, nsITimer::TYPE_ONE_SHOT));
-      mWindowToClose = window;
-    }
-  }
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsExternalAppHandler::Notify(nsITimer* timer)
-{
-  NS_ASSERTION(mWindowToClose, "No window to close after timer fired");
-
-  mWindowToClose->Close();
-  mWindowToClose = nullptr;
-  mTimer = nullptr;
-
-  return NS_OK;
-}
-
 NS_IMETHODIMP
 nsExternalAppHandler::GetName(nsACString& aName)
 {
   aName.AssignLiteral("nsExternalAppHandler");
   return NS_OK;
 }
 
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////
--- a/uriloader/exthandler/nsExternalHelperAppService.h
+++ b/uriloader/exthandler/nsExternalHelperAppService.h
@@ -20,32 +20,31 @@
 #include "nsIStreamListener.h"
 #include "nsIFile.h"
 #include "nsIFileStreams.h"
 #include "nsIOutputStream.h"
 #include "nsString.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIChannel.h"
-#include "nsITimer.h"
 #include "nsIBackgroundFileSaver.h"
 
 #include "nsIHandlerService.h"
 #include "nsCOMPtr.h"
 #include "nsIObserver.h"
 #include "nsCOMArray.h"
 #include "nsWeakReference.h"
 #include "nsIPrompt.h"
 #include "nsAutoPtr.h"
 #include "mozilla/Attributes.h"
 
 class nsExternalAppHandler;
 class nsIMIMEInfo;
 class nsITransfer;
-class nsPIDOMWindowOuter;
+class MaybeCloseWindowHelper;
 
 /**
  * The helper app service. Responsible for handling content that Mozilla
  * itself can not handle
  */
 class nsExternalHelperAppService
 : public nsIExternalHelperAppService,
   public nsPIExternalAppLauncher,
@@ -206,27 +205,25 @@ private:
  * a nsIStreamListener. It saves the incoming data into a temp file. The handler
  * is bound to an application when it is created. When it receives an
  * OnStopRequest it launches the application using the temp file it has
  * stored the data into.  We create a handler every time we have to process
  * data using a helper app.
  */
 class nsExternalAppHandler final : public nsIStreamListener,
                                    public nsIHelperAppLauncher,
-                                   public nsITimerCallback,
                                    public nsIBackgroundFileSaverObserver,
                                    public nsINamed
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSIHELPERAPPLAUNCHER
   NS_DECL_NSICANCELABLE
-  NS_DECL_NSITIMERCALLBACK
   NS_DECL_NSIBACKGROUNDFILESAVEROBSERVER
   NS_DECL_NSINAMED
 
   /**
    * @param aMIMEInfo       MIMEInfo object, representing the type of the
    *                        content that should be handled
    * @param aFileExtension  The extension we need to append to our temp file,
    *                        INCLUDING the ".". e.g. .mp3
@@ -292,18 +289,17 @@ protected:
    * should use in parenting. If null, we use mContentContext.
    */
   nsCOMPtr<nsIInterfaceRequestor> mWindowContext;
 
   /**
    * Used to close the window on a timer, to avoid any exceptions that are
    * thrown if we try to close the window before it's fully loaded.
    */
-  nsCOMPtr<nsPIDOMWindowOuter> mWindowToClose;
-  nsCOMPtr<nsITimer> mTimer;
+  RefPtr<MaybeCloseWindowHelper> mMaybeCloseWindowHelper;
 
   /**
    * The following field is set if we were processing an http channel that had
    * a content disposition header which specified the SUGGESTED file name we
    * should present to the user in the save to disk dialog. 
    */
   nsString mSuggestedFileName;
 
@@ -316,23 +312,16 @@ protected:
   
   /**
    * The canceled flag is set if the user canceled the launching of this
    * application before we finished saving the data to a temp file.
    */
   bool mCanceled;
 
   /**
-   * This is set based on whether the channel indicates that a new window
-   * was opened specifically for this download.  If so, then we
-   * close it.
-   */
-  bool mShouldCloseWindow;
-
-  /**
    * True if a stop request has been issued.
    */
   bool mStopRequestIssued; 
 
   bool mIsFileChannel;
 
   /**
    * One of the REASON_ constants from nsIHelperAppLauncherDialog. Indicates the
@@ -465,23 +454,16 @@ protected:
 
   typedef enum { kReadError, kWriteError, kLaunchError } ErrorType;
   /**
    * Utility function to send proper error notification to web progress listener
    */
   void SendStatusChange(ErrorType type, nsresult aStatus, nsIRequest *aRequest, const nsString& path);
 
   /**
-   * Closes the window context if it does not have a refresh header
-   * and it never displayed content before the external helper app
-   * service was invoked.
-   */
-  nsresult MaybeCloseWindow();
-
-  /**
    * Set in nsHelperDlgApp.js. This is always null after the user has chosen an
    * action.
    */
   nsCOMPtr<nsIWebProgressListener2> mDialogProgressListener;
   /**
    * Set once the user has chosen an action. This is null after the download
    * has been canceled or completes.
    */