Bug 1552449. Make the nsIDocShell interface to print preview less susceptible to inadvertent misuse. r=bobowen
authorJonathan Watt <jwatt@jwatt.org>
Tue, 07 May 2019 14:12:44 +0100
changeset 474357 73c9f2d6d3a067cd3f0a1982e22f60d70123ca21
parent 474356 62321377e204a921c6cf4282565756267fbea2dc
child 474358 930f0f51b681aea2a5e915a2770f80a9914ed3df
push id36031
push userrgurzau@mozilla.com
push dateFri, 17 May 2019 21:43:13 +0000
treeherdermozilla-central@1ae707852b60 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1552449
milestone68.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 1552449. Make the nsIDocShell interface to print preview less susceptible to inadvertent misuse. r=bobowen Differential Revision: https://phabricator.services.mozilla.com/D31598
docshell/base/nsDocShell.cpp
docshell/base/nsIDocShell.idl
layout/base/nsDocumentViewer.cpp
layout/base/tests/chrome/printpreview_bug396024_helper.xul
layout/base/tests/chrome/printpreview_bug482976_helper.xul
layout/base/tests/chrome/printpreview_helper.xul
toolkit/actors/PrintingChild.jsm
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -12911,17 +12911,17 @@ nsresult nsDocShell::CharsetChangeStopDo
   return NS_ERROR_DOCSHELL_REQUEST_REJECTED;
 }
 
 void nsDocShell::SetIsPrinting(bool aIsPrinting) {
   mIsPrintingOrPP = aIsPrinting;
 }
 
 NS_IMETHODIMP
