Rename AttachBehavior to RecalcStyleBehavior

Rename AttachBehavior to RecalcStyleBehavior

This patch rename the enum and makes editing trigger a recalcStyle using the new enum value
DeprecatedRecalcStyleImmediatlelyForEditing. This will be slower, but editing causes so many
recalc styles and does so much work already I don't think this is going to be an issue.

This means we now lazy attach everywhere.

BUG=276689
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157355

[ojan, 2013-09-04 18:05:12]

I'm not sure this is the right direction to take things. What do you
envision the end result, post all cleanup to be? I was picturing that the
editing code itself would call updateStyleIfNeeded in all the places where
it calls insertData/deleteData and that we'd get rid of the AttachBehavior
enum/argument entirely. Were you picturing something different?


On Wed, Sep 4, 2013 at 12:25 AM, <esprehn@chromium.org> wrote:

> Reviewers: eseidel, ojan, pdr,
>
> Description:
> Split DeprecatedAttachNow into DeprecatedEditingAttach and
> DeprecatedParserAttach
>
> This patch splits the enum and makes editing trigger a recalcStyle when it
> wants
> to
> "attach now". This will be slower, but editing causes so many recalc
> styles and
> does
> so much work already I don't think this is going to be an issue.
>
> Once the html parser is switched over to lazyAttach this means we'll lazy
> attach
> everywhere.
>
> BUG=276689
>
> Please review this at
https://codereview.chromium.**org/23874007/<https://codereview.chromium.org/2...
>
> SVN Base:
https://chromium.googlesource.**com/chromium/blink.git@master<https://chromiu...
>
> Affected files:
>   M Source/core/dom/CharacterData.**cpp
>   M Source/core/dom/ContainerNode.**cpp
>   M Source/core/dom/Node.h
>   M Source/core/dom/Text.cpp
>   M Source/core/editing/**DeleteFromTextNodeCommand.cpp
>   M Source/core/editing/**InsertIntoTextNodeCommand.cpp
>   M Source/core/editing/**SplitTextNodeCommand.cpp
>   M Source/core/html/parser/**HTMLConstructionSite.cpp
>
>
> Index: Source/core/dom/CharacterData.**cpp
> diff --git a/Source/core/dom/**CharacterData.cpp b/Source/core/dom/**
> CharacterData.cpp
> index 6fd51f4c8a49172fb63a53d129daf1**d41b8519ba..**
> 7dad2e8e53f1ef1e9193ec1e4fdf3b**0a82ccaada 100644
> --- a/Source/core/dom/**CharacterData.cpp
> +++ b/Source/core/dom/**CharacterData.cpp
> @@ -98,7 +98,7 @@ unsigned CharacterData::**parserAppendData(const
> String& string, unsigned offset,
>
>      ASSERT(!renderer() || isTextNode());
>      if (isTextNode())
> -        toText(this)->**updateTextRenderer(oldLength, 0,
> DeprecatedAttachNow);
> +        toText(this)->**updateTextRenderer(oldLength, 0,
> DeprecatedParserAttachNow);
>
>      document().incDOMTreeVersion()**;
>
> Index: Source/core/dom/ContainerNode.**cpp
> diff --git a/Source/core/dom/**ContainerNode.cpp b/Source/core/dom/**
> ContainerNode.cpp
> index 1ab2dc7a8b44eb64f2662334963825**3ded761cb0..**
> 0ff5c8fc559c61145c02080fbfd427**6481b0f5c7 100644
> --- a/Source/core/dom/**ContainerNode.cpp
> +++ b/Source/core/dom/**ContainerNode.cpp
> @@ -65,6 +65,8 @@ unsigned NoEventDispatchAssertion::s_**count = 0;
>
>  static inline void attachAfterInsertion(Node* node, AttachBehavior
> attachBehavior = AttachLazily)
>  {
> +    ASSERT(attachBehavior == AttachLazily || attachBehavior ==
> DeprecatedParserAttachNow);
> +
>      if (node->attached() || !node->parentNode() ||
> !node->parentNode()->attached(**))
>          return;
>
> @@ -121,7 +123,7 @@ void
ContainerNode::**takeAllChildrenFrom(**ContainerNode*
> oldParent)
>          // (if the document changed or oldParent was in a shadow tree,
> AND *this is in a shadow tree).
>          // Can we do better?
>          treeScope().adoptIfNeeded(**child.get());
> -        parserAppendChild(child.get(), DeprecatedAttachNow);
> +        parserAppendChild(child.get())**;
>      }
>  }
>
> Index: Source/core/dom/Node.h
> diff --git a/Source/core/dom/Node.h b/Source/core/dom/Node.h
> index 5766efdf7cf2985d8b1fa33a2b40b1**2b830cb005..**
> e0093742f31d2fbfcaefd3aac1c52e**7f6e381c80 100644
> --- a/Source/core/dom/Node.h
> +++ b/Source/core/dom/Node.h
> @@ -101,7 +101,8 @@ enum StyleChangeSource {
>  };
>
>  enum AttachBehavior {
> -    DeprecatedAttachNow,
> +    DeprecatedParserAttachNow,
> +    DeprecatedEditingAttachNow,
>      AttachLazily,
>  };
>
> Index: Source/core/dom/Text.cpp
> diff --git a/Source/core/dom/Text.cpp b/Source/core/dom/Text.cpp
> index 7a39eda580c161b4ca53126f8d575f**2e416eb46d..**
> 1250d23db944d92ca47ca78ac41b3f**21b0504a46 100644
> --- a/Source/core/dom/Text.cpp
> +++ b/Source/core/dom/Text.cpp
> @@ -298,10 +298,13 @@ void Text::updateTextRenderer(**unsigned
> offsetOfReplacedData, unsigned lengthOfRe
>          return;
>      RenderText* textRenderer = toRenderText(renderer());
>      if (!textRenderer || !textRendererIsNeeded(**NodeRenderingContext(this,
> textRenderer->style()))) {
> -        if (attachBehavior == DeprecatedAttachNow)
> +        if (attachBehavior == DeprecatedParserAttachNow) {
>              reattach();
> -        else
> -            lazyReattach();
> +            return;
> +        }
> +        lazyReattach();
> +        if (attachBehavior == DeprecatedEditingAttachNow)
> +            document().**updateStyleIfNeeded();
>          return;
>      }
>      textRenderer->**setTextWithOffset(dataImpl(), offsetOfReplacedData,
> lengthOfReplacedData);
> Index: Source/core/editing/**DeleteFromTextNodeCommand.cpp
> diff --git a/Source/core/editing/**DeleteFromTextNodeCommand.cpp
> b/Source/core/editing/**DeleteFromTextNodeCommand.cpp
> index 03fb87e02447b10a04da166407bbfe**edebccb5b8..**
> 6ae1ae2390a4c93ffa81dd1d1a3d54**0bfc18824f 100644
> --- a/Source/core/editing/**DeleteFromTextNodeCommand.cpp
> +++ b/Source/core/editing/**DeleteFromTextNodeCommand.cpp
> @@ -56,7 +56,7 @@ void DeleteFromTextNodeCommand::**doApply()
>      if (es.hadException())
>          return;
>
> -    m_node->deleteData(m_offset, m_count, es, DeprecatedAttachNow);
> +    m_node->deleteData(m_offset, m_count, es, DeprecatedEditingAttachNow);
>  }
>
>  void DeleteFromTextNodeCommand::**doUnapply()
> @@ -66,7 +66,7 @@ void DeleteFromTextNodeCommand::**doUnapply()
>      if (!m_node->rendererIsEditable()**)
>          return;
>
> -    m_node->insertData(m_offset, m_text, IGNORE_EXCEPTION,
> DeprecatedAttachNow);
> +    m_node->insertData(m_offset, m_text, IGNORE_EXCEPTION,
> DeprecatedEditingAttachNow);
>  }
>
>  #ifndef NDEBUG
> Index: Source/core/editing/**InsertIntoTextNodeCommand.cpp
> diff --git a/Source/core/editing/**InsertIntoTextNodeCommand.cpp
> b/Source/core/editing/**InsertIntoTextNodeCommand.cpp
> index d2ddc99502aa1a41278cf050f5aff1**49f0c4fb5e..**
> 33ffd9869c60f898cfa12d60125003**4fd66b63dc 100644
> --- a/Source/core/editing/**InsertIntoTextNodeCommand.cpp
> +++ b/Source/core/editing/**InsertIntoTextNodeCommand.cpp
> @@ -60,7 +60,7 @@ void InsertIntoTextNodeCommand::**doApply()
>              renderText->**momentarilyRevealLastTypedChar**acter(m_offset
> + m_text.length() - 1);
>      }
>
> -    m_node->insertData(m_offset, m_text, IGNORE_EXCEPTION,
> DeprecatedAttachNow);
> +    m_node->insertData(m_offset, m_text, IGNORE_EXCEPTION,
> DeprecatedEditingAttachNow);
>  }
>
>  void InsertIntoTextNodeCommand::**doUnapply()
> @@ -68,7 +68,7 @@ void InsertIntoTextNodeCommand::**doUnapply()
>      if (!m_node->rendererIsEditable()**)
>          return;
>
> -    m_node->deleteData(m_offset, m_text.length(), IGNORE_EXCEPTION,
> DeprecatedAttachNow);
> +    m_node->deleteData(m_offset, m_text.length(), IGNORE_EXCEPTION,
> DeprecatedEditingAttachNow);
>  }
>
>  #ifndef NDEBUG
> Index: Source/core/editing/**SplitTextNodeCommand.cpp
> diff --git a/Source/core/editing/**SplitTextNodeCommand.cpp
> b/Source/core/editing/**SplitTextNodeCommand.cpp
> index 81f38af7d869d0b40fd3e256c47a32**d03b8a3d72..**
> d977a720a941baef503f8c703f178f**f56ec85409 100644
> --- a/Source/core/editing/**SplitTextNodeCommand.cpp
> +++ b/Source/core/editing/**SplitTextNodeCommand.cpp
> @@ -76,7 +76,7 @@ void SplitTextNodeCommand::**doUnapply()
>
>      String prefixText = m_text1->data();
>
> -    m_text2->insertData(0, prefixText, ASSERT_NO_EXCEPTION,
> DeprecatedAttachNow);
> +    m_text2->insertData(0, prefixText, ASSERT_NO_EXCEPTION,
> DeprecatedEditingAttachNow);
>
>      document().markers()->**copyMarkers(m_text1.get(), 0,
> prefixText.length(), m_text2.get(), 0);
>      m_text1->remove(ASSERT_NO_**EXCEPTION);
> @@ -100,7 +100,7 @@ void SplitTextNodeCommand::**insertText1AndTrimText2()
>      m_text2->parentNode()->**insertBefore(m_text1.get(), m_text2.get(),
> es);
>      if (es.hadException())
>          return;
> -    m_text2->deleteData(0, m_offset, es, DeprecatedAttachNow);
> +    m_text2->deleteData(0, m_offset, es, DeprecatedEditingAttachNow);
>  }
>
>  #ifndef NDEBUG
> Index: Source/core/html/parser/**HTMLConstructionSite.cpp
> diff --git a/Source/core/html/parser/**HTMLConstructionSite.cpp
> b/Source/core/html/parser/**HTMLConstructionSite.cpp
> index fc0e957f39f2071c2c293e7163669f**342930eed9..**
> 0f895e3284d52ababc5d553528918f**aac952a047 100644
> --- a/Source/core/html/parser/**HTMLConstructionSite.cpp
> +++ b/Source/core/html/parser/**HTMLConstructionSite.cpp
> @@ -105,7 +105,7 @@ static inline void
executeInsertTask(**HTMLConstructionSiteTask&
> task)
>  {
>      ASSERT(task.operation == HTMLConstructionSiteTask::**Insert);
>
> -    insert(task, DeprecatedAttachNow);
> +    insert(task, DeprecatedParserAttachNow);
>
>      task.child->**beginParsingChildren();
>
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[ojan, 2013-09-04 21:07:23]

As discussed in person, lets go ahead with a cleaned up version of this once my
other lazy attach patches land.

[esprehn, 2013-09-06 00:25:57]

On 2013/09/04 21:07:23, ojan wrote:
> As discussed in person, lets go ahead with a cleaned up version of this once
my
> other lazy attach patches land.

Okay this is ready for review again.

[eseidel, 2013-09-06 04:08:37]

lgtm

[commit-bot: I haz the power, 2013-09-06 04:08:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/esprehn@chromium.org/23874007/16001

[commit-bot: I haz the power, 2013-09-06 04:28:45]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_blink_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-06 06:14:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/esprehn@chromium.org/23874007/28001

[commit-bot: I haz the power, 2013-09-06 07:17:51]

Change committed as 157355