New issue
Advanced search Search tips

Issue 623005 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

isEditablePosition() should not update layout tree

Project Member Reported by yosin@chromium.org, Jun 24 2016

Issue description

isEditablePosition() is called in deep of stack and it is bad that predicated has side effect.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20c47d5dabdf4e4170d53404bb93f4ca39677a46

commit 20c47d5dabdf4e4170d53404bb93f4ca39677a46
Author: yosin <yosin@chromium.org>
Date: Mon Jun 27 02:53:15 2016

Make Editor::replaceSelectionWithFragment() to update layout tree explicitly

This patch makes |Editor::replaceSelectionWithFragment()| to update layout
tree explicitly for preparation of crrev.com/2089993003, Get rid of
|EUpdateStyle| parameter from |isEditablePosition()|.

BUG= 623005 
TEST=n/a; no behavior changes

Review-Url: https://codereview.chromium.org/2099643002
Cr-Commit-Position: refs/heads/master@{#402099}

[modify] https://crrev.com/20c47d5dabdf4e4170d53404bb93f4ca39677a46/third_party/WebKit/Source/core/editing/Editor.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0aad49f542b1119bf3ef413ca9c1c08d2aecb699

commit 0aad49f542b1119bf3ef413ca9c1c08d2aecb699
Author: yosin <yosin@chromium.org>
Date: Mon Jun 27 02:54:15 2016

Make LocalFrame::nodeImage() to take an image for the element with :-webkit-drag pseudo class

This patch makes |LocalFrame::nodeImage()| to take image for element with
":-webkit-drag" pseudo class by re-factoring |DragImageBuilder|.

Before this patch, |nodeImage()| doesn't take ":-webkit-drag" and paints
dragged element with dirty layout tree, e.g. |LayoutObject::updateDragState()|
sets "Needs Style Recalc" for ":-webkit-drag". This causes layout tree update
in |BlockPainter::paint()| due by |LayoutObject::hasCursor()| which calls
|Document::updateStyleAndLayoutTreeIgnorePendingStylesheets()| via
|isEditingPosition()|.

This patch is a preparation of hoisting update layout for |isEditablePosition()|
, http://crrev.com/2089993003, which is called during layout tree building for
checking whether |LayoutBlock|
have caret or not.

BUG= 623005 
TEST=run-webkit-tests --gtest_filter=LocalFrameTest.nodeImage*

Review-Url: https://codereview.chromium.org/2094653002
Cr-Commit-Position: refs/heads/master@{#402100}

[modify] https://crrev.com/0aad49f542b1119bf3ef413ca9c1c08d2aecb699/third_party/WebKit/Source/core/core.gypi
[modify] https://crrev.com/0aad49f542b1119bf3ef413ca9c1c08d2aecb699/third_party/WebKit/Source/core/frame/LocalFrame.cpp
[add] https://crrev.com/0aad49f542b1119bf3ef413ca9c1c08d2aecb699/third_party/WebKit/Source/core/frame/LocalFrameTest.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00e7954de686c4f376c5eae0cdf367f842f263b0

commit 00e7954de686c4f376c5eae0cdf367f842f263b0
Author: yosin <yosin@chromium.org>
Date: Mon Jun 27 05:02:10 2016

Make SpellChecker::respondToChangedSelection() to update layout tree explicitly

This patch makes |SpellChecker::respondToChangedSelection()| to update layout
tree explicitly for preparation of getting rid of grammar checking related code.

A test expectation of fast/repaint/inline-outline-repaint.html is updated, since
this patch reduced number of update test tree in |respondToChangedSelection()|
by calling |inShadowIncludingDocument()| before |isContentEditable()|, which
updates layout tree.

This patch is also a preparation of crrev.com/2089993003, Get rid of
|EUpdateStyle| parameter from |isEditablePosition()|.

BUG= 619452 ,  623005 
TEST=n/a; no user visible changes

Review-Url: https://codereview.chromium.org/2092063002
Cr-Commit-Position: refs/heads/master@{#402115}

[modify] https://crrev.com/00e7954de686c4f376c5eae0cdf367f842f263b0/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/00e7954de686c4f376c5eae0cdf367f842f263b0/third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/019e4a359a1f7db417cf1132e5fd982cfacec21c

commit 019e4a359a1f7db417cf1132e5fd982cfacec21c
Author: yosin <yosin@chromium.org>
Date: Tue Jun 28 02:14:42 2016

Get rid of unused parameter EUpdateStyle parameter from isEditablePosition()

This patch gets rid of unused parameter |EUpdateStyle| from
|isEditablePosition()| to reduce code size for improving code health.

This patch also introduces |needsLayoutTreeUpdate()| for checking
|isEditablePosition()| called with clean layout tree.

BUG= 623005 
TEST=n/a; no behavior changes

Review-Url: https://codereview.chromium.org/2089993003
Cr-Commit-Position: refs/heads/master@{#402379}

[modify] https://crrev.com/019e4a359a1f7db417cf1132e5fd982cfacec21c/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/019e4a359a1f7db417cf1132e5fd982cfacec21c/third_party/WebKit/Source/core/editing/EditingUtilities.h
[modify] https://crrev.com/019e4a359a1f7db417cf1132e5fd982cfacec21c/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
[modify] https://crrev.com/019e4a359a1f7db417cf1132e5fd982cfacec21c/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/019e4a359a1f7db417cf1132e5fd982cfacec21c/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp
[modify] https://crrev.com/019e4a359a1f7db417cf1132e5fd982cfacec21c/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp

Comment 5 by yosin@chromium.org, Jun 28 2016

Status: Fixed (was: Started)

Sign in to add a comment