Bug 1557878. Replace nsIWebBrowserPrint::enumerateDocumentNames with a getter for the top document's name. r=bobowen
authorJonathan Watt <jwatt@jwatt.org>
Thu, 30 May 2019 11:43:16 +0100
changeset 477990 5db384ff08c5dcb7d35668d97a3c28bbc30797e5
parent 477989 9cb41597752d0e6177d14663b164d7f486d595d9
child 477991 bc502c949746cad77572cd1132c2c5e22067eccf
push id113390
push userjwatt@jwatt.org
push dateSun, 09 Jun 2019 11:50:07 +0000
treeherdermozilla-inbound@5db384ff08c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1557878
milestone69.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 1557878. Replace nsIWebBrowserPrint::enumerateDocumentNames with a getter for the top document's name. r=bobowen Differential Revision: https://phabricator.services.mozilla.com/D34260
layout/base/nsDocumentViewer.cpp
layout/printing/nsPrintJob.cpp
layout/printing/nsPrintJob.h
toolkit/components/browser/nsIWebBrowserPrint.idl
toolkit/components/printingui/ipc/PrintDataUtils.cpp
widget/cocoa/nsPrintDialogX.mm
widget/cocoa/nsPrintSettingsServiceX.mm
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -3743,48 +3743,41 @@ nsDocumentViewer::GetCurrentPrintSetting
   *aCurrentPrintSettings = nullptr;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
   *aCurrentPrintSettings = mPrintJob->GetCurrentPrintSettings().take();
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsDocumentViewer::GetDocumentName(nsAString& aDocName) {
+#  ifdef NS_PRINTING
+  return mPrintJob->GetDocumentName(aDocName);
+#  else
+  return NS_ERROR_FAILURE;
+#  endif
+}
+
+NS_IMETHODIMP
 nsDocumentViewer::Cancel() {
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
   return mPrintJob->Cancel();
 }
 
 NS_IMETHODIMP
 nsDocumentViewer::ExitPrintPreview() {
   if (GetIsPrinting()) return NS_ERROR_FAILURE;
   NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
 
   if (GetIsPrintPreview()) {
     ReturnToGalleyPresentation();
   }
   return NS_OK;
 }
 
-//----------------------------------------------------------------------------------
-// Enumerate all the documents for their titles
-NS_IMETHODIMP
-nsDocumentViewer::EnumerateDocumentNames(uint32_t* aCount,
-                                         char16_t*** aResult) {
-#  ifdef NS_PRINTING
-  NS_ENSURE_ARG(aCount);
-  NS_ENSURE_ARG_POINTER(aResult);
-  NS_ENSURE_TRUE(mPrintJob, NS_ERROR_FAILURE);
-
-  return mPrintJob->EnumerateDocumentNames(aCount, aResult);
-#  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);
 
   *aPrintPreviewNumPages = mPrintJob->GetPrintPreviewNumPages();
   return *aPrintPreviewNumPages > 0 ? NS_OK : NS_ERROR_FAILURE;
--- a/layout/printing/nsPrintJob.cpp
+++ b/layout/printing/nsPrintJob.cpp
@@ -1101,45 +1101,22 @@ int32_t nsPrintJob::GetPrintPreviewNumPa
   nsresult rv = GetSeqFrameAndCountPagesInternal(printData->mPrintObject,
                                                  seqFrame, numPages);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return 0;
   }
   return numPages;
 }
 
