Commit cfa39f78 authored by Emilio Cobos Álvarez's avatar Emilio Cobos Álvarez Committed by ealvarez@mozilla.com
Browse files

Bug 1967507 - Simplify custom content container set-up. r=smaug,devtools-reviewers,nchevobbe 

I haven't been able to reproduce this locally, but I see what's going
on, and I think we could do better.

This avoids the frame flush from nsCanvasFrame::CreateAnonymousContent,
and in general makes the anonymous content timing a lot more reasonable
(see the FlushPendingNotifications call that's getting removed).

The ServoStyleSet changes are needed now that there are multiple style
roots that can be unstyled like that.

It also simplifies the API as now it's not dependent on having a canvas
frame around (and thus doesn't need to flush layout).

Differential Revision: https://phabricator.services.mozilla.com/D250531
parent 0904ecb9

devtools/server/actors/highlighters/utils/markup.js

+2 −17
Changes for devtools/server/actors/highlighters/utils/markup.js: 2 added lines, 17 removed lines.
Original line number Diff line number Diff line
@@ -173,23 +173,15 @@ exports.isNodeValid = isNodeValid; 
 * @param {Boolean} options.waitForDocumentToLoad

 *        Set to false to try to insert the anonymous content even if the document

 *        isn't loaded yet. Defaults to true.

 * @param {Boolean} options.avoidForcedSynchronousLayoutUpdate

 *        If options.waitForDocumentToLoad is false, by default, we'll force a synchronous

 *        layout update to show the highlighter. Set to false to avoid such behaviour.

 */

function CanvasFrameAnonymousContentHelper(

  highlighterEnv,

  nodeBuilder,

  {

    waitForDocumentToLoad = true,

    avoidForcedSynchronousLayoutUpdate = false,

  } = {}

  { waitForDocumentToLoad = true } = {}

) {

  this.highlighterEnv = highlighterEnv;

  this.nodeBuilder = nodeBuilder;

  this.waitForDocumentToLoad = !!waitForDocumentToLoad;

  this.avoidForcedSynchronousLayoutUpdate =

    !!avoidForcedSynchronousLayoutUpdate;



  this._onWindowReady = this._onWindowReady.bind(this);

  this.highlighterEnv.on("window-ready", this._onWindowReady);
@@ -252,14 +244,7 @@ CanvasFrameAnonymousContentHelper.prototype = { 
    // that scenario, fixes when we're adding anonymous content in a tab that

    // is not the active one (see bug 1260043 and bug 1260044)

    try {

      // If we didn't wait for the document to load and avoidForcedSynchronousLayoutUpdate

      // isn't true, we want to force a layout update to ensure the anonymous content will

      // be rendered (see Bug 1580394).

      const forceSynchronousLayoutUpdate =

        !this.waitForDocumentToLoad && !this.avoidForcedSynchronousLayoutUpdate;

      this._content = this.anonymousContentDocument.insertAnonymousContent(

        forceSynchronousLayoutUpdate

      );

      this._content = this.anonymousContentDocument.insertAnonymousContent();

    } catch (e) {

      // If the `insertAnonymousContent` fails throwing a `NS_ERROR_UNEXPECTED`, it means

      // we don't have access to a `CustomContentContainer` yet (see bug 1365075).

devtools/server/actors/highlighters/viewport-size-on-resize.js

+0 −6
Changes for devtools/server/actors/highlighters/viewport-size-on-resize.js: 0 added lines, 6 removed lines.
Original line number Diff line number Diff line
@@ -23,12 +23,6 @@ class ViewportSizeOnResizeHighlighter extends ViewportSizeHighlighter { 
      prefix: "viewport-size-on-resize-highlighter-",

      hideTimeout: HIDE_TIMEOUT_MS,

      waitForDocumentToLoad: false,

      // We don't want to force a layout update as the highlighter will probably be hidden

      // to begin with and could cause animation to be triggered (as well as a warning

      // message to be printed in the console).

      // We don't care too much if we'd need the layout update anyway as the highlighter

      // is shown on resize, which do trigger a layout update.

      avoidForcedSynchronousLayoutUpdate: true,

    });

  }

}

devtools/server/actors/highlighters/viewport-size.js

+0 −4
Changes for devtools/server/actors/highlighters/viewport-size.js: 0 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -30,8 +30,6 @@ class ViewportSizeHighlighter { 
   *        Defaults to "viewport-size-highlighter-"

   * @param {Boolean} options.waitForDocumentToLoad: Option that will be passed to

   *        CanvasFrameAnonymousContentHelper. Defaults to true

   * @param {Boolean} options.avoidForcedSynchronousLayoutUpdate: Option that will be passed to

   *        CanvasFrameAnonymousContentHelper. Defaults to false

   */

  constructor(highlighterEnv, parent, options = {}) {

    this.env = highlighterEnv;
@@ -45,8 +43,6 @@ class ViewportSizeHighlighter { 
      this._buildMarkup.bind(this),

      {

        waitForDocumentToLoad: options?.waitForDocumentToLoad ?? true,

        avoidForcedSynchronousLayoutUpdate:

          options?.avoidForcedSynchronousLayoutUpdate ?? false,

      }

    );

    this._onPageResize = this._onPageResize.bind(this);

dom/base/Document.cpp

+93 −38
Changes for dom/base/Document.cpp: 93 added lines, 38 removed lines.
Original line number Diff line number Diff line
@@ -60,6 +60,7 @@ 
#include "mozilla/DocumentStyleRootIterator.h"

#include "mozilla/EditorBase.h"

#include "mozilla/EditorCommands.h"

#include "mozilla/dom/BindContext.h"

#include "mozilla/Encoding.h"

#include "mozilla/ErrorResult.h"

#include "mozilla/EventDispatcher.h"
@@ -2646,13 +2647,12 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(Document) 
  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFragmentDirective)

  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mHighlightRegistry)

  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingFullscreenEvents)









  // Traverse all Document nsCOMPtrs.









  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParser)









  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScriptGlobalObject)









  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mListenerManager)









  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mStyleSheetSetList)









  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mScriptLoader)









  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mCustomContentContainer)

















  DocumentOrShadowRoot::Traverse(tmp, cb);


















