Bug 1552490: rename `nsCopySupport::HTMLCopy`, change `nsDocumentEncoder::IsVisible`, add unit to buffer size name. r=hsivonen
authorMirko Brodesser <mbrodesser@mozilla.com>
Mon, 20 May 2019 07:43:43 +0000
changeset 474472 1d9f4477a199f8c33b520683384a11f22b4e51ce
parent 474471 e3d25aeb90a855b036e95bdf7f3fe9101822a46f
child 474473 c19a74cafdb7b009cd6d5c800c904754e2dc614e
push id36040
push userrgurzau@mozilla.com
push dateMon, 20 May 2019 13:43:21 +0000
treeherdermozilla-central@319a369ccde4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershsivonen
bugs1552490
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 1552490: rename `nsCopySupport::HTMLCopy`, change `nsDocumentEncoder::IsVisible`, add unit to buffer size name. r=hsivonen Because it states more clearly what the functions and the constant are about. Differential Revision: https://phabricator.services.mozilla.com/D31615
dom/base/nsCopySupport.cpp
dom/base/nsCopySupport.h
dom/base/nsDocumentEncoder.cpp
dom/base/nsINode.h
layout/generic/nsFrameSelection.cpp
--- a/dom/base/nsCopySupport.cpp
+++ b/dom/base/nsCopySupport.cpp
@@ -341,19 +341,19 @@ static nsresult PutToClipboard(
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = clipboard->SetData(transferable, nullptr, aClipboardID);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return rv;
 }
 
-nsresult nsCopySupport::HTMLCopy(Selection* aSel, Document* aDoc,
-                                 int16_t aClipboardID,
-                                 bool aWithRubyAnnotation) {
+nsresult nsCopySupport::EncodeDocumentWithContextAndPutToClipboard(
+    Selection* aSel, Document* aDoc, int16_t aClipboardID,
+    bool aWithRubyAnnotation) {
   NS_ENSURE_TRUE(aDoc, NS_ERROR_NULL_POINTER);
 
   uint32_t additionalFlags = nsIDocumentEncoder::SkipInvisibleContent;
   if (aWithRubyAnnotation) {
     additionalFlags |= nsIDocumentEncoder::OutputRubyAnnotation;
   }
 
   EncodedDocumentWithContext encodedDocumentWithContext;
@@ -910,18 +910,18 @@ bool nsCopySupport::FireClipboardEvent(E
         }
         return false;
       }
       // XXX Code which decides whether we should copy text with ruby
       // annotation is currenct depending on whether each range of the
       // selection is inside a same ruby container. But we really should
       // expose the full functionality in browser. See bug 1130891.
       bool withRubyAnnotation = IsSelectionInsideRuby(sel);
-      // call the copy code
-      nsresult rv = HTMLCopy(sel, doc, aClipboardType, withRubyAnnotation);
+      nsresult rv = EncodeDocumentWithContextAndPutToClipboard(
+          sel, doc, aClipboardType, withRubyAnnotation);
       if (NS_FAILED(rv)) {
         return false;
       }
     } else {
       return false;
     }
   } else if (clipboardData) {
     // check to see if any data was put on the data transfer.
--- a/dom/base/nsCopySupport.h
+++ b/dom/base/nsCopySupport.h
@@ -28,32 +28,31 @@ class Selection;
 class nsCopySupport {
   // class of static helper functions for copy support
  public:
   static nsresult ClearSelectionCache();
 
   /**
    * @param aDoc Needs to be not nullptr.
    */
-  static nsresult HTMLCopy(mozilla::dom::Selection* aSel,
-                           mozilla::dom::Document* aDoc, int16_t aClipboardID,
-                           bool aWithRubyAnnotation);
+  static nsresult EncodeDocumentWithContextAndPutToClipboard(
+      mozilla::dom::Selection* aSel, mozilla::dom::Document* aDoc,
+      int16_t aClipboardID, bool aWithRubyAnnotation);
 
   // Get the selection, or entire document, in the format specified by the mime
   // type (text/html or text/plain). If aSel is non-null, use it, otherwise get
   // the entire doc.
   static nsresult GetContents(const nsACString& aMimeType, uint32_t aFlags,
                               mozilla::dom::Selection* aSel,
                               mozilla::dom::Document* aDoc, nsAString& outdata);
 
   static nsresult ImageCopy(nsIImageLoadingContent* aImageElement,
                             nsILoadContext* aLoadContext, int32_t aCopyFlags);
 
-  // Get the selection as a transferable. Similar to HTMLCopy except does
-  // not deal with the clipboard.
+  // Get the selection as a transferable.
   // @param aSelection Can be nullptr.
   // @param aDocument Needs to be not nullptr.
   // @param aTransferable Needs to be not nullptr.
   static nsresult GetTransferableForSelection(
       mozilla::dom::Selection* aSelection, mozilla::dom::Document* aDocument,
       nsITransferable** aTransferable);
 
   // Same as GetTransferableForSelection, but doesn't skip invisible content.
--- a/dom/base/nsDocumentEncoder.cpp
+++ b/dom/base/nsDocumentEncoder.cpp
@@ -80,55 +80,56 @@ class nsDocumentEncoder : public nsIDocu
 
   virtual int32_t GetImmediateContextCount(
       const nsTArray<nsINode*>& aAncestorArray) {
     return -1;
   }
 
   nsresult FlushText(nsAString& aString, bool aForce);
 
-  bool IsVisibleNode(nsINode* aNode) {
-    MOZ_ASSERT(aNode, "null node");
-
+  bool IsInvisibleNodeAndShouldBeSkipped(nsINode& aNode) const {
     if (mFlags & SkipInvisibleContent) {
       // Treat the visibility of the ShadowRoot as if it were
       // the host content.
       //
       // FIXME(emilio): I suspect instead of this a bunch of the GetParent()
       // calls here should be doing GetFlattenedTreeParent, then this condition
       // should be unreachable...
-      if (ShadowRoot* shadowRoot = ShadowRoot::FromNode(aNode)) {
-        aNode = shadowRoot->GetHost();
+      nsINode* node{&aNode};
+      if (ShadowRoot* shadowRoot = ShadowRoot::FromNode(node)) {
+        node = shadowRoot->GetHost();
       }
 
-      if (aNode->IsContent()) {
-        nsIFrame* frame = aNode->AsContent()->GetPrimaryFrame();
+      if (node->IsContent()) {
+        nsIFrame* frame = node->AsContent()->GetPrimaryFrame();
         if (!frame) {
-          if (aNode->IsElement() && aNode->AsElement()->IsDisplayContents()) {
-            return true;
+          if (node->IsElement() && node->AsElement()->IsDisplayContents()) {
+            return false;
           }
-          if (aNode->IsText()) {
+          if (node->IsText()) {
             // We have already checked that our parent is visible.
             //
             // FIXME(emilio): Text not assigned to a <slot> in Shadow DOM should
             // probably return false...
-            return true;
+            return false;
           }
-          if (aNode->IsHTMLElement(nsGkAtoms::rp)) {
+          if (node->IsHTMLElement(nsGkAtoms::rp)) {
             // Ruby parentheses are part of ruby structure, hence
             // shouldn't be stripped out even if it is not displayed.
-            return true;
+            return false;
           }
-          return false;
+          return true;
         }
         bool isVisible = frame->StyleVisibility()->IsVisible();
-        if (!isVisible && aNode->IsText()) return false;
+        if (!isVisible && node->IsText()) {
+          return true;
+        }
       }
     }
-    return true;
+    return false;
   }
 
   virtual bool IncludeInContext(nsINode* aNode);
 
   void Clear();
 
   class MOZ_STACK_CLASS AutoReleaseDocumentIfNeeded final {
    public:
@@ -344,17 +345,17 @@ nsresult nsDocumentEncoder::SerializeNod
     } else if (aOriginalNode.IsText()) {
       const nsCOMPtr<nsINode> parent = aOriginalNode.GetParent();
       if (parent && parent->IsElement()) {
         mSerializer->ScanElementForPreformat(parent->AsElement());
       }
     }
   }
 
-  if (!IsVisibleNode(&aOriginalNode)) {
+  if (IsInvisibleNodeAndShouldBeSkipped(aOriginalNode)) {
     return NS_OK;
   }
 
   FixupNodeDeterminer fixupNodeDeterminer{mNodeFixup, aFixupNode,
                                           aOriginalNode};
   nsINode* node = &fixupNodeDeterminer.GetFixupNodeFallBackToOriginalNode();
 
   if (node->IsElement()) {
@@ -406,17 +407,17 @@ nsresult nsDocumentEncoder::SerializeNod
     } else if (aNode.IsText()) {
       const nsCOMPtr<nsINode> parent = aNode.GetParent();
       if (parent && parent->IsElement()) {
         mSerializer->ForgetElementForPreformat(parent->AsElement());
       }
     }
   }
 
-  if (!IsVisibleNode(&aNode)) {
+  if (IsInvisibleNodeAndShouldBeSkipped(aNode)) {
     return NS_OK;
   }
 
   if (aNode.IsElement()) {
     mSerializer->AppendElementEnd(aNode.AsElement(), aStr);
   }
   return NS_OK;
 }
@@ -424,36 +425,39 @@ nsresult nsDocumentEncoder::SerializeNod
 nsresult nsDocumentEncoder::SerializeToStringRecursive(nsINode* aNode,
                                                        nsAString& aStr,
                                                        bool aDontSerializeRoot,
                                                        uint32_t aMaxLength) {
   if (aMaxLength > 0 && aStr.Length() >= aMaxLength) {
     return NS_OK;
   }
 
-  if (!IsVisibleNode(aNode)) return NS_OK;
+  NS_ENSURE_TRUE(aNode, NS_ERROR_NULL_POINTER);
 
-  nsresult rv = NS_OK;
+  if (IsInvisibleNodeAndShouldBeSkipped(*aNode)) {
+    return NS_OK;
+  }
 
-  MOZ_ASSERT(aNode, "aNode shouldn't be nullptr.");
   FixupNodeDeterminer fixupNodeDeterminer{mNodeFixup, nullptr, *aNode};
   nsINode* maybeFixedNode =
       &fixupNodeDeterminer.GetFixupNodeFallBackToOriginalNode();
 
   if ((mFlags & SkipInvisibleContent) &&
       !(mFlags & OutputNonTextContentAsPlaceholder)) {
     if (aNode->IsContent()) {
       if (nsIFrame* frame = aNode->AsContent()->GetPrimaryFrame()) {
         if (!frame->IsSelectable(nullptr)) {
           aDontSerializeRoot = true;
         }
       }
     }
   }
 
+  nsresult rv = NS_OK;
+
   if (!aDontSerializeRoot) {
     int32_t endOffset = -1;
     if (aMaxLength > 0) {
       MOZ_ASSERT(aMaxLength >= aStr.Length());
       endOffset = aMaxLength - aStr.Length();
     }
     rv = SerializeNodeStart(*aNode, 0, endOffset, aStr, maybeFixedNode);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -580,17 +584,19 @@ static bool IsTextNode(nsINode* aNode) {
 
 nsresult nsDocumentEncoder::SerializeRangeNodes(nsRange* const aRange,
                                                 nsINode* const aNode,
                                                 nsAString& aString,
                                                 const int32_t aDepth) {
   nsCOMPtr<nsIContent> content = do_QueryInterface(aNode);
   NS_ENSURE_TRUE(content, NS_ERROR_FAILURE);
 
-  if (!IsVisibleNode(aNode)) return NS_OK;
+  if (IsInvisibleNodeAndShouldBeSkipped(*aNode)) {
+    return NS_OK;
+  }
 
   nsresult rv = NS_OK;
 
   // get start and end nodes for this recursion level
   nsCOMPtr<nsIContent> startNode, endNode;
   {
     int32_t start = mStartRootIndex - aDepth;
     if (start >= 0 && (uint32_t)start <= mStartNodes.Length())
@@ -783,21 +789,23 @@ nsresult nsDocumentEncoder::SerializeRan
   nsresult rv = NS_OK;
 
   rv = SerializeRangeContextStart(mCommonAncestors, aOutputString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (startContainer == endContainer && IsTextNode(startContainer)) {
     if (mFlags & SkipInvisibleContent) {
       // Check that the parent is visible if we don't a frame.
-      // IsVisibleNode() will do it when there's a frame.
+      // IsInvisibleNodeAndShouldBeSkipped() will do it when there's a frame.
       nsCOMPtr<nsIContent> content = do_QueryInterface(startContainer);
       if (content && !content->GetPrimaryFrame()) {
         nsIContent* parent = content->GetParent();
-        if (!parent || !IsVisibleNode(parent)) return NS_OK;
+        if (!parent || IsInvisibleNodeAndShouldBeSkipped(*parent)) {
+          return NS_OK;
+        }
       }
     }
     rv = SerializeNodeStart(*startContainer, startOffset, endOffset,
                             aOutputString);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = SerializeNodeEnd(*startContainer, aOutputString);
     NS_ENSURE_SUCCESS(rv, rv);
   } else {
@@ -842,19 +850,19 @@ nsDocumentEncoder::EncodeToStringWithMax
 
   if (!mDocument) return NS_ERROR_NOT_INITIALIZED;
 
   AutoReleaseDocumentIfNeeded autoReleaseDocument(this);
 
   aOutputString.Truncate();
 
   nsString output;
-  static const size_t bufferSize = 2048;
+  static const size_t kStringBufferSizeInBytes = 2048;
   if (!mCachedBuffer) {
-    mCachedBuffer = nsStringBuffer::Alloc(bufferSize).take();
+    mCachedBuffer = nsStringBuffer::Alloc(kStringBufferSizeInBytes).take();
     if (NS_WARN_IF(!mCachedBuffer)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
   NS_ASSERTION(
       !mCachedBuffer->IsReadonly(),
       "nsIDocumentEncoder shouldn't keep reference to non-readonly buffer!");
   static_cast<char16_t*>(mCachedBuffer->Data())[0] = char16_t(0);
@@ -970,17 +978,17 @@ nsDocumentEncoder::EncodeToStringWithMax
   rv = mSerializer->Flush(output);
 
   mCachedBuffer = nsStringBuffer::FromString(output);
   // We have to be careful how we set aOutputString, because we don't
   // want it to end up sharing mCachedBuffer if we plan to reuse it.
   bool setOutput = false;
   // Try to cache the buffer.
   if (mCachedBuffer) {
-    if (mCachedBuffer->StorageSize() == bufferSize &&
+    if ((mCachedBuffer->StorageSize() == kStringBufferSizeInBytes) &&
         !mCachedBuffer->IsReadonly()) {
       mCachedBuffer->AddRef();
     } else {
       if (NS_SUCCEEDED(rv)) {
         mCachedBuffer->ToString(output.Length(), aOutputString);
         setOutput = true;
       }
       mCachedBuffer = nullptr;
--- a/dom/base/nsINode.h
+++ b/dom/base/nsINode.h
@@ -461,17 +461,17 @@ class nsINode : public mozilla::dom::Eve
    * like event handler compilation.  Returning null means to use the
    * global object as the scope chain parent.
    */
   virtual nsINode* GetScopeChainParent() const;
 
   MOZ_CAN_RUN_SCRIPT mozilla::dom::Element* GetParentFlexElement();
 
   /**
-   * Return whether the node is an Element node
+   * Return whether the node is an Element node. Faster than using `NodeType()`.
    */
   bool IsElement() const { return GetBoolFlag(NodeIsElement); }
 
   /**
    * Return this node as an Element.  Should only be used for nodes
    * for which IsElement() is true.  This is defined inline in Element.h.
    */
   inline mozilla::dom::Element* AsElement();
--- a/layout/generic/nsFrameSelection.cpp
+++ b/layout/generic/nsFrameSelection.cpp
@@ -2742,33 +2742,34 @@ nsresult nsFrameSelection::UpdateSelecti
     Selection* aSel) {
   PresShell* presShell = aSel->GetPresShell();
   if (!presShell) {
     return NS_OK;
   }
   nsCOMPtr<Document> aDoc = presShell->GetDocument();
 
   if (aDoc && aSel && !aSel->IsCollapsed()) {
-    return nsCopySupport::HTMLCopy(aSel, aDoc, nsIClipboard::kSelectionCache,
-                                   false);
+    return nsCopySupport::EncodeDocumentWithContextAndPutToClipboard(
+        aSel, aDoc, nsIClipboard::kSelectionCache, false);
   }
 
   return NS_OK;
 }
 
 // mozilla::AutoCopyListener
 
 int16_t AutoCopyListener::sClipboardID = -1;
 
 /*
  * What we do now:
  * On every selection change, we copy to the clipboard anew, creating a
  * HTML buffer, a transferable, an nsISupportsString and
- * a huge mess every time.  This is basically what nsCopySupport::HTMLCopy()
- * does to move the selection into the clipboard for Edit->Copy.
+ * a huge mess every time.  This is basically what
+ * nsCopySupport::EncodeDocumentWithContextAndPutToClipboard() does to move the
+ * selection into the clipboard for Edit->Copy.
  *
  * What we should do, to make our end of the deal faster:
  * Create a singleton transferable with our own magic converter.  When selection
  * changes (use a quick cache to detect ``real'' changes), we put the new
  * Selection in the transferable.  Our magic converter will take care of
  * transferable->whatever-other-format when the time comes to actually
  * hand over the clipboard contents.
  *
@@ -2825,13 +2826,15 @@ void AutoCopyListener::OnSelectionChange
     // If on macOS, clear the current selection transferable cached
     // on the parent process (nsClipboard) when the selection is empty.
     DebugOnly<nsresult> rv = nsCopySupport::ClearSelectionCache();
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                          "nsCopySupport::ClearSelectionCache() failed");
     return;
   }
 
-  // Call the copy code.
   DebugOnly<nsresult> rv =
-      nsCopySupport::HTMLCopy(&aSelection, aDocument, sClipboardID, false);
-  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "nsCopySupport::HTMLCopy() failed");
+      nsCopySupport::EncodeDocumentWithContextAndPutToClipboard(
+          &aSelection, aDocument, sClipboardID, false);
+  NS_WARNING_ASSERTION(
+      NS_SUCCEEDED(rv),
+      "nsCopySupport::EncodeDocumentWithContextAndPutToClipboard() failed");
 }