Bug 1549844. DeCOMtaminate some nsPrintJob methods. r=bobowen
authorJonathan Watt <jwatt@jwatt.org>
Fri, 12 Apr 2019 14:53:26 +0100
changeset 531869 efed47379383b1fc34b70f4bb78dbc5b694e2107
parent 531868 1ec1cbe74a0ad042b662e6577f709da5639968a6
child 531870 220720dbc492807edbcc203d1dab95b632ec4a00
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1549844
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 1549844. DeCOMtaminate some nsPrintJob methods. r=bobowen Differential Revision: https://phabricator.services.mozilla.com/D30269
layout/base/nsDocumentViewer.cpp
layout/printing/nsPrintJob.cpp
layout/printing/nsPrintJob.h
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -1818,19 +1818,17 @@ nsDocumentViewer::Destroy() {
   // references.  If we do this stuff in the destructor, the
   // destructor might never be called (especially if we're being
   // used from JS.
 
 #ifdef NS_PRINTING
   if (mPrintJob) {
     RefPtr<nsPrintJob> printJob = std::move(mPrintJob);
 #  ifdef NS_PRINT_PREVIEW
-    bool doingPrintPreview;
-    printJob->GetDoingPrintPreview(&doingPrintPreview);
-    if (doingPrintPreview) {
+    if (printJob->IsDoingPrintPreview()) {
       printJob->FinishPrintPreview();
     }
 #  endif
     printJob->Destroy();
     MOZ_ASSERT(!mPrintJob,
                "mPrintJob shouldn't be recreated while destroying it");
   }
 #endif
@@ -3718,45 +3716,40 @@ nsDocumentViewer::GetGlobalPrintSettings
   return nsPrintJob::GetGlobalPrintSettings(aGlobalPrintSettings);
 }
 
 // XXX This always returns false for subdocuments
 NS_IMETHODIMP
 nsDocumentViewer::GetDoingPrint(bool* aDoingPrint) {
   NS_ENSURE_ARG_POINTER(aDoingPrint);
 
-  *aDoingPrint = false;
-  if (mPrintJob) {
-    // XXX shouldn't this be GetDoingPrint() ?
-    return mPrintJob->GetDoingPrintPreview(aDoingPrint);
-  }
+  // XXX shouldn't this be GetDoingPrint() ?
+  *aDoingPrint = mPrintJob ? mPrintJob->IsDoingPrintPreview() : false;
   return NS_OK;
 }
 
 // XXX This always returns false for subdocuments
 NS_IMETHODIMP
 nsDocumentViewer::GetDoingPrintPreview(bool* aDoingPrintPreview) {
   NS_ENSURE_ARG_POINTER(aDoingPrintPreview);
 
-  *aDoingPrintPreview = false;
-  if (mPrintJob) {
-    return mPrintJob->GetDoingPrintPreview(aDoingPrintPreview);
-  }
+  *aDoingPrintPreview = mPrintJob ? mPrintJob->IsDoingPrintPreview() : false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetCurrentPrintSettings(
     nsIPrintSettings** aCurrentPrintSettings) {
   NS_ENSURE_ARG_POINTER(aCurrentPrintSettings);
 
   *aCurrentPrintSettings = nullptr;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
-  return mPrintJob->GetCurrentPrintSettings(aCurrentPrintSettings);
+  *aCurrentPrintSettings = mPrintJob->GetCurrentPrintSettings().take();
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::Cancel() {
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
   return mPrintJob->Cancelled();
 }
 
@@ -3788,65 +3781,70 @@ nsDocumentViewer::EnumerateDocumentNames
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetIsFramesetFrameSelected(bool* aIsFramesetFrameSelected) {
 #  ifdef NS_PRINTING
   *aIsFramesetFrameSelected = false;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
-  return mPrintJob->GetIsFramesetFrameSelected(aIsFramesetFrameSelected);
+  *aIsFramesetFrameSelected = mPrintJob->IsFramesetFrameSelected();
+  return NS_OK;
 #  else
   return NS_ERROR_FAILURE;
 #  endif
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetPrintPreviewNumPages(int32_t* aPrintPreviewNumPages) {
 #  ifdef NS_PRINTING
   NS_ENSURE_ARG_POINTER(aPrintPreviewNumPages);
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
-  return mPrintJob->GetPrintPreviewNumPages(aPrintPreviewNumPages);
+  *aPrintPreviewNumPages = mPrintJob->GetPrintPreviewNumPages();
+  return *aPrintPreviewNumPages > 0 ? NS_OK : NS_ERROR_FAILURE;
 #  else
   return NS_ERROR_FAILURE;
 #  endif
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetIsFramesetDocument(bool* aIsFramesetDocument) {
 #  ifdef NS_PRINTING
   *aIsFramesetDocument = false;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
-  return mPrintJob->GetIsFramesetDocument(aIsFramesetDocument);
+  *aIsFramesetDocument = mPrintJob->IsFramesetDocument();
+  return NS_OK;
 #  else
   return NS_ERROR_FAILURE;
 #  endif
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetIsIFrameSelected(bool* aIsIFrameSelected) {
 #  ifdef NS_PRINTING
   *aIsIFrameSelected = false;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
-  return mPrintJob->GetIsIFrameSelected(aIsIFrameSelected);
+  *aIsIFrameSelected = mPrintJob->IsIFrameSelected();
+  return NS_OK;
 #  else
   return NS_ERROR_FAILURE;
 #  endif
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::GetIsRangeSelection(bool* aIsRangeSelection) {
 #  ifdef NS_PRINTING
   *aIsRangeSelection = false;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
-  return mPrintJob->GetIsRangeSelection(aIsRangeSelection);
+  *aIsRangeSelection = mPrintJob->IsRangeSelection();
+  return NS_OK;
 #  else
   return NS_ERROR_FAILURE;
 #  endif
 }
 
 //----------------------------------------------------------------------------------
 // Printing/Print Preview Helpers
 //----------------------------------------------------------------------------------
--- a/layout/printing/nsPrintJob.cpp
+++ b/layout/printing/nsPrintJob.cpp
@@ -982,33 +982,31 @@ nsresult nsPrintJob::DoCommonPrint(bool 
 
   // We will enable scripting later after printing has finished.
   scriptSuppressor.Disconnect();
 
   return NS_OK;
 }
 
 //---------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::Print(nsIPrintSettings* aPrintSettings,
-                  nsIWebProgressListener* aWebProgressListener) {
+nsresult nsPrintJob::Print(nsIPrintSettings* aPrintSettings,
+                           nsIWebProgressListener* aWebProgressListener) {
   // If we have a print preview document, use that instead of the original
   // mDocument. That way animated images etc. get printed using the same state
   // as in print preview.
   Document* doc = mPrtPreview && mPrtPreview->mPrintObject
                       ? mPrtPreview->mPrintObject->mDocument
                       : mDocument;
 
   return CommonPrint(false, aPrintSettings, aWebProgressListener, doc);
 }
 
-NS_IMETHODIMP
-nsPrintJob::PrintPreview(nsIPrintSettings* aPrintSettings,
-                         mozIDOMWindowProxy* aChildDOMWin,
-                         nsIWebProgressListener* aWebProgressListener) {
+nsresult nsPrintJob::PrintPreview(
+    nsIPrintSettings* aPrintSettings, mozIDOMWindowProxy* aChildDOMWin,
+    nsIWebProgressListener* aWebProgressListener) {
   // Get the DocShell and see if it is busy
   // (We can't Print Preview this document if it is still busy)
   nsCOMPtr<nsIDocShell> docShell(do_QueryReferent(mContainer));
   NS_ENSURE_STATE(docShell);
 
   auto busyFlags = docShell->GetBusyFlags();
   if (busyFlags != nsIDocShell::BUSY_FLAGS_NONE) {
     CloseProgressDialog(aWebProgressListener);
@@ -1021,87 +1019,74 @@ nsPrintJob::PrintPreview(nsIPrintSetting
   nsCOMPtr<Document> doc = window->GetDoc();
   NS_ENSURE_STATE(doc);
 
   // Document is not busy -- go ahead with the Print Preview
   return CommonPrint(true, aPrintSettings, aWebProgressListener, doc);
 }
 
 //----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetIsFramesetDocument(bool* aIsFramesetDocument) {
+bool nsPrintJob::IsFramesetDocument() const {
   nsCOMPtr<nsIDocShell> webContainer(do_QueryReferent(mContainer));
-  *aIsFramesetDocument = IsParentAFrameSet(webContainer);
-  return NS_OK;
+  return IsParentAFrameSet(webContainer);
 }
 
 //----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetIsIFrameSelected(bool* aIsIFrameSelected) {
-  *aIsIFrameSelected = false;
-
+bool nsPrintJob::IsIFrameSelected() {
   // Get the docshell for this documentviewer
   nsCOMPtr<nsIDocShell> webContainer(do_QueryReferent(mContainer));
   // Get the currently focused window
   nsCOMPtr<nsPIDOMWindowOuter> currentFocusWin = FindFocusedDOMWindow();
   if (currentFocusWin && webContainer) {
     // Get whether the doc contains a frameset
     // Also, check to see if the currently focus docshell
     // is a child of this docshell
     bool isParentFrameSet;
-    *aIsIFrameSelected = IsThereAnIFrameSelected(webContainer, currentFocusWin,
-                                                 isParentFrameSet);
+    return IsThereAnIFrameSelected(webContainer, currentFocusWin,
+                                   isParentFrameSet);
   }
-  return NS_OK;
-}
-
-//----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetIsRangeSelection(bool* aIsRangeSelection) {
-  // Get the currently focused window
-  nsCOMPtr<nsPIDOMWindowOuter> currentFocusWin = FindFocusedDOMWindow();
-  *aIsRangeSelection = IsThereARangeSelection(currentFocusWin);
-  return NS_OK;
+  return false;
 }
 
 //----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetIsFramesetFrameSelected(bool* aIsFramesetFrameSelected) {
+bool nsPrintJob::IsRangeSelection() {
   // Get the currently focused window
   nsCOMPtr<nsPIDOMWindowOuter> currentFocusWin = FindFocusedDOMWindow();
-  *aIsFramesetFrameSelected = currentFocusWin != nullptr;
-  return NS_OK;
+  return IsThereARangeSelection(currentFocusWin);
 }
 
 //----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetPrintPreviewNumPages(int32_t* aPrintPreviewNumPages) {
-  NS_ENSURE_ARG_POINTER(aPrintPreviewNumPages);
-
-  nsIFrame* seqFrame = nullptr;
-  *aPrintPreviewNumPages = 0;
-
+bool nsPrintJob::IsFramesetFrameSelected() const {
+  // Get the currently focused window
+  nsCOMPtr<nsPIDOMWindowOuter> currentFocusWin = FindFocusedDOMWindow();
+  return currentFocusWin != nullptr;
+}
+
+//----------------------------------------------------------------------------------
+int32_t nsPrintJob::GetPrintPreviewNumPages() {
   // When calling this function, the FinishPrintPreview() function might not
   // been called as there are still some
   RefPtr<nsPrintData> printData = mPrtPreview ? mPrtPreview : mPrt;
   if (NS_WARN_IF(!printData)) {
-    return NS_ERROR_FAILURE;
+    return 0;
   }
-  nsresult rv = GetSeqFrameAndCountPagesInternal(
-      printData->mPrintObject, seqFrame, *aPrintPreviewNumPages);
+  nsIFrame* seqFrame = nullptr;
+  int32_t numPages = 0;
+  nsresult rv = GetSeqFrameAndCountPagesInternal(printData->mPrintObject,
+                                                 seqFrame, numPages);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return NS_ERROR_FAILURE;
+    return 0;
   }
-  return NS_OK;
+  return numPages;
 }
 
 //----------------------------------------------------------------------------------
 // Enumerate all the documents for their titles
-NS_IMETHODIMP
-nsPrintJob::EnumerateDocumentNames(uint32_t* aCount, char16_t*** aResult) {
+nsresult nsPrintJob::EnumerateDocumentNames(uint32_t* aCount,
+                                            char16_t*** aResult) {
   NS_ENSURE_ARG(aCount);
   NS_ENSURE_ARG_POINTER(aResult);
 
   *aCount = 0;
   *aResult = nullptr;
 
   int32_t numDocs = mPrt->mPrintDocList.Length();
   char16_t** array = (char16_t**)moz_xmalloc(numDocs * sizeof(char16_t*));
@@ -1135,47 +1120,24 @@ nsresult nsPrintJob::GetGlobalPrintSetti
       do_GetService(sPrintSettingsServiceContractID, &rv);
   if (NS_SUCCEEDED(rv)) {
     rv = printSettingsService->GetGlobalPrintSettings(aGlobalPrintSettings);
   }
   return rv;
 }
 
 //----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetDoingPrint(bool* aDoingPrint) {
-  NS_ENSURE_ARG_POINTER(aDoingPrint);
-  *aDoingPrint = mIsDoingPrinting;
-  return NS_OK;
-}
-
-//----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetDoingPrintPreview(bool* aDoingPrintPreview) {
-  NS_ENSURE_ARG_POINTER(aDoingPrintPreview);
-  *aDoingPrintPreview = mIsDoingPrintPreview;
-  return NS_OK;
-}
-
-//----------------------------------------------------------------------------------
-NS_IMETHODIMP
-nsPrintJob::GetCurrentPrintSettings(nsIPrintSettings** aCurrentPrintSettings) {
-  NS_ENSURE_ARG_POINTER(aCurrentPrintSettings);
-
+already_AddRefed<nsIPrintSettings> nsPrintJob::GetCurrentPrintSettings() {
   if (mPrt) {
-    *aCurrentPrintSettings = mPrt->mPrintSettings;
-
-  } else if (mPrtPreview) {
-    *aCurrentPrintSettings = mPrtPreview->mPrintSettings;
-
-  } else {
-    *aCurrentPrintSettings = nullptr;
+    return do_AddRef(mPrt->mPrintSettings);
   }
-  NS_IF_ADDREF(*aCurrentPrintSettings);
-  return NS_OK;
+  if (mPrtPreview) {
+    return do_AddRef(mPrtPreview->mPrintSettings);
+  }
+  return nullptr;
 }
 
 //-----------------------------------------------------------------
 //-- Section: Pre-Reflow Methods
 //-----------------------------------------------------------------
 
 //---------------------------------------------------------------------
 // This method checks to see if there is at least one printer defined
@@ -2849,17 +2811,17 @@ void nsPrintJob::CleanupDocTitleArray(ch
   free(aArray);
   aArray = nullptr;
   aCount = 0;
 }
 
 /** ---------------------------------------------------
  *  Get the Focused Frame for a documentviewer
  */
-already_AddRefed<nsPIDOMWindowOuter> nsPrintJob::FindFocusedDOMWindow() {
+already_AddRefed<nsPIDOMWindowOuter> nsPrintJob::FindFocusedDOMWindow() const {
   nsIFocusManager* fm = nsFocusManager::GetFocusManager();
   NS_ENSURE_TRUE(fm, nullptr);
 
   nsPIDOMWindowOuter* window = mDocument->GetWindow();
   NS_ENSURE_TRUE(window, nullptr);
 
   nsCOMPtr<nsPIDOMWindowOuter> rootWindow = window->GetPrivateRoot();
   NS_ENSURE_TRUE(rootWindow, nullptr);
@@ -2873,17 +2835,17 @@ already_AddRefed<nsPIDOMWindowOuter> nsP
   if (IsWindowsInOurSubTree(focusedWindow)) {
     return focusedWindow.forget();
   }
 
   return nullptr;
 }
 
 //---------------------------------------------------------------------
-bool nsPrintJob::IsWindowsInOurSubTree(nsPIDOMWindowOuter* window) {
+bool nsPrintJob::IsWindowsInOurSubTree(nsPIDOMWindowOuter* window) const {
   bool found = false;
 
   // now check to make sure it is in "our" tree of docshells
   if (window) {
     nsCOMPtr<nsIDocShell> docShell = window->GetDocShell();
 
     if (docShell) {
       // get this DocViewer docshell
--- a/layout/printing/nsPrintJob.h
+++ b/layout/printing/nsPrintJob.h
@@ -54,31 +54,33 @@ class nsPrintJob final : public nsIObser
   // nsISupports interface...
   NS_DECL_ISUPPORTS
 
   // nsIObserver
   NS_DECL_NSIOBSERVER
 
   NS_DECL_NSIWEBPROGRESSLISTENER
 
-  // Old nsIWebBrowserPrint methods; not cleaned up yet
-  NS_IMETHOD Print(nsIPrintSettings* aPrintSettings,
-                   nsIWebProgressListener* aWebProgressListener);
-  NS_IMETHOD PrintPreview(nsIPrintSettings* aPrintSettings,
-                          mozIDOMWindowProxy* aChildDOMWin,
-                          nsIWebProgressListener* aWebProgressListener);
-  NS_IMETHOD GetIsFramesetDocument(bool* aIsFramesetDocument);
-  NS_IMETHOD GetIsIFrameSelected(bool* aIsIFrameSelected);
-  NS_IMETHOD GetIsRangeSelection(bool* aIsRangeSelection);
-  NS_IMETHOD GetIsFramesetFrameSelected(bool* aIsFramesetFrameSelected);
-  NS_IMETHOD GetPrintPreviewNumPages(int32_t* aPrintPreviewNumPages);
-  NS_IMETHOD EnumerateDocumentNames(uint32_t* aCount, char16_t*** aResult);
-  NS_IMETHOD GetDoingPrint(bool* aDoingPrint);
-  NS_IMETHOD GetDoingPrintPreview(bool* aDoingPrintPreview);
-  NS_IMETHOD GetCurrentPrintSettings(nsIPrintSettings** aCurrentPrintSettings);
+  // Our nsIWebBrowserPrint implementation defers to these methods.
+  nsresult Print(nsIPrintSettings* aPrintSettings,
+                 nsIWebProgressListener* aWebProgressListener);
+  nsresult PrintPreview(nsIPrintSettings* aPrintSettings,
+                        mozIDOMWindowProxy* aChildDOMWin,
+                        nsIWebProgressListener* aWebProgressListener);
+  bool IsDoingPrint() const { return mIsDoingPrinting; }
+  bool IsDoingPrintPreview() const { return mIsDoingPrintPreview; }
+  bool IsFramesetDocument() const;
+  bool IsIFrameSelected();
+  bool IsRangeSelection();
+  bool IsFramesetFrameSelected() const;
+  /// If the returned value is not greater than zero, an error occurred.
+  int32_t GetPrintPreviewNumPages();
+  /// Callers are responsible for free'ing aResult.
+  nsresult EnumerateDocumentNames(uint32_t* aCount, char16_t*** aResult);
+  already_AddRefed<nsIPrintSettings> GetCurrentPrintSettings();
 
   // This enum tells indicates what the default should be for the title
   // if the title from the document is null
   enum eDocTitleDefault { eDocTitleDefBlank, eDocTitleDefURLDoc };
 
   void Destroy();
   void DestroyPrintingData();
 
@@ -142,23 +144,23 @@ class nsPrintJob final : public nsIObser
   bool IsThereARangeSelection(nsPIDOMWindowOuter* aDOMWin);
 
   void FirePrintingErrorEvent(nsresult aPrintError);
   //---------------------------------------------------------------------
 
   // Timer Methods
   nsresult StartPagePrintTimer(const mozilla::UniquePtr<nsPrintObject>& aPO);
 
-  bool IsWindowsInOurSubTree(nsPIDOMWindowOuter* aDOMWindow);
+  bool IsWindowsInOurSubTree(nsPIDOMWindowOuter* aDOMWindow) const;
   bool IsThereAnIFrameSelected(nsIDocShell* aDocShell,
                                nsPIDOMWindowOuter* aDOMWin,
                                bool& aIsParentFrameSet);
 
   // get the currently infocus frame for the document viewer
-  already_AddRefed<nsPIDOMWindowOuter> FindFocusedDOMWindow();
+  already_AddRefed<nsPIDOMWindowOuter> FindFocusedDOMWindow() const;
 
   void GetDisplayTitleAndURL(const mozilla::UniquePtr<nsPrintObject>& aPO,
                              nsAString& aTitle, nsAString& aURLStr,
                              eDocTitleDefault aDefType);
 
   bool CheckBeforeDestroy();
   nsresult Cancelled();