-//----------------------------------------------------------------------------------
-// Enumerate all the documents for their titles
-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*));
-
-  for (int32_t i = 0; i < numDocs; i++) {
-    nsPrintObject* po = mPrt->mPrintDocList.ElementAt(i);
-    NS_ASSERTION(po, "nsPrintObject can't be null!");
-    nsAutoString docTitleStr;
-    nsAutoString docURLStr;
-    GetDocumentTitleAndURL(po->mDocument, docTitleStr, docURLStr);
-
-    // Use the URL if the doc is empty
-    if (docTitleStr.IsEmpty() && !docURLStr.IsEmpty()) {
-      docTitleStr = docURLStr;
-    }
-    array[i] = ToNewUnicode(docTitleStr);
+nsresult nsPrintJob::GetDocumentName(nsAString& aDocName) {
+  nsAutoString docURLStr;
+  GetDocumentTitleAndURL(mPrt->mPrintObject->mDocument, aDocName, docURLStr);
+  if (aDocName.IsEmpty() && !docURLStr.IsEmpty()) {
+    aDocName = docURLStr;
   }
-  *aCount = numDocs;
-  *aResult = array;
-
   return NS_OK;
 }
 
 //----------------------------------------------------------------------------------
 nsresult nsPrintJob::GetGlobalPrintSettings(
     nsIPrintSettings** aGlobalPrintSettings) {
   NS_ENSURE_ARG_POINTER(aGlobalPrintSettings);
 
--- a/layout/printing/nsPrintJob.h
+++ b/layout/printing/nsPrintJob.h
@@ -112,18 +112,17 @@ class nsPrintJob final : public nsIObser
                         nsIWebProgressListener* aWebProgressListener);
 
   bool IsDoingPrint() const { return mIsDoingPrinting; }
   bool IsDoingPrintPreview() const { return mIsDoingPrintPreview; }
   bool IsIFrameSelected();
   bool IsRangeSelection();
   /// 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);
+  nsresult GetDocumentName(nsAString& aDocName);
   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();
--- a/toolkit/components/browser/nsIWebBrowserPrint.idl
+++ b/toolkit/components/browser/nsIWebBrowserPrint.idl
@@ -41,16 +41,23 @@ interface nsIWebBrowserPrint : nsISuppor
    * that was passed into either "print" or "print preview"
    *
    * This enables any consumers of the interface to have access
    * to the "current" PrintSetting at later points in the execution
    */
   readonly attribute nsIPrintSettings currentPrintSettings;
 
   /**
+   * The "name" of the document that is to be printed.  This is the document's
+   * title, unless that's empty, in which case it is a sanitized version of the
+   * document's URL.
+   */
+  readonly attribute AString documentName;
+
+  /**
    * Returns whether it is in Print mode
    */
   readonly attribute boolean doingPrint;
 
   /**
    * Returns whether it is in Print Preview mode
    */
   readonly attribute boolean doingPrintPreview;
@@ -105,26 +112,15 @@ interface nsIWebBrowserPrint : nsISuppor
 
   /**
    * Cancels the current print
    * @return void
    */
   void cancel();
 
   /**
-   * Returns an array of the names of all documents names (Title or URL)
-   * and sub-documents. This will return a single item if the attr "isFramesetDocument" is false
-   * and may return any number of items is "isFramesetDocument" is true
-   *
-   * @param  aCount - returns number of printers returned
-   * @param  aResult - returns array of names
-   * @return void
-   */
-  void enumerateDocumentNames(out uint32_t aCount,[retval, array, size_is(aCount)] out wstring aResult);
-
-  /**
    * This exists PrintPreview mode and returns browser window to galley mode
    * @return void
    */
   void exitPrintPreview();
 
 };
 
--- a/toolkit/components/printingui/ipc/PrintDataUtils.cpp
+++ b/toolkit/components/printingui/ipc/PrintDataUtils.cpp
@@ -34,16 +34,23 @@ MockWebBrowserPrint::GetGlobalPrintSetti
 
 NS_IMETHODIMP
 MockWebBrowserPrint::GetCurrentPrintSettings(
     nsIPrintSettings** aCurrentPrintSettings) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
+MockWebBrowserPrint::GetDocumentName(nsAString& aDocName) {
+  // The only consumer that cares about this is the OS X printing dialog.
+  aDocName = mData.printJobName();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 MockWebBrowserPrint::GetDoingPrint(bool* aDoingPrint) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 MockWebBrowserPrint::GetDoingPrintPreview(bool* aDoingPrintPreview) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -82,34 +89,12 @@ NS_IMETHODIMP
 MockWebBrowserPrint::PrintPreviewNavigate(int16_t aNavType, int32_t aPageNum) {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 MockWebBrowserPrint::Cancel() { return NS_ERROR_NOT_IMPLEMENTED; }
 
 NS_IMETHODIMP
-MockWebBrowserPrint::EnumerateDocumentNames(uint32_t* aCount,
-                                            char16_t*** aResult) {
-  *aCount = 0;
-  *aResult = nullptr;
-
-  if (mData.printJobName().IsEmpty()) {
-    return NS_OK;
-  }
-
-  // The only consumer that cares about this is the OS X printing
-  // dialog, and even then, it only cares about the first document
-  // name. That's why we only send a single document name through
-  // PrintData.
-  char16_t** array = (char16_t**)moz_xmalloc(sizeof(char16_t*));
-  array[0] = ToNewUnicode(mData.printJobName());
-
-  *aCount = 1;
-  *aResult = array;
-  return NS_OK;
-}
-
-NS_IMETHODIMP
 MockWebBrowserPrint::ExitPrintPreview() { return NS_ERROR_NOT_IMPLEMENTED; }
 
 }  // namespace embedding
 }  // namespace mozilla
--- a/widget/cocoa/nsPrintDialogX.mm
+++ b/widget/cocoa/nsPrintDialogX.mm
@@ -48,38 +48,31 @@ nsPrintDialogServiceX::Show(nsPIDOMWindo
       do_GetService("@mozilla.org/gfx/printsettings-service;1");
 
   // Set the printer name and then read the saved printer settings
   // from prefs. Reading printer-specific prefs requires the printer
   // name to be set.
   settingsX->SetPrinterNameFromPrintInfo();
   printSettingsSvc->InitPrintSettingsFromPrefs(settingsX, true, nsIPrintSettings::kInitSaveAll);
   // Set the print job title
-  char16_t** docTitles;
-  uint32_t titleCount;
-  nsresult rv = aWebBrowserPrint->EnumerateDocumentNames(&titleCount, &docTitles);
-  if (NS_SUCCEEDED(rv) && titleCount > 0) {
+  nsAutoString docName;
+  nsresult rv = aWebBrowserPrint->GetDocumentName(docName);
+  if (NS_SUCCEEDED(rv)) {
     // Print Core of Application Service sent print job with names exceeding
     // 255 bytes. This is a workaround until fix it.
     // (https://openradar.appspot.com/34428043)
     nsAutoString adjustedTitle;
-    PrintTarget::AdjustPrintJobNameForIPP(nsDependentString(docTitles[0]), adjustedTitle);
+    PrintTarget::AdjustPrintJobNameForIPP(docName, adjustedTitle);
     CFStringRef cfTitleString = CFStringCreateWithCharacters(
         NULL, reinterpret_cast<const UniChar*>(adjustedTitle.BeginReading()),
         adjustedTitle.Length());
     if (cfTitleString) {
       ::PMPrintSettingsSetJobName(settingsX->GetPMPrintSettings(), cfTitleString);
       CFRelease(cfTitleString);
     }
-    for (int32_t i = titleCount - 1; i >= 0; i--) {
-      free(docTitles[i]);
-    }
-    free(docTitles);
-    docTitles = NULL;
-    titleCount = 0;
   }
 
   NSPrintInfo* printInfo = settingsX->GetCocoaPrintInfo();
 
   // Put the print info into the current print operation, since that's where
   // [panel runModal] will look for it. We create the view because otherwise
   // we'll get unrelated warnings printed to the console.
   NSView* tmpView = [[NSView alloc] init];
--- a/widget/cocoa/nsPrintSettingsServiceX.mm
+++ b/widget/cocoa/nsPrintSettingsServiceX.mm
@@ -43,33 +43,22 @@ nsresult nsPrintSettingsServiceX::Serial
                                                             nsIWebBrowserPrint* aWBP,
                                                             PrintData* data) {
   // If we are in the child process, we don't need to populate
   // nsPrintSettingsX completely. The parent discards almost all of
   // this data (bug 1328975). Furthermore, reading some of the
   // printer/printing settings from the OS causes a connection to the
   // printer to be made which is blocked by sandboxing and results in hangs.
   if (aWBP) {
-    // When serializing an nsIWebBrowserPrint, we need to pass up the first
-    // document name. We could pass up the entire collection of document
-    // names, but the OS X printing prompt code only really cares about
-    // the first one, so we just send the first to save IPC traffic.
-    char16_t** docTitles;
-    uint32_t titleCount;
-    nsresult rv = aWBP->EnumerateDocumentNames(&titleCount, &docTitles);
+    // When serializing an nsIWebBrowserPrint, we need to pass up the
+    // document name.
+    nsAutoString docName;
+    nsresult rv = aWBP->GetDocumentName(docName);
     if (NS_SUCCEEDED(rv)) {
-      if (titleCount > 0) {
-        data->printJobName().Assign(docTitles[0]);
-      }
-
-      for (int32_t i = titleCount - 1; i >= 0; i--) {
-        free(docTitles[i]);
-      }
-      free(docTitles);
-      docTitles = nullptr;
+      data->printJobName().Assign(docName.get());
     }
   }
 
   return NS_OK;
 }
 
 nsresult nsPrintSettingsServiceX::SerializeToPrintDataParent(nsIPrintSettings* aSettings,
                                                              nsIWebBrowserPrint* aWBP,