Bug 1347983 - Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process, r=smaug
authorMichael Layzell <michael@thelayzells.com>
Thu, 16 Mar 2017 14:32:26 -0400
changeset 347985 3b2c7853ae71bd4ec36eb0c8be1a5a4200ade4f8
parent 347984 79bacf0ea46664a09a1f09996e70d0d3a4af042d
child 347986 38ecd019ecebe7533852db0253393d2708527437
push id88130
push usermichael@thelayzells.com
push dateThu, 16 Mar 2017 18:33:08 +0000
treeherdermozilla-inbound@38ecd019eceb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1347983
milestone55.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 1347983 - Part 1: Make forms submitted from a large-allocation page not leave the large-allocation process, r=smaug MozReview-Commit-ID: 7SEdTJN9Xd2
browser/base/content/browser.js
browser/modules/E10SUtils.jsm
docshell/base/nsDocShell.cpp
toolkit/components/browser/nsIWebBrowserChrome3.idl
xpfe/appshell/nsContentTreeOwner.cpp
xpfe/appshell/nsIXULBrowserWindow.idl
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4469,30 +4469,33 @@ var XULBrowserWindow = {
   // Called before links are navigated to to allow us to retarget them if needed.
   onBeforeLinkTraversal(originalTarget, linkURI, linkNode, isAppTab) {
     let target = BrowserUtils.onBeforeLinkTraversal(originalTarget, linkURI, linkNode, isAppTab);
     SocialUI.closeSocialPanelForLinkTraversal(target, linkNode);
     return target;
   },
 
   // Check whether this URI should load in the current process
-  shouldLoadURI(aDocShell, aURI, aReferrer, aTriggeringPrincipal) {
+  shouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData, aTriggeringPrincipal) {
     if (!gMultiProcessBrowser)
       return true;
 
     let browser = aDocShell.QueryInterface(Ci.nsIDocShellTreeItem)
                            .sameTypeRootTreeItem
                            .QueryInterface(Ci.nsIDocShell)
                            .chromeEventHandler;
 
     // Ignore loads that aren't in the main tabbrowser
     if (browser.localName != "browser" || !browser.getTabBrowser || browser.getTabBrowser() != gBrowser)
       return true;
 
-    if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aReferrer)) {
+    if (!E10SUtils.shouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData)) {
+      // XXX: Do we want to complain if we have post data but are still
+      // redirecting the load? Perhaps a telemetry probe? Theoretically we
+      // shouldn't do this, as it throws out data. See bug 1348018.
       E10SUtils.redirectLoad(aDocShell, aURI, aReferrer, aTriggeringPrincipal, false);
       return false;
     }
 
     return true;
   },
 
   onProgressChange(aWebProgress, aRequest,
--- a/browser/modules/E10SUtils.jsm
+++ b/browser/modules/E10SUtils.jsm
@@ -189,25 +189,27 @@ this.E10SUtils = {
     }
   },
 
   shouldLoadURIInThisProcess(aURI) {
     let remoteType = Services.appinfo.remoteType;
     return remoteType == this.getRemoteTypeForURIObject(aURI, true, remoteType);
   },
 
