Bug 1551882. Refactor the print preview listener code to make its purpose clear. r=bobowen
authorJonathan Watt <jwatt@jwatt.org>
Thu, 16 May 2019 23:16:11 +0100
changeset 533131 b224c05dc123549420aa8747e1600c0bb68da0f6
parent 533130 3d36061643f440a3f06380e690bf38cfbe89892b
child 533132 8a7327b91ed243944681e5a82c1da2db7023d297
push id11276
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:11:24 +0000
treeherdermozilla-beta@847755a7c325 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1551882
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 1551882. Refactor the print preview listener code to make its purpose clear. r=bobowen Differential Revision: https://phabricator.services.mozilla.com/D31252
layout/printing/PrintPreviewUserEventSuppressor.cpp
layout/printing/PrintPreviewUserEventSuppressor.h
layout/printing/moz.build
layout/printing/nsPrintData.cpp
layout/printing/nsPrintData.h
layout/printing/nsPrintJob.cpp
layout/printing/nsPrintJob.h
layout/printing/nsPrintPreviewListener.cpp
layout/printing/nsPrintPreviewListener.h
rename from layout/printing/nsPrintPreviewListener.cpp
rename to layout/printing/PrintPreviewUserEventSuppressor.cpp
--- a/layout/printing/nsPrintPreviewListener.cpp
+++ b/layout/printing/PrintPreviewUserEventSuppressor.cpp
@@ -1,76 +1,59 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "nsPrintPreviewListener.h"
+#include "PrintPreviewUserEventSuppressor.h"
 
 #include "mozilla/TextEvents.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Event.h"  // for Event
 #include "nsIDOMWindow.h"
 #include "nsPIDOMWindow.h"
 #include "mozilla/dom/Document.h"
 #include "nsIDocShell.h"
 #include "nsPresContext.h"
 #include "nsFocusManager.h"
 #include "nsLiteralString.h"
 
-using namespace mozilla;
 using namespace mozilla::dom;
 