-nsDocShell::GetPrintPreview(nsIWebBrowserPrint** aPrintPreview) {
+nsDocShell::InitOrReusePrintPreviewViewer(nsIWebBrowserPrint** aPrintPreview) {
   *aPrintPreview = nullptr;
 #if NS_PRINT_PREVIEW
   nsCOMPtr<nsIDocumentViewerPrint> print = do_QueryInterface(mContentViewer);
   if (!print || !print->IsInitializedForPrintPreview()) {
     // XXX: Creating a brand new content viewer to host preview every
     // time we enter here seems overwork. We could skip ahead to where
     // we QI the mContentViewer if the current URI is either about:blank
     // or about:printpreview.
@@ -12942,16 +12942,29 @@ nsDocShell::GetPrintPreview(nsIWebBrowse
   nsCOMPtr<nsIWebBrowserPrint> result = do_QueryInterface(print);
   result.forget(aPrintPreview);
   return NS_OK;
 #else
   return NS_ERROR_NOT_IMPLEMENTED;
 #endif
 }
 
+NS_IMETHODIMP nsDocShell::ExitPrintPreview() {
+#if NS_PRINT_PREVIEW
+#  ifdef DEBUG
+  nsCOMPtr<nsIDocumentViewerPrint> vp = do_QueryInterface(mContentViewer);
+  MOZ_ASSERT(vp && vp->IsInitializedForPrintPreview());
+#  endif
+  nsCOMPtr<nsIWebBrowserPrint> viewer = do_QueryInterface(mContentViewer);
+  return viewer->ExitPrintPreview();
+#else
+  return NS_OK;
+#endif
+}
+
 #ifdef DEBUG
 unsigned long nsDocShell::gNumberOfDocShells = 0;
 #endif
 
 NS_IMETHODIMP
 nsDocShell::GetCanExecuteScripts(bool* aResult) {
   *aResult = mCanExecuteScripts;
   return NS_OK;
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -544,21 +544,38 @@ interface nsIDocShell : nsIDocShellTreeI
 
   /**
    * Allows nsDocumentViewer to tell the top-level same-type docshell that
    * one of the documents under it is printing.
    */
   [noscript, notxpcom] void setIsPrinting(in boolean aIsPrinting);
 
   /**
-   * If the current content viewer isn't initialized for print preview,
-   * it is replaced with one which is and to which an about:blank document
-   * is loaded.
+   * This method should only be called on a docShell that has been specifically
+   * created to display a print preview document.  If the current document
+   * viewer isn't initialized for print preview when this method is called, it
+   * is replaced with a new viewer with an about:blank document (with the URL
+   * about:printpreview).  The viewer is then returned, ready for the print
+   * preview document to be constructed when viewer.printPreview() is called.
+   *
+   * The same viewer will be returned on subsequent calls since various bits of
+   * code assume that, once created, the viewer is never replaced.  Note,
+   * however, that the viewer's document will be replaced with a new document
+   * each time printPreview() is called on it (which is what we do to take
+   * account of print preview settings changes).  Replacing the document
+   * viewer's document breaks the normally unchanging 1:1 relationship between
+   * a document and its viewer, but that seems to be okay.
    */
-  readonly attribute nsIWebBrowserPrint printPreview;
+  nsIWebBrowserPrint initOrReusePrintPreviewViewer();
+
+  /**
+   * Propagated to the print preview document viewer.  Must only be called on
+   * a document viewer that has been initialized for print preview.
+   */
+  void exitPrintPreview();
 
   /**
    * Whether this docshell can execute scripts based on its hierarchy.
    * The rule of thumb here is that we disable js if this docshell or any
    * of its parents disallow scripting.
    */
   [infallible] readonly attribute boolean canExecuteScripts;
 
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -3546,17 +3546,17 @@ nsDocumentViewer::Print(nsIPrintSettings
 
 NS_IMETHODIMP
 nsDocumentViewer::PrintPreview(nsIPrintSettings* aPrintSettings,
                                mozIDOMWindowProxy* aChildDOMWin,
                                nsIWebProgressListener* aWebProgressListener) {
 #  if defined(NS_PRINTING) && defined(NS_PRINT_PREVIEW)
   MOZ_ASSERT(IsInitializedForPrintPreview(),
              "For print preview nsIWebBrowserPrint must be from "
-             "docshell.printPreview!");
+             "docshell.initOrReusePrintPreviewViewer!");
 
   NS_ENSURE_ARG_POINTER(aChildDOMWin);
   nsresult rv = NS_OK;
 
   if (GetIsPrinting()) {
     nsPrintJob::CloseProgressDialog(aWebProgressListener);
     return NS_ERROR_FAILURE;
   }
--- a/layout/base/tests/chrome/printpreview_bug396024_helper.xul
+++ b/layout/base/tests/chrome/printpreview_bug396024_helper.xul
@@ -12,17 +12,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 <script type="application/javascript">
 <![CDATA[
 var is = window.opener.wrappedJSObject.is;
 var ok = window.opener.wrappedJSObject.ok;
 var todo = window.opener.wrappedJSObject.todo;
 var SimpleTest = window.opener.wrappedJSObject.SimpleTest;
 var gWbp;
 function printpreview() {
-  gWbp = window.frames[1].docShell.printPreview;
+  gWbp = window.frames[1].docShell.initOrReusePrintPreviewViewer();
   var listener = {
     onLocationChange: function(webProgress, request, location, flags) { },
     onProgressChange: function(webProgress, request, curSelfProgress, 
                                maxSelfProgress, curTotalProgress,
                                maxTotalProgress) { },
     onSecurityChange: function(webProgress, request, state) { },
     onStateChange: function(webProgress, request, stateFlags, status) { },
     onStatusChange: function(webProgress, request, status, message) { },
@@ -39,17 +39,17 @@ function printpreview() {
   prefs.setBoolPref('print.show_print_progress', false);
   //XXX I would have thought this would work, instead I'm forced to use prefs service
   gWbp.globalPrintSettings.showPrintProgress = false; 
   gWbp.printPreview(gWbp.globalPrintSettings, window.frames[0], listener);
   prefs.clearUserPref('print.show_print_progress');
 }
 
 function exitprintpreview() {
-  window.frames[1].docShell.printPreview.exitPrintPreview();
+  window.frames[1].docShell.exitPrintPreview();
 }
 
 function finish() {
   SimpleTest.finish();
   window.close();
 }
 
 function run()
@@ -85,17 +85,17 @@ function run2() {
     document.getElementById("i").removeEventListener("load", arguments.callee, true);
     setTimeout(run3, 0);
   };
   document.getElementById("i").addEventListener("load", loadhandler, true);
   window.frames[0].location.reload();
 }
 
 function run3() {
-  gWbp = window.frames[1].docShell.printPreview;
+  gWbp = window.frames[1].docShell.initOrReusePrintPreviewViewer();
   ok(gWbp.doingPrintPreview, "Should be doing print preview");
   exitprintpreview();
   setTimeout(run4, 0);
 }
 
 function run4() {
   var i = document.getElementById("i");
   i.remove();
@@ -104,17 +104,17 @@ function run4() {
     setTimeout(run5, 0);
   };
   i.addEventListener("load", loadhandler, true);
   document.documentElement.getBoundingClientRect();
   document.documentElement.appendChild(i);
 }
 
 function run5() {
-  gWbp = window.frames[0].docShell.printPreview;
+  gWbp = window.frames[0].docShell.initOrReusePrintPreviewViewer();
   ok(!gWbp.doingPrintPreview, "Should not be doing print preview anymore2");
 
   //XXX this shouldn't be necessary, see bug 405555
   printpreview();
   exitprintpreview();
   finish(); //should not have crashed after all of this
 }
 ]]></script>
--- a/layout/base/tests/chrome/printpreview_bug482976_helper.xul
+++ b/layout/base/tests/chrome/printpreview_bug482976_helper.xul
@@ -12,17 +12,17 @@ https://bugzilla.mozilla.org/show_bug.cg
 <script type="application/javascript">
 <![CDATA[
 var is = window.opener.wrappedJSObject.is;
 var ok = window.opener.wrappedJSObject.ok;
 var todo = window.opener.wrappedJSObject.todo;
 var SimpleTest = window.opener.wrappedJSObject.SimpleTest;
 var gWbp;
 function printpreview() {
-  gWbp = window.frames[1].docShell.printPreview;
+  gWbp = window.frames[1].docShell.initOrReusePrintPreviewViewer();
   var listener = {
     onLocationChange: function(webProgress, request, location, flags) { },
     onProgressChange: function(webProgress, request, curSelfProgress, 
                                maxSelfProgress, curTotalProgress,
                                maxTotalProgress) { },
     onSecurityChange: function(webProgress, request, state) { },
     onStateChange: function(webProgress, request, stateFlags, status) { },
     onStatusChange: function(webProgress, request, status, message) { },
@@ -39,17 +39,17 @@ function printpreview() {
   prefs.setBoolPref('print.show_print_progress', false);
   //XXX I would have thought this would work, instead I'm forced to use prefs service
   gWbp.globalPrintSettings.showPrintProgress = false; 
   gWbp.printPreview(gWbp.globalPrintSettings, window.frames[0], listener);
   prefs.clearUserPref('print.show_print_progress');
 }
 
 function exitprintpreview() {
-  window.frames[1].docShell.printPreview.exitPrintPreview();
+  window.frames[1].docShell.exitPrintPreview();
 }
 
 function finish() {
   SimpleTest.finish();
   window.close();
 }
 
 function run1()
--- a/layout/base/tests/chrome/printpreview_helper.xul
+++ b/layout/base/tests/chrome/printpreview_helper.xul
@@ -19,17 +19,17 @@ var ctx2;
 var counter = 0;
 
 var file = Cc["@mozilla.org/file/directory_service;1"]
              .getService(Ci.nsIProperties)
              .get("TmpD", Ci.nsIFile);
 filePath = file.path;
 
 function printpreview(hasMozPrintCallback) {
-  gWbp = window.frames[1].docShell.printPreview;
+  gWbp = window.frames[1].docShell.initOrReusePrintPreviewViewer();
   var listener = {
     onLocationChange: function(webProgress, request, location, flags) { },
     onProgressChange: function(webProgress, request, curSelfProgress, 
                                maxSelfProgress, curTotalProgress,
                                maxTotalProgress) { },
     onSecurityChange: function(webProgress, request, state) { },
     onStateChange: function(webProgress, request, stateFlags, status) { },
     onStatusChange: function(webProgress, request, status, message) { },
@@ -60,17 +60,17 @@ function printpreview(hasMozPrintCallbac
     is(after, 1, "Should have called afterprint listener!");
   }
   window.frames[0].removeEventListener("beforeprint", beforeprint, true);
   window.frames[0].removeEventListener("afterprint", afterprint, true);
   prefs.clearUserPref('print.show_print_progress');
 }
 
 function exitprintpreview() {
-  window.frames[1].docShell.printPreview.exitPrintPreview();
+  window.frames[1].docShell.exitPrintPreview();
 }
 
 function finish() {
   SimpleTest.finish();
   window.close();
 }
 
 function runTests()
--- a/toolkit/actors/PrintingChild.jsm
+++ b/toolkit/actors/PrintingChild.jsm
@@ -283,17 +283,17 @@ class PrintingChild extends ActorChild {
       // The print preview docshell will be in a different TabGroup, so
       // printPreviewInitialize must be run in a separate runnable to avoid
       // touching a different TabGroup in our own runnable.
       let printPreviewInitialize = () => {
         try {
           let listener = new PrintingListener(this.mm);
 
           this.printPreviewInitializingInfo = { changingBrowsers };
-          docShell.printPreview.printPreview(printSettings, contentWindow, listener);
+          docShell.initOrReusePrintPreviewViewer().printPreview(printSettings, contentWindow, listener);
         } catch (error) {
           // This might fail if we, for example, attempt to print a XUL document.
           // In that case, we inform the parent to bail out of print preview.
           Cu.reportError(error);
           this.printPreviewInitializingInfo = null;
           this.mm.sendAsyncMessage("Printing:Preview:Entered", { failed: true });
         }
       };
@@ -313,17 +313,17 @@ class PrintingChild extends ActorChild {
       // In that case, we inform the parent to bail out of print preview.
       Cu.reportError(error);
       this.mm.sendAsyncMessage("Printing:Preview:Entered", { failed: true });
     }
   }
 
   exitPrintPreview(glo) {
     this.printPreviewInitializingInfo = null;
-    this.docShell.printPreview.exitPrintPreview();
+    this.docShell.initOrReusePrintPreviewViewer().exitPrintPreview();
   }
 
   print(contentWindow, simplifiedMode, defaultPrinterName) {
     let printSettings = this.getPrintSettings(defaultPrinterName);
     let printCancelled = false;
 
     // If we happen to be on simplified mode, we need to set docURL in order
     // to generate header/footer content correctly, since simplified tab has
@@ -380,24 +380,24 @@ class PrintingChild extends ActorChild {
   }
 
   logKeyedTelemetry(id, key) {
     let histogram = Services.telemetry.getKeyedHistogramById(id);
     histogram.add(key);
   }
 
   updatePageCount() {
-    let numPages = this.docShell.printPreview.printPreviewNumPages;
+    let numPages = this.docShell.initOrReusePrintPreviewViewer().printPreviewNumPages;
     this.mm.sendAsyncMessage("Printing:Preview:UpdatePageCount", {
       numPages,
     });
   }
 
   navigate(navType, pageNum) {
-    this.docShell.printPreview.printPreviewNavigate(navType, pageNum);
+    this.docShell.initOrReusePrintPreviewViewer().printPreviewNavigate(navType, pageNum);
   }
 }
 
 PrintingChild.prototype.QueryInterface =
   ChromeUtils.generateQI([Ci.nsIPrintingPromptService]);
 
 function PrintingListener(global) {
   this.global = global;