-  shouldLoadURI(aDocShell, aURI, aReferrer) {
+  shouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData) {
     // Inner frames should always load in the current process
     if (aDocShell.QueryInterface(Ci.nsIDocShellTreeItem).sameTypeParent)
       return true;
 
     // If we are in a Large-Allocation process, and it wouldn't be content visible
     // to change processes, we want to load into a new process so that we can throw
-    // this one out.
-    if (Services.appinfo.remoteType == LARGE_ALLOCATION_REMOTE_TYPE &&
+    // this one out. We don't want to move into a new process if we have post data,
+    // because we would accidentally throw out that data.
+    if (!aHasPostData &&
+        Services.appinfo.remoteType == LARGE_ALLOCATION_REMOTE_TYPE &&
         !aDocShell.awaitingLargeAlloc &&
         aDocShell.isOnlyToplevelInTabGroup) {
       return false;
     }
 
     // If the URI can be loaded in the current process then continue
     return this.shouldLoadURIInThisProcess(aURI);
   },
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -10606,17 +10606,18 @@ nsDocShell::InternalLoad(nsIURI* aURI,
     mTiming->NotifyUnloadAccepted(mCurrentURI);
   }
 
   // Check if the webbrowser chrome wants the load to proceed; this can be
   // used to cancel attempts to load URIs in the wrong process.
   nsCOMPtr<nsIWebBrowserChrome3> browserChrome3 = do_GetInterface(mTreeOwner);
   if (browserChrome3) {
     bool shouldLoad;
-    rv = browserChrome3->ShouldLoadURI(this, aURI, aReferrer, aTriggeringPrincipal, &shouldLoad);
+    rv = browserChrome3->ShouldLoadURI(this, aURI, aReferrer, !!aPostData,
+                                       aTriggeringPrincipal, &shouldLoad);
     if (NS_SUCCEEDED(rv) && !shouldLoad) {
       return NS_OK;
     }
   }
 
   if (browserChrome3 && aCheckForPrerender) {
     nsCOMPtr<nsIRunnable> ev =
       new InternalLoadEvent(this, aURI, aOriginalURI, aLoadReplace,
--- a/toolkit/components/browser/nsIWebBrowserChrome3.idl
+++ b/toolkit/components/browser/nsIWebBrowserChrome3.idl
@@ -40,22 +40,27 @@ interface nsIWebBrowserChrome3 : nsIWebB
    * Determines whether a load should continue.
    *
    * @param aDocShell
    *        The docshell performing the load.
    * @param aURI
    *        The URI being loaded.
    * @param aReferrer
    *        The referrer of the load.
+   * @param aHasPostData
+   *        True if the load which is being asked about has associated post data
+   *        which would be discarded if the load was redirected across process
+   *        boundaries.
    * @param aTriggeringPrincipal
    *        The principal that initiated the load of aURI.
    */
   bool shouldLoadURI(in nsIDocShell    aDocShell,
                      in nsIURI         aURI,
                      in nsIURI         aReferrer,
+                     in boolean        aHasPostData,
                      in nsIPrincipal   aTriggeringPrincipal);
 
   bool shouldLoadURIInThisProcess(in nsIURI aURI);
 
   /**
    * Attempts to load the currently loaded page into a fresh process to increase
    * available memory.
    *
--- a/xpfe/appshell/nsContentTreeOwner.cpp
+++ b/xpfe/appshell/nsContentTreeOwner.cpp
@@ -386,26 +386,27 @@ NS_IMETHODIMP nsContentTreeOwner::OnBefo
   
   _retval = originalTarget;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsContentTreeOwner::ShouldLoadURI(nsIDocShell *aDocShell,
                                                 nsIURI *aURI,
                                                 nsIURI *aReferrer,
+                                                bool aHasPostData,
                                                 nsIPrincipal* aTriggeringPrincipal,
                                                 bool *_retval)
 {
   NS_ENSURE_STATE(mXULWindow);
 
   nsCOMPtr<nsIXULBrowserWindow> xulBrowserWindow;
   mXULWindow->GetXULBrowserWindow(getter_AddRefs(xulBrowserWindow));
 
   if (xulBrowserWindow)
-    return xulBrowserWindow->ShouldLoadURI(aDocShell, aURI, aReferrer,
+    return xulBrowserWindow->ShouldLoadURI(aDocShell, aURI, aReferrer, aHasPostData,
                                            aTriggeringPrincipal, _retval);
 
   *_retval = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP nsContentTreeOwner::ShouldLoadURIInThisProcess(nsIURI* aURI,
                                                              bool* aRetVal)
--- a/xpfe/appshell/nsIXULBrowserWindow.idl
+++ b/xpfe/appshell/nsIXULBrowserWindow.idl
@@ -56,22 +56,27 @@ interface nsIXULBrowserWindow : nsISuppo
    * Determines whether a load should continue.
    *
    * @param aDocShell
    *        The docshell performing the load.
    * @param aURI
    *        The URI being loaded.
    * @param aReferrer
    *        The referrer of the load.
+   * @param aHasPostData
+   *        True if the load which is being asked about has associated post data
+   *        which would be discarded if the load was redirected across process
+   *        boundaries.
    * @param aTriggeringPrincipal
    *        The principal that initiated the load of aURI.
    */
   bool shouldLoadURI(in nsIDocShell    aDocShell,
                      in nsIURI         aURI,
                      in nsIURI         aReferrer,
+                     in boolean        aHasPostData,
                      in nsIPrincipal   aTriggeringPrincipal);
   /**
    * Show/hide a tooltip (when the user mouses over a link, say).
    */
   void showTooltip(in long x, in long y, in AString tooltip, in AString direction);
   void hideTooltip();
 
   /**