-NS_IMPL_ISUPPORTS(nsPrintPreviewListener, nsIDOMEventListener)
-
-//
-// nsPrintPreviewListener ctor
-//
-nsPrintPreviewListener::nsPrintPreviewListener(EventTarget* aTarget)
-    : mEventTarget(aTarget) {
-  NS_ADDREF_THIS();
-}  // ctor
+namespace mozilla {
 
-nsPrintPreviewListener::~nsPrintPreviewListener() {}
+PrintPreviewUserEventSuppressor::PrintPreviewUserEventSuppressor(
+    EventTarget* aTarget)
+    : mEventTarget(aTarget) {
+  AddListeners();
+}
 
-//-------------------------------------------------------
-//
-// AddListeners
-//
-// Subscribe to the events that will allow us to track various events.
-//
-nsresult nsPrintPreviewListener::AddListeners() {
+NS_IMPL_ISUPPORTS(PrintPreviewUserEventSuppressor, nsIDOMEventListener)
+
+void PrintPreviewUserEventSuppressor::AddListeners() {
   if (mEventTarget) {
     mEventTarget->AddEventListener(NS_LITERAL_STRING("click"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("contextmenu"), this,
                                    true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("keydown"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("keypress"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("keyup"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("mousedown"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("mousemove"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("mouseout"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("mouseover"), this, true);
     mEventTarget->AddEventListener(NS_LITERAL_STRING("mouseup"), this, true);
 
     mEventTarget->AddSystemEventListener(NS_LITERAL_STRING("keydown"), this,
                                          true);
   }
-
-  return NS_OK;
 }
 
-//-------------------------------------------------------
-//
-// RemoveListeners
-//
-// Unsubscribe from all the various events that we were listening to.
-//
-nsresult nsPrintPreviewListener::RemoveListeners() {
+void PrintPreviewUserEventSuppressor::RemoveListeners() {
   if (mEventTarget) {
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("click"), this, true);
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("contextmenu"), this,
                                       true);
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("keydown"), this, true);
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("keypress"), this,
                                       true);
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("keyup"), this, true);
@@ -82,34 +65,27 @@ nsresult nsPrintPreviewListener::RemoveL
                                       true);
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("mouseover"), this,
                                       true);
     mEventTarget->RemoveEventListener(NS_LITERAL_STRING("mouseup"), this, true);
 
     mEventTarget->RemoveSystemEventListener(NS_LITERAL_STRING("keydown"), this,
                                             true);
   }
-
-  return NS_OK;
 }
 
-//-------------------------------------------------------
-//
-// GetActionForEvent
-//
-// Helper function to let certain key events through
-//
 enum eEventAction {
   eEventAction_Tab,
   eEventAction_ShiftTab,
   eEventAction_Propagate,
   eEventAction_Suppress,
   eEventAction_StopPropagation
 };
 
+// Helper function to let certain key events through
 static eEventAction GetActionForEvent(Event* aEvent) {
   WidgetKeyboardEvent* keyEvent = aEvent->WidgetEventPtr()->AsKeyboardEvent();
   if (!keyEvent) {
     return eEventAction_Suppress;
   }
 
   if (keyEvent->mFlags.mInSystemGroup) {
     NS_ASSERTION(keyEvent->mMessage == eKeyDown,
@@ -145,21 +121,21 @@ static eEventAction GetActionForEvent(Ev
       return eEventAction_Propagate;
     }
   }
 
   return eEventAction_Suppress;
 }
 
 NS_IMETHODIMP
-nsPrintPreviewListener::HandleEvent(Event* aEvent) {
+PrintPreviewUserEventSuppressor::HandleEvent(Event* aEvent) {
   nsCOMPtr<nsIContent> content =
       do_QueryInterface(aEvent ? aEvent->GetOriginalTarget() : nullptr);
   if (content && !content->IsXULElement()) {
-    eEventAction action = ::GetActionForEvent(aEvent);
+    eEventAction action = GetActionForEvent(aEvent);
     switch (action) {
       case eEventAction_Tab:
       case eEventAction_ShiftTab: {
         nsAutoString eventString;
         aEvent->GetType(eventString);
         if (eventString.EqualsLiteral("keydown")) {
           // Handle tabbing explicitly here since we don't want focus ending up
           // inside the content document, bug 244128.
@@ -168,20 +144,19 @@ nsPrintPreviewListener::HandleEvent(Even
 
           Document* parentDoc = doc->GetParentDocument();
           NS_ASSERTION(parentDoc, "no parent document");
 
           nsCOMPtr<nsPIDOMWindowOuter> win = parentDoc->GetWindow();
 
           nsIFocusManager* fm = nsFocusManager::GetFocusManager();
           if (fm && win) {
-            dom::Element* fromElement =
-                parentDoc->FindContentForSubDocument(doc);
+            Element* fromElement = parentDoc->FindContentForSubDocument(doc);
             bool forward = (action == eEventAction_Tab);
-            RefPtr<dom::Element> result;
+            RefPtr<Element> result;
             fm->MoveFocus(win, fromElement,
                           forward ? nsIFocusManager::MOVEFOCUS_FORWARD
                                   : nsIFocusManager::MOVEFOCUS_BACKWARD,
                           nsIFocusManager::FLAG_BYKEY, getter_AddRefs(result));
           }
         }
       }
         MOZ_FALLTHROUGH;
@@ -194,8 +169,10 @@ nsPrintPreviewListener::HandleEvent(Even
         break;
       case eEventAction_Propagate:
         // intentionally empty
         break;
     }
   }
   return NS_OK;
 }
+
+}  // namespace mozilla
rename from layout/printing/nsPrintPreviewListener.h
rename to layout/printing/PrintPreviewUserEventSuppressor.h
--- a/layout/printing/nsPrintPreviewListener.h
+++ b/layout/printing/PrintPreviewUserEventSuppressor.h
@@ -1,51 +1,53 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef nsPrintPreviewListener_h__
-#define nsPrintPreviewListener_h__
+#ifndef mozilla_PrintPreviewUserEventSuppressor_h
+#define mozilla_PrintPreviewUserEventSuppressor_h
 
-// Interfaces needed to be included
+#include "nsCOMPtr.h"
 #include "nsIDOMEventListener.h"
-// Helper Classes
-#include "nsCOMPtr.h"
 #include "mozilla/Attributes.h"
 
 namespace mozilla {
+
 namespace dom {
 class EventTarget;
 }  // namespace dom
-}  // namespace mozilla
 
-//
-// class nsPrintPreviewListener
-//
-// The class that listens to the chrome events and tells the embedding
-// chrome to show context menus, as appropriate. Handles registering itself
-// with the DOM with AddChromeListeners() and removing itself with
-// RemoveChromeListeners().
-//
-class nsPrintPreviewListener final : public nsIDOMEventListener
-
-{
+/**
+ * A class that filters out certain user events targeted at the given event
+ * target (a document).  Intended for use with the Print Preview document to
+ * stop users from doing anything that would break printing invariants.  (For
+ * example, blocks opening of the context menu, interaction with form controls,
+ * content selection, etc.)
+ */
+class PrintPreviewUserEventSuppressor final : public nsIDOMEventListener {
  public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIDOMEVENTLISTENER
 
-  explicit nsPrintPreviewListener(mozilla::dom::EventTarget* aTarget);
+  explicit PrintPreviewUserEventSuppressor(dom::EventTarget* aTarget);
 
-  // Add/remove the relevant listeners, based on what interfaces
-  // the embedding chrome implements.
-  nsresult AddListeners();
-  nsresult RemoveListeners();
+  /**
+   * Must be called before releasing this object in order to break the strong
+   * reference cycle between ourselves and the document we're listening to,
+   * or else the objects in the cylce will be leaked (since this class does
+   * not participate in cycle collection).
+   */
+  void StopSuppressing() { RemoveListeners(); }
 
  private:
-  ~nsPrintPreviewListener();
+  ~PrintPreviewUserEventSuppressor() { RemoveListeners(); }
+
+  void AddListeners();
+  void RemoveListeners();
 
   nsCOMPtr<mozilla::dom::EventTarget> mEventTarget;
+};
 
-};  // class nsPrintPreviewListener
+}  // namespace mozilla
 
-#endif /* nsPrintPreviewListener_h__ */
+#endif  // mozilla_PrintPreviewUserEventSuppressor_h
--- a/layout/printing/moz.build
+++ b/layout/printing/moz.build
@@ -23,17 +23,17 @@ XPIDL_MODULE = 'layout_printing'
 UNIFIED_SOURCES += [
     'DrawEventRecorder.cpp',
     'ipc/RemotePrintJobChild.cpp',
     'ipc/RemotePrintJobParent.cpp',
     'nsPagePrintTimer.cpp',
     'nsPrintData.cpp',
     'nsPrintJob.cpp',
     'nsPrintObject.cpp',
-    'nsPrintPreviewListener.cpp',
+    'PrintPreviewUserEventSuppressor.cpp',
     'PrintTranslator.cpp',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
 FINAL_LIBRARY = 'xul'
 LOCAL_INCLUDES += [
     '../base',
--- a/layout/printing/nsPrintData.cpp
+++ b/layout/printing/nsPrintData.cpp
@@ -5,19 +5,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsPrintData.h"
 
 #include "nsIStringBundle.h"
 #include "nsIServiceManager.h"
 #include "nsIWidget.h"
 #include "nsPrintObject.h"
-#include "nsPrintPreviewListener.h"
 #include "nsIWebProgressListener.h"
 #include "mozilla/Services.h"
+#include "PrintPreviewUserEventSuppressor.h"
 
 //-----------------------------------------------------
 // PR LOGGING
 #include "mozilla/Logging.h"
 
 static mozilla::LazyLogModule gPrintingLog("printing");
 
 #define PR_PL(_p1) MOZ_LOG(gPrintingLog, mozilla::LogLevel::Debug, _p1);
@@ -33,18 +33,17 @@ nsPrintData::nsPrintData(ePrintDataType 
       mOnStartSent(false),
       mIsAborted(false),
       mPreparingForPrint(false),
       mDocWasToBeDestroyed(false),
       mShrinkToFit(false),
       mPrintFrameType(nsIPrintSettings::kFramesAsIs),
       mNumPrintablePages(0),
       mNumPagesPrinted(0),
-      mShrinkRatio(1.0),
-      mPPEventListeners(nullptr) {
+      mShrinkRatio(1.0) {
   nsCOMPtr<nsIStringBundle> brandBundle;
   nsCOMPtr<nsIStringBundleService> svc =
       mozilla::services::GetStringBundleService();
   if (svc) {
     svc->CreateBundle("chrome://branding/locale/brand.properties",
                       getter_AddRefs(brandBundle));
     if (brandBundle) {
       brandBundle->GetStringFromName("brandShortName", mBrandName);
@@ -52,20 +51,19 @@ nsPrintData::nsPrintData(ePrintDataType 
   }
 
   if (mBrandName.IsEmpty()) {
     mBrandName.AssignLiteral(u"Mozilla Document");
   }
 }
 
 nsPrintData::~nsPrintData() {
-  // remove the event listeners
-  if (mPPEventListeners) {
-    mPPEventListeners->RemoveListeners();
-    NS_RELEASE(mPPEventListeners);
+  if (mPPEventSuppressor) {
+    mPPEventSuppressor->StopSuppressing();
+    mPPEventSuppressor = nullptr;
   }
 
   // Only Send an OnEndPrinting if we have started printing
   if (mOnStartSent && mType != eIsPrintPreview) {
     OnEndPrinting();
   }
 
   if (mPrintDC) {
--- a/layout/printing/nsPrintData.h
+++ b/layout/printing/nsPrintData.h
@@ -13,36 +13,41 @@
 // Interfaces
 #include "nsDeviceContext.h"
 #include "nsIPrintProgressParams.h"
 #include "nsIPrintSettings.h"
 #include "nsISupportsImpl.h"
 #include "nsTArray.h"
 #include "nsCOMArray.h"
 
-// Classes
 class nsPrintObject;
-class nsPrintPreviewListener;
 class nsIWebProgressListener;
 
+namespace mozilla {
+class PrintPreviewUserEventSuppressor;
+}  // namespace mozilla
+
 //------------------------------------------------------------------------
 // nsPrintData Class
 //
 // mPreparingForPrint - indicates that we have started Printing but
 //   have not gone to the timer to start printing the pages. It gets turned
 //   off right before we go to the timer.
 //
 // mDocWasToBeDestroyed - Gets set when "someone" tries to unload the document
 //   while we were prparing to Print. This typically happens if a user starts
 //   to print while a page is still loading. If they start printing and pause
 //   at the print dialog and then the page comes in, we then abort printing
 //   because the document is no longer stable.
 //
 //------------------------------------------------------------------------
 class nsPrintData {
+  typedef mozilla::PrintPreviewUserEventSuppressor
+      PrintPreviewUserEventSuppressor;
+
  public:
   typedef enum { eIsPrinting, eIsPrintPreview } ePrintDataType;
 
   explicit nsPrintData(ePrintDataType aType);
 
   NS_INLINE_DECL_REFCOUNTING(nsPrintData)
 
   // Listener Helper Methods
@@ -77,17 +82,17 @@ class nsPrintData {
   bool mDocWasToBeDestroyed;  // see comments above
   bool mShrinkToFit;
   int16_t mPrintFrameType;
   int32_t mNumPrintablePages;
   int32_t mNumPagesPrinted;
   float mShrinkRatio;
 
   nsCOMPtr<nsIPrintSettings> mPrintSettings;
-  nsPrintPreviewListener* mPPEventListeners;
+  RefPtr<PrintPreviewUserEventSuppressor> mPPEventSuppressor;
 
   nsString mBrandName;  //  needed as a substitute name for a document
 
  private:
   nsPrintData() = delete;
   nsPrintData& operator=(const nsPrintData& aOther) = delete;
 
   ~nsPrintData();  // non-virtual
--- a/layout/printing/nsPrintJob.cpp
+++ b/layout/printing/nsPrintJob.cpp
@@ -33,18 +33,16 @@
 #include "nsIServiceManager.h"
 #include "nsGkAtoms.h"
 #include "nsXPCOM.h"
 #include "nsISupportsPrimitives.h"
 
 static const char sPrintSettingsServiceContractID[] =
     "@mozilla.org/gfx/printsettings-service;1";
 
-// Printing Events
-#include "nsPrintPreviewListener.h"
 #include "nsThreadUtils.h"
 
 // Printing
 #include "nsIWebBrowserPrint.h"
 
 // Print Preview
 #include "imgIContainer.h"  // image animation mode constants
 
@@ -111,16 +109,17 @@ static const char kPrintingPromptService
 #include "nsFocusManager.h"
 #include "nsRange.h"
 #include "nsIURIFixup.h"
 #include "mozilla/Components.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/HTMLFrameElement.h"
 #include "nsContentList.h"
 #include "nsIChannel.h"
+#include "PrintPreviewUserEventSuppressor.h"
 #include "xpcpublic.h"
 #include "nsVariant.h"
 #include "mozilla/ServoStyleSet.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 //-----------------------------------------------------
@@ -566,27 +565,26 @@ nsresult nsPrintJob::Cancelled() {
   return NS_ERROR_FAILURE;
 }
 
 //-------------------------------------------------------
 // Install our event listeners on the document to prevent
 // some events from being processed while in PrintPreview
 //
 // No return code - if this fails, there isn't much we can do
-void nsPrintJob::InstallPrintPreviewListener() {
-  if (!mPrt->mPPEventListeners) {
+void nsPrintJob::SuppressPrintPreviewUserEvents() {
+  if (!mPrt->mPPEventSuppressor) {
     nsCOMPtr<nsIDocShell> docShell = do_QueryReferent(mContainer);
     if (!docShell) {
       return;
     }
 
     if (nsPIDOMWindowOuter* win = docShell->GetWindow()) {
       nsCOMPtr<EventTarget> target = win->GetFrameElementInternal();
-      mPrt->mPPEventListeners = new nsPrintPreviewListener(target);
-      mPrt->mPPEventListeners->AddListeners();
+      mPrt->mPPEventSuppressor = new PrintPreviewUserEventSuppressor(target);
     }
   }
 }
 
 //-----------------------------------------------------------------
 nsresult nsPrintJob::GetSeqFrameAndCountPages(nsIFrame*& aSeqFrame,
                                               int32_t& aCount) {
   MOZ_ASSERT(mPrtPreview);
@@ -1002,17 +1000,17 @@ nsresult nsPrintJob::DoCommonPrint(bool 
   if (aIsPrintPreview) {
     bool notifyOnInit = false;
     ShowPrintProgress(false, notifyOnInit);
 
     // Very important! Turn Off scripting
     TurnScriptingOn(false);
 
     if (!notifyOnInit) {
-      InstallPrintPreviewListener();
+      SuppressPrintPreviewUserEvents();
       rv = InitPrintDocConstruction(false);
     } else {
       rv = NS_OK;
     }
   } else {
     bool doNotify;
     ShowPrintProgress(true, doNotify);
     if (!doNotify) {
--- a/layout/printing/nsPrintJob.h
+++ b/layout/printing/nsPrintJob.h
@@ -104,17 +104,23 @@ class nsPrintJob final : public nsIObser
   bool PrintDocContent(const mozilla::UniquePtr<nsPrintObject>& aPO,
                        nsresult& aStatus);
   nsresult DoPrint(const mozilla::UniquePtr<nsPrintObject>& aPO);
 
   void SetPrintPO(nsPrintObject* aPO, bool aPrint);
 
   void TurnScriptingOn(bool aDoTurnOn);
   bool CheckDocumentForPPCaching();
-  void InstallPrintPreviewListener();
+
+  /**
+   * Filters out certain user events while Print Preview is open to prevent
+   * the user from interacting with the Print Preview document and breaking
+   * printing invariants.
+   */
+  void SuppressPrintPreviewUserEvents();
 
   // nsIDocumentViewerPrint Printing Methods
   bool HasPrintCallbackCanvas();
   bool PrePrintPage();
   bool PrintPage(nsPrintObject* aPOect, bool& aInRange);
   bool DonePrintingPages(nsPrintObject* aPO, nsresult aResult);
 
   //---------------------------------------------------------------------