@@ -2766,6 +2766,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Document)















  nsAutoScriptBlocker scriptBlocker;

















  tmp->RemoveCustomContentContainer();

















  nsINode::Unlink(tmp);

















  while (tmp->HasChildren()) {










@@ -7701,13 +7703,17 @@ Element* Document::GetRootElementInternal() const {















void Document::InsertChildBefore(nsIContent* aKid, nsIContent* aBeforeThis,









                                 bool aNotify, ErrorResult& aRv) {









  if (aKid->IsElement() && GetRootElement()) {









  const bool isElementInsertion = aKid->IsElement();









  if (isElementInsertion && GetRootElement()) {









    NS_WARNING("Inserting root element when we already have one");









    aRv.ThrowHierarchyRequestError("There is already a root element.");









    return;









  }

















  nsINode::InsertChildBefore(aKid, aBeforeThis, aNotify, aRv);









  if (isElementInsertion && !aRv.Failed()) {









    CreateCustomContentContainerIfNeeded();









  }









}

















void Document::RemoveChildNode(nsIContent* aKid, bool aNotify,










@@ -8687,55 +8693,102 @@ void Document::RuleRemoved(StyleSheet& aSheet, css::Rule& aRule) {







  }









}

















static Element* GetCustomContentContainer(PresShell* aPresShell) {









  if (!aPresShell || !aPresShell->GetCanvasFrame()) {









    return nullptr;









static void UnbindAnonymousContent(AnonymousContent& aAnonContent) {









  nsCOMPtr<nsINode> parent = aAnonContent.Host()->GetParentNode();









  if (!parent) {









    return;









  }









  MOZ_ASSERT(parent->IsElement());









  MOZ_ASSERT(parent->AsElement()->IsRootOfNativeAnonymousSubtree());









  parent->RemoveChildNode(aAnonContent.Host(), true);









}

















  return aPresShell->GetCanvasFrame()->GetCustomContentContainer();









static void BindAnonymousContent(AnonymousContent& aAnonContent,









                                 Element& aContainer) {









  UnbindAnonymousContent(aAnonContent);









  aContainer.AppendChildTo(aAnonContent.Host(), true, IgnoreErrors());









}

















already_AddRefed<AnonymousContent> Document::InsertAnonymousContent(









    bool aForce, ErrorResult& aRv) {









  RefPtr<PresShell> shell = GetPresShell();









  if (aForce && !GetCustomContentContainer(shell)) {









    FlushPendingNotifications(FlushType::Layout);









    shell = GetPresShell();









void Document::RemoveCustomContentContainer() {









  RefPtr container = std::move(mCustomContentContainer);









  if (!container) {









    return;









  }









  nsAutoScriptBlocker scriptBlocker;









  if (DevToolsAnonymousAndShadowEventsEnabled()) {









    container->QueueDevtoolsAnonymousEvent(/* aIsRemove = */ true);









  }









  if (PresShell* ps = GetPresShell()) {









    ps->ContentWillBeRemoved(container, nullptr);









  }









  container->UnbindFromTree();









}

















void Document::CreateCustomContentContainerIfNeeded() {









  if (mAnonymousContents.IsEmpty()) {









    MOZ_ASSERT(!mCustomContentContainer);









    return;









  }









  if (mCustomContentContainer) {









    return;









  }









  RefPtr root = GetRootElement();









  if (NS_WARN_IF(!root)) {









    // We'll deal with it when we get a root element, if needed.









    return;









  }









  // Create the custom content container.









  RefPtr container = CreateHTMLElement(nsGkAtoms::div);









#ifdef DEBUG









  // We restyle our mCustomContentContainer, even though it's root anonymous









  // content.  Normally that's not OK because the frame constructor doesn't know









  // how to order the frame tree in such cases, but we make this work for this









  // particular case, so it's OK.









  container->SetProperty(nsGkAtoms::restylableAnonymousNode,









                         reinterpret_cast<void*>(true));









#endif  // DEBUG









  container->SetProperty(nsGkAtoms::docLevelNativeAnonymousContent,









                         reinterpret_cast<void*>(true));









  container->SetIsNativeAnonymousRoot();









  // Do not create an accessible object for the container.









  container->SetAttr(kNameSpaceID_None, nsGkAtoms::role, u"presentation"_ns,









                     false);









  container->SetAttr(kNameSpaceID_None, nsGkAtoms::_class,









                     u"moz-custom-content-container"_ns, false);









  nsAutoScriptBlocker scriptBlocker;









  BindContext context(*root, BindContext::ForNativeAnonymous);









  if (NS_WARN_IF(NS_FAILED(container->BindToTree(context, *root)))) {









    container->UnbindFromTree();









    return;









  }









  mCustomContentContainer = container;









  if (DevToolsAnonymousAndShadowEventsEnabled()) {









    container->QueueDevtoolsAnonymousEvent(/* aIsRemove = */ false);









  }









  if (PresShell* ps = GetPresShell()) {









    ps->ContentAppended(container);









  }









  for (auto& anonContent : mAnonymousContents) {









    BindAnonymousContent(*anonContent, *container);









  }









}

















already_AddRefed<AnonymousContent> Document::InsertAnonymousContent(









    ErrorResult& aRv) {









  RefPtr<AnonymousContent> anonContent = AnonymousContent::Create(*this);









  if (!anonContent) {









    aRv.Throw(NS_ERROR_OUT_OF_MEMORY);









    return nullptr;









  }

















  mAnonymousContents.AppendElement(anonContent);

















  if (RefPtr<Element> container = GetCustomContentContainer(shell)) {









    // If the container is empty and we have other anon content we should be









    // about to show all the other anonymous content nodes.









    if (container->HasChildren() || mAnonymousContents.Length() == 1) {









      container->AppendChildTo(anonContent->Host(), true, IgnoreErrors());









      if (auto* canvasFrame = shell->GetCanvasFrame()) {









        canvasFrame->ShowCustomContentContainer();









      }









    }









  if (RefPtr container = mCustomContentContainer) {









    BindAnonymousContent(*anonContent, *container);









  } else {









    CreateCustomContentContainerIfNeeded();









  }

















  return anonContent.forget();









}

















static void RemoveAnonContentFromCanvas(AnonymousContent& aAnonContent,









                                        PresShell* aPresShell) {









  RefPtr<Element> container = GetCustomContentContainer(aPresShell);









  if (!container) {









    return;









  }









  container->RemoveChild(*aAnonContent.Host(), IgnoreErrors());









}

















void Document::RemoveAnonymousContent(AnonymousContent& aContent) {









  nsAutoScriptBlocker scriptBlocker;


















@@ -8745,11 +8798,10 @@ void Document::RemoveAnonymousContent(AnonymousContent& aContent) {







  }

















  mAnonymousContents.RemoveElementAt(index);









  RemoveAnonContentFromCanvas(aContent, GetPresShell());









  UnbindAnonymousContent(aContent);

















  if (mAnonymousContents.IsEmpty() &&









      GetCustomContentContainer(GetPresShell())) {









    GetPresShell()->GetCanvasFrame()->HideCustomContentContainer();









  if (mAnonymousContents.IsEmpty()) {









    RemoveCustomContentContainer();









  }









}


















@@ -12001,6 +12053,8 @@ void Document::Destroy() {







    transition->SkipTransition(SkipTransitionReason::DocumentHidden);









  }

















  RemoveCustomContentContainer();

















  ReportDocumentUseCounters();









  ReportShadowedHTMLDocumentProperties();









  ReportLCP();










@@ -12531,6 +12585,7 @@ void Document::WillRemoveRoot() {







    transition->SkipTransition(SkipTransitionReason::RootRemoved);









  }

















  RemoveCustomContentContainer();









  IncrementExpandoGeneration(*this);









}










































dom/base/Document.h



+11
−2



























Changes for dom/base/Document.h: 11 added lines, 2 removed lines.



Original line number
Diff line number
Diff line







@@ -1307,13 +1307,18 @@ class Document : public nsINode,







   */









  nsresult GetSrcdocData(nsAString& aSrcdocData);



















  already_AddRefed<AnonymousContent> InsertAnonymousContent(bool aForce,









                                                            ErrorResult&);









  already_AddRefed<AnonymousContent> InsertAnonymousContent(ErrorResult&);









  void RemoveAnonymousContent(AnonymousContent&);









  nsTArray<RefPtr<AnonymousContent>>& GetAnonymousContents() {









    return mAnonymousContents;









  }









  Element* GetCustomContentContainer() const { return mCustomContentContainer; }



















 private:









  void CreateCustomContentContainerIfNeeded();









  void RemoveCustomContentContainer();



















 public:









  TimeStamp GetPageUnloadingEventTimeStamp() const {









    if (!mParentDocument) {









      return mPageUnloadingEventTimeStamp;










@@ -5261,6 +5266,10 @@ class Document : public nsINode,

















  nsTArray<RefPtr<AnonymousContent>> mAnonymousContents;



















  // The <div class="moz-custom-content-container"> that we use to wrap all the









  // mAnonymousContents roots. It's a NAC root, child of the root element.









  RefPtr<Element> mCustomContentContainer;



















  uint32_t mBlockDOMContentLoaded;



















  // Our live MediaQueryLists