blink::canonicalPosition() may have side-effect breaking callers document lifecycle state if there is pending style sheets |
||||||||||||||
Issue descriptionFrom crbug.com/601001 #28 by skyostil@chromium.org: So I was able to reproduce this on Android using ClusterFuzz but not on desktop so far. It looks like this happens even if I turn off frame throttling, so something else is going on here. Looks like something is invalidating the document during synchronous paint, which causes Document::updateLayoutTree() (called by PendingSelection::commit) to blow up. I added this assert to find out who: inline void Document::scheduleLayoutTreeUpdateIfNeeded() { + RELEASE_ASSERT(lifecycle().state() != 17); And the resulting backtrace is shown below. Turns out there are nodes with a placeholder style in the document. Maybe someone who knows more about pending style sheets could take a peek? 4-13 19:55:31.773 500 500 F DEBUG : #00 pc 00a2eea0 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::Node::markAncestorsWithChildNeedsStyleRecalc()+519) 04-13 19:55:31.773 500 500 F DEBUG : #01 pc 00a2f14f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::Node::setNeedsStyleRecalc(blink::StyleChangeType, blink::StyleChangeReasonForTracing const&)+646) 04-13 19:55:31.773 500 500 F DEBUG : #02 pc 00965333 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::Document::updateLayoutTreeIgnorePendingStylesheets()+442) 04-13 19:55:31.773 500 500 F DEBUG : #03 pc 00964a9f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::Document::updateLayoutIgnorePendingStylesheets(blink::Document::RunPostLayoutTasks)+8) 04-13 19:55:31.773 500 500 F DEBUG : #04 pc 011ba4b9 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::canonicalPositionOf(blink::PositionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > const&)+468) 04-13 19:55:31.774 500 500 F DEBUG : #05 pc 011ad72b /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::VisiblePositionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> >::create(blink::PositionWithAffinityTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > const&)+146) 04-13 19:55:31.774 500 500 F DEBUG : #06 pc 011addf1 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::createVisiblePosition(blink::PositionTemplate<blink::EditingAlgorithm<blink::FlatTreeTraversal> > const&, blink::TextAffinity)+144) 04-13 19:55:31.774 500 500 F DEBUG : #07 pc 01193831 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.774 500 500 F DEBUG : #08 pc 01193c3f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.774 500 500 F DEBUG : #09 pc 01975379 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutView::commitPendingSelection()+308) 04-13 19:55:31.774 500 500 F DEBUG : #10 pc 0197cb45 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutView::selectionStartEnd(int&, int&)+12) 04-13 19:55:31.774 500 500 F DEBUG : #11 pc 019eed23 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.774 500 500 F DEBUG : #12 pc 01624327 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.774 500 500 F DEBUG : #13 pc 019f24ff /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #14 pc 0161fc55 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #15 pc 019eab8b /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #16 pc 0168b22b /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #17 pc 01a0df17 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #18 pc 016305db /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #19 pc 015f0e0d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #20 pc 015efe65 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.775 500 500 F DEBUG : #21 pc 017bb935 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paintObject(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.776 500 500 F DEBUG : #22 pc 015edc8f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.776 500 500 F DEBUG : #23 pc 017bb74d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paint(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.776 500 500 F DEBUG : #24 pc 015ef035 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.776 500 500 F DEBUG : #25 pc 015eed67 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.776 500 500 F DEBUG : #26 pc 017bb841 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paintChildren(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.776 500 500 F DEBUG : #27 pc 015f0e57 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.776 500 500 F DEBUG : #28 pc 015efe65 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.776 500 500 F DEBUG : #29 pc 017bb935 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paintObject(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.776 500 500 F DEBUG : #30 pc 015edc8f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.777 500 500 F DEBUG : #31 pc 017bb74d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paint(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.777 500 500 F DEBUG : #32 pc 015ef035 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.777 500 500 F DEBUG : #33 pc 015eed67 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.777 500 500 F DEBUG : #34 pc 017bb841 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paintChildren(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.777 500 500 F DEBUG : #35 pc 015f0e57 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.777 500 500 F DEBUG : #36 pc 015efe65 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.777 500 500 F DEBUG : #37 pc 017bb935 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paintObject(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.777 500 500 F DEBUG : #38 pc 015edc8f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so 04-13 19:55:31.777 500 500 F DEBUG : #39 pc 017bb74d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::LayoutBlock::paint(blink::PaintInfo const&, blink::LayoutPoint const&) const+136) 04-13 19:55:31.777 500 500 F DEBUG : #40 pc 0166b369 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintFragmentWithPhase(blink::PaintPhase, blink::PaintLayerFragment const&, blink::GraphicsContext&, blink::ClipRect const&, blink::PaintLayerPaintingInfo const&, unsigned int, blink::PaintLayerPainter::ClipState)+1584) 04-13 19:55:31.778 500 500 F DEBUG : #41 pc 0166b9cf /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintForegroundForFragmentsWithPhase(blink::PaintPhase, WTF::Vector<blink::PaintLayerFragment, 1u, WTF::PartitionAllocator> const&, blink::GraphicsContext&, blink::PaintLayerPaintingInfo const&, unsigned int, blink::PaintLayerPainter::ClipState)+302) 04-13 19:55:31.778 500 500 F DEBUG : #42 pc 01669425 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintForegroundForFragments(WTF::Vector<blink::PaintLayerFragment, 1u, WTF::PartitionAllocator> const&, blink::GraphicsContext&, blink::LayoutRect const&, blink::PaintLayerPaintingInfo const&, bool, unsigned int)+704) 04-13 19:55:31.778 500 500 F DEBUG : #43 pc 01666a37 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintLayerContents(blink::GraphicsContext&, blink::PaintLayerPaintingInfo const&, unsigned int, blink::PaintLayerPainter::FragmentPolicy)+5402) 04-13 19:55:31.778 500 500 F DEBUG : #44 pc 0166543b /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintLayerContentsAndReflection(blink::GraphicsContext&, blink::PaintLayerPaintingInfo const&, unsigned int, blink::PaintLayerPainter::FragmentPolicy)+350) 04-13 19:55:31.778 500 500 F DEBUG : #45 pc 01663b97 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintLayer(blink::GraphicsContext&, blink::PaintLayerPaintingInfo const&, unsigned int)+1126) 04-13 19:55:31.778 500 500 F DEBUG : #46 pc 01668efd /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintChildren(unsigned int, blink::GraphicsContext&, blink::PaintLayerPaintingInfo const&, unsigned int)+1012) 04-13 19:55:31.779 500 500 F DEBUG : #47 pc 01666a5f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PaintLayerPainter::paintLayerContents(blink::GraphicsContext&, blink::PaintLayerPaintingInfo const&, unsigned int, blink::PaintLayerPainter::FragmentPolicy)+5442) 04-13 19:55:31.779 500 500 F DEBUG : #48 pc 019bb7cb /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::CompositedLayerMapping::doPaintTask(blink::GraphicsLayerPaintInfo const&, blink::GraphicsLayer const&, unsigned int const&, blink::GraphicsContext&, blink::IntRect const&) const+1954) 04-13 19:55:31.779 500 500 F DEBUG : #49 pc 019bdfbb /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::CompositedLayerMapping::paintContents(blink::GraphicsLayer const*, blink::GraphicsContext&, unsigned int, blink::IntRect const&) const+1586) 04-13 19:55:31.779 500 500 F DEBUG : #50 pc 00228361 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_platform.cr.so (blink::GraphicsLayer::paintWithoutCommit(blink::IntRect const*, blink::GraphicsContext::DisabledMode)+700) 04-13 19:55:31.779 500 500 F DEBUG : #51 pc 0022801f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_platform.cr.so (blink::GraphicsLayer::paint(blink::IntRect const*, blink::GraphicsContext::DisabledMode)+134) 04-13 19:55:31.779 500 500 F DEBUG : #52 pc 0134636b /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::synchronizedPaintRecursively(blink::GraphicsLayer*)+38) 04-13 19:55:31.779 500 500 F DEBUG : #53 pc 0134640d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::synchronizedPaintRecursively(blink::GraphicsLayer*)+200) 04-13 19:55:31.779 500 500 F DEBUG : #54 pc 0134640d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::synchronizedPaintRecursively(blink::GraphicsLayer*)+200) 04-13 19:55:31.780 500 500 F DEBUG : #55 pc 0134640d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::synchronizedPaintRecursively(blink::GraphicsLayer*)+200) 04-13 19:55:31.780 500 500 F DEBUG : #56 pc 0134640d /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::synchronizedPaintRecursively(blink::GraphicsLayer*)+200) 04-13 19:55:31.780 500 500 F DEBUG : #57 pc 013453e9 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::synchronizedPaint()+496) 04-13 19:55:31.780 500 500 F DEBUG : #58 pc 013442d9 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::FrameView::updateLifecyclePhasesInternal(blink::FrameView::LifeCycleUpdateOption)+1412) 04-13 19:55:31.780 500 500 F DEBUG : #59 pc 015b5c39 /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_core.cr.so (blink::PageAnimator::updateAllLifecyclePhases(blink::LocalFrame&)+38) 04-13 19:55:31.780 500 500 F DEBUG : #60 pc 001b201f /data/app/com.google.android.apps.chrome-1/lib/arm/libblink_web.cr.so (blink::WebViewImpl::updateAllLifecyclePhases()+466) 04-13 19:55:31.780 500 500 F DEBUG : #61 pc 018a6f0f /data/app/com.google.android.apps.chrome-1/lib/arm/libcontent.cr.so (content::RenderWidgetCompositor::UpdateLayerTreeHost()+166) 04-13 19:55:31.780 500 500 F DEBUG : #62 pc 003d2a19 /data/app/com.google.android.apps.chrome-1/lib/arm/libcc.cr.so (cc::ProxyMain::BeginMainFrame(std::__1::unique_ptr<cc::BeginMainFrameAndCommitState, std::__1::default_delete<cc::BeginMainFrameAndCommitState> >)+1140) 04-13 19:55:31.780 500 500 F DEBUG : #63 pc 003f5695 /data/app/com.google.android.apps.chrome-1/lib/arm/libcc.cr.so
,
Apr 14 2016
Yosin@ I still assign this to you because the call causing unexpected document lifecycle update is from selection code which is supposed not to have such side-effects.
,
May 11 2016
PendingSeleciton::commit() should not cause layout, because it has assertion
DCHECK(!layoutView.needsLayout());
It seems layoutView.needsLayout() != document.needsLayoutTreeUpdate().
I'll add DCHECK(!document.needsLayoutTreeUpdate()).
,
Jul 18 2016
Issue 628575 has been merged into this issue.
,
Jul 18 2016
Issue 627201 has been merged into this issue.
,
Jul 19 2016
Users experienced this crash on the following builds: Win Canary 54.0.2799.0 - 0.47 CPM, 19 reports, 17 clients (signature blink::FrameView::setNeedsLayout) Mac Canary 54.0.2799.0 - 0.61 CPM, 3 reports, 3 clients (signature blink::FrameView::setNeedsLayout) If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates. - Go/Fracas
,
Aug 10 2016
,
Sep 5 2016
Users experienced this crash on the following builds: Mac Canary 55.0.2849.0 - 1.76 CPM, 2 reports, 2 clients (signature blink::FrameView::setNeedsLayout) If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates. - Go/Fracas
,
Sep 10 2016
Bug 621360 is another case that blink::canonicalPosition() causes reentrance of document lifecycle update. Now the reentrance is guarded by a DCHECK or early return.
,
Sep 14 2016
Issue 537821 has been merged into this issue.
,
Sep 19 2016
Link which helps in further triaging. https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3AFrameView%3A%3AsetNeedsLayout%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D wangxianzhu@ thanks for investigating. Will it be possible to have a fix before M54 hits stable? ( First week of OCT). Tagging with a proper milestone.
,
Sep 30 2016
Could you change PendingSelection::commit() to use layout-free version of VisiblePosition ctor and VisibleSelection ctor to fix this issue?
,
Sep 30 2016
,
Sep 30 2016
Once we change PendingSelection::commit() not to cause layout, we can say this bug is fixed.
It makes selection code works with dirty layout tree even if selection code requires clean layout tree.
AFAIK, there are four cases causes recursive layout:
1. Pending style sheet in some special situation, Document::m_hasNodesWithPlaceholderStyle is true
then mark setNeedsStyleRecalc(SubtreeStyleChange, StyleChangeReasonForTracing::create(StyleChangeReason::CleanupPlaceholderStyles));
2. LayoutTextTrackContainer::layout():
if (updateSizes(toLayoutVideo(*mediaLayoutObject)))
toElement(node())->setInlineStyleProperty(CSSPropertyFontSize, m_fontSize, CSSPrimitiveValue::UnitType::Pixels);
}
3. SVG Filter
PaintLayer::filterNeedsPaintInvalidation()
toElement(layoutObject()->node())->scheduleSVGFilterLayerUpdateHack();
,
Sep 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a83990e41918b82190490fd2be243144a9426ea6 commit a83990e41918b82190490fd2be243144a9426ea6 Author: xiaochengh <xiaochengh@chromium.org> Date: Fri Sep 30 08:35:36 2016 Prune CreateVisibleSelectionDeprecated from PendingSelection BUG= 651373 , 603684 Review-Url: https://codereview.chromium.org/2382993002 Cr-Commit-Position: refs/heads/master@{#422066} [modify] https://crrev.com/a83990e41918b82190490fd2be243144a9426ea6/third_party/WebKit/Source/core/editing/PendingSelection.cpp
,
Oct 12 2016
,
Oct 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e7a92d5235ce35ad2f7127ebbefbcd357e569379 commit e7a92d5235ce35ad2f7127ebbefbcd357e569379 Author: yosin <yosin@chromium.org> Date: Thu Oct 13 03:49:44 2016 Introduce Selection class This patch introduces |Selection| class which is exclude visible position canonicalization from |VisibleSelection|. The relation between |Selection| and |VisibleSelection| is similar to |Position| and |VisiblePosition|. This patch is a preparation of implementing lazy visible position canonicalization on |FrameSelection|, http://crrev.com/1958093002 BUG= 139552 , 603684 , 606499 , 625533 , 644648 TEST=run_webkit_unittests --gtest_filler=SelectionTest.* Review-Url: https://codereview.chromium.org/2393403002 Cr-Commit-Position: refs/heads/master@{#424950} [modify] https://crrev.com/e7a92d5235ce35ad2f7127ebbefbcd357e569379/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/e7a92d5235ce35ad2f7127ebbefbcd357e569379/third_party/WebKit/Source/core/editing/BUILD.gn [add] https://crrev.com/e7a92d5235ce35ad2f7127ebbefbcd357e569379/third_party/WebKit/Source/core/editing/SelectionTemplate.cpp [add] https://crrev.com/e7a92d5235ce35ad2f7127ebbefbcd357e569379/third_party/WebKit/Source/core/editing/SelectionTemplate.h [add] https://crrev.com/e7a92d5235ce35ad2f7127ebbefbcd357e569379/third_party/WebKit/Source/core/editing/SelectionTemplateTest.cpp
,
Nov 28 2016
,
Dec 28 2016
ClusterFuzz testcase 5162739622477824 is flaky and no longer reproduces, so closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Feb 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/157413286770a7ac5a24c446a30c08f749738276 commit 157413286770a7ac5a24c446a30c08f749738276 Author: yosin <yosin@chromium.org> Date: Mon Feb 13 10:55:13 2017 Make FrameSelection to hold non-canonicalized DOM positions This patch makes |FrameSelection| to hold non-canonicalized DOM positions in |SelectionEditor| to align Selection API specification[1] for improving interoperatbility[2]. Before this patch we holds selection as |VisibleSelection| as canonicalized DOM positions. This behavior is not align with Selection API specification[1] then the most complained issue of Blink from editing-tf@w3c. The heart of this patch is holding selection as |SelectionInDOMTree| and compute |VisibleSelection| on-demand with cache of computed |VisibleSelection|. |VisibleSelection| cache is invalidate each DOM tree change and style change since canonicalization referes CSS style properties, e.g display, visibility, -webkit-user-modify, etc, and layout dimension. |SelectionEditor| utilizes |SynchronousMutationObserver| to relocate |m_selectionInDOMTree| instead of |FrameSelection|. Before this patch |FrameSelection| relocates |VisibleSelection| with dirty layout tree then sets |FrameSelection::setSelection()|. To void cyclic reference between |FrameSelection| and |SelectionEditor|, we could not move relocation part to |SelectionEditor|. This patch also updates |FrameSelection::updatePostionAfterAdoptingTextNodesMerged()| to handle |PositonAnchorType|. # Highlight of changes ## FrameCaret - Compute caret position after "layout clean" rather than each selection change to align rendering pipeline. ## CharacterData Changes timing of notifying character data update for ease of relocation of positions. ## FrameSelection - Move |m_isHandleVisible| to |SelectionInDOMTree| as follow-up of [5]. - Move selection relocation to |SelectionEditor|; following patch will move implementations to "SelectionEditor.cpp" ## SelecitonEdtior - Make it to hold |SelectionInDOMTree| with relocation at DOM mutation. - Caching |VisibleSelection| # Brief description of test expectation changes: ## ImeTest.java: This patch gets rid of redundant selection change event from - |testImePaste|, - |testContentEditableEvents_DeleteSurroundingText| - |testInputTextEvents_DeleteSurroundingText| ## LayoutTests Before this patch, Blink uses |VisibleSelection| when it sets even if style and layout changed. This is wrong and unexpected behavior since positions in |VisibleSelection| can no longer be canonicalized positions. This patch changes this behavior to return "sane" canonicalized positions with clean style and layout tree. This patch is the result of many attempts. Previous changes can be found in [3][4]. [1] https://www.w3.org/TR/selection-api/ W3C Selection API [2] https://goo.gl/9v1zOK Improving Interoperatbility of Selection [3] http://crrev.com/1958093002 [4] http://crrev.com/2637013002 [5] http://crrev.com/2651803007 Added isHandleVisible to |SelectionTemplate| BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 TEST=See changes in this patch CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2680943004 Cr-Commit-Position: refs/heads/master@{#449928} [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/execCommand/crash-indenting-list-item.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/execCommand/crash-inserting-list.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/execCommand/remove_format_and_extract_contents.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/selection/character-data-mutation.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/selection/document-mutation.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_details_crash.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_iframe_crash.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/selection/selection_remove_children.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/style/justify-left-crash.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/editing/undo/redo-selection-modify-crash.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/fast/dom/delete-contents.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/fast/dom/shadow/selection-in-nested-shadow.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/fast/dynamic/move-node-with-selection.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/fast/events/drag_and_drop_into_removed_on_focus.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content-expected.txt [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/platform/mac/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/platform/win/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/LayoutTests/svg/foreignObject/viewport-foreignobject-crash-expected.html [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/FrameCaret.cpp [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/FrameCaret.h [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/FrameSelection.h [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/SelectionEditor.h [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/157413286770a7ac5a24c446a30c08f749738276/third_party/WebKit/Source/core/page/FocusController.cpp
,
Feb 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47591962f15392d4af5eb1f69f3b79a8e2990947 commit 47591962f15392d4af5eb1f69f3b79a8e2990947 Author: gcasto <gcasto@chromium.org> Date: Mon Feb 13 18:25:57 2017 Revert of Make FrameSelection to hold non-canonicalized positions (patchset #9 id:180001 of https://codereview.chromium.org/2680943004/ ) Reason for revert: This patch looks like it is causing failures in editing/execCommand/move-up-down-should-skip-hidden-elements.html on Windows 7 (https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Win7%20%28dbg%29). Original issue's description: > Make FrameSelection to hold non-canonicalized DOM positions > > This patch makes |FrameSelection| to hold non-canonicalized DOM positions in > |SelectionEditor| to align Selection API specification[1] for improving > interoperatbility[2]. > > Before this patch we holds selection as |VisibleSelection| as canonicalized > DOM positions. This behavior is not align with Selection API specification[1] > then the most complained issue of Blink from editing-tf@w3c. > > The heart of this patch is holding selection as |SelectionInDOMTree| and > compute |VisibleSelection| on-demand with cache of computed |VisibleSelection|. > > |VisibleSelection| cache is invalidate each DOM tree change and style change > since canonicalization referes CSS style properties, e.g display, visibility, > -webkit-user-modify, etc, and layout dimension. > > |SelectionEditor| utilizes |SynchronousMutationObserver| to relocate > |m_selectionInDOMTree| instead of |FrameSelection|. Before this patch > |FrameSelection| relocates |VisibleSelection| with dirty layout tree then > sets |FrameSelection::setSelection()|. To void cyclic reference between > |FrameSelection| and |SelectionEditor|, we could not move relocation part to > |SelectionEditor|. > > This patch also updates > |FrameSelection::updatePostionAfterAdoptingTextNodesMerged()| to handle > |PositonAnchorType|. > > # Highlight of changes > ## FrameCaret > - Compute caret position after "layout clean" rather than each selection change > to align rendering pipeline. > > ## CharacterData > Changes timing of notifying character data update for ease of relocation of > positions. > > ## FrameSelection > - Move |m_isHandleVisible| to |SelectionInDOMTree| as follow-up of [5]. > - Move selection relocation to |SelectionEditor|; following patch will move > implementations to "SelectionEditor.cpp" > > ## SelecitonEdtior > - Make it to hold |SelectionInDOMTree| with relocation at DOM mutation. > - Caching |VisibleSelection| > > # Brief description of test expectation changes: > ## ImeTest.java: > This patch gets rid of redundant selection change event from > - |testImePaste|, > - |testContentEditableEvents_DeleteSurroundingText| > - |testInputTextEvents_DeleteSurroundingText| > > ## LayoutTests > Before this patch, Blink uses |VisibleSelection| when it sets even if style and > layout changed. This is wrong and unexpected behavior since positions in > |VisibleSelection| can no longer be canonicalized positions. This patch changes > this behavior to return "sane" canonicalized positions with clean style and > layout tree. > > This patch is the result of many attempts. Previous changes can be found in > [3][4]. > > [1] https://www.w3.org/TR/selection-api/ W3C Selection API > [2] https://goo.gl/9v1zOK Improving Interoperatbility of Selection > [3] http://crrev.com/1958093002 > [4] http://crrev.com/2637013002 > [5] http://crrev.com/2651803007 Added isHandleVisible to |SelectionTemplate| > > BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 > TEST=See changes in this patch > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > > Review-Url: https://codereview.chromium.org/2680943004 > Cr-Commit-Position: refs/heads/master@{#449928} > Committed: https://chromium.googlesource.com/chromium/src/+/157413286770a7ac5a24c446a30c08f749738276 TBR=tkent@chromium.org,changwan@chromium.org,xiaochengh@chromium.org,yoichio@chromium.org,yosin@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 Review-Url: https://codereview.chromium.org/2694823002 Cr-Commit-Position: refs/heads/master@{#450018} [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/execCommand/crash-indenting-list-item.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/execCommand/crash-inserting-list.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/execCommand/remove_format_and_extract_contents.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/selection/character-data-mutation.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/selection/document-mutation.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_details_crash.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_iframe_crash.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/selection/selection_remove_children.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/style/justify-left-crash.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/editing/undo/redo-selection-modify-crash.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/fast/dom/delete-contents.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/fast/dom/shadow/selection-in-nested-shadow.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/fast/dynamic/move-node-with-selection.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/fast/events/drag_and_drop_into_removed_on_focus.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content-expected.txt [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/platform/mac/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/platform/win/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/LayoutTests/svg/foreignObject/viewport-foreignobject-crash-expected.html [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/FrameCaret.cpp [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/FrameCaret.h [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/FrameSelection.h [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/SelectionEditor.h [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/47591962f15392d4af5eb1f69f3b79a8e2990947/third_party/WebKit/Source/core/page/FocusController.cpp
,
Feb 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17c84b2b6519c821dc319e79f2a7a4508508e20f commit 17c84b2b6519c821dc319e79f2a7a4508508e20f Author: yosin <yosin@chromium.org> Date: Tue Feb 14 06:34:37 2017 Make FrameSelection to hold non-canonicalized DOM positions This patch makes |FrameSelection| to hold non-canonicalized DOM positions in |SelectionEditor| to align Selection API specification[1] for improving interoperatbility[2]. Before this patch we holds selection as |VisibleSelection| as canonicalized DOM positions. This behavior is not align with Selection API specification[1] then the most complained issue of Blink from editing-tf@w3c. The heart of this patch is holding selection as |SelectionInDOMTree| and compute |VisibleSelection| on-demand with cache of computed |VisibleSelection|. |VisibleSelection| cache is invalidate each DOM tree change and style change since canonicalization referes CSS style properties, e.g display, visibility, -webkit-user-modify, etc, and layout dimension. |SelectionEditor| utilizes |SynchronousMutationObserver| to relocate |m_selectionInDOMTree| instead of |FrameSelection|. Before this patch |FrameSelection| relocates |VisibleSelection| with dirty layout tree then sets |FrameSelection::setSelection()|. To void cyclic reference between |FrameSelection| and |SelectionEditor|, we could not move relocation part to |SelectionEditor|. This patch also updates |FrameSelection::updatePostionAfterAdoptingTextNodesMerged()| to handle |PositonAnchorType|. # Highlight of changes ## FrameCaret - Compute caret position after "layout clean" rather than each selection change to align rendering pipeline. ## CharacterData Changes timing of notifying character data update for ease of relocation of positions. ## FrameSelection - Move |m_isHandleVisible| to |SelectionInDOMTree| as follow-up of [5]. - Move selection relocation to |SelectionEditor|; following patch will move implementations to "SelectionEditor.cpp" ## SelecitonEdtior - Make it to hold |SelectionInDOMTree| with relocation at DOM mutation. - Caching |VisibleSelection| # Brief description of test expectation changes: ## ImeTest.java: This patch gets rid of redundant selection change event from - |testImePaste|, - |testContentEditableEvents_DeleteSurroundingText| - |testInputTextEvents_DeleteSurroundingText| ## LayoutTests Before this patch, Blink uses |VisibleSelection| when it sets even if style and layout changed. This is wrong and unexpected behavior since positions in |VisibleSelection| can no longer be canonicalized positions. This patch changes this behavior to return "sane" canonicalized positions with clean style and layout tree. This patch is the result of many attempts. Previous changes can be found in [3][4]. [1] https://www.w3.org/TR/selection-api/ W3C Selection API [2] https://goo.gl/9v1zOK Improving Interoperatbility of Selection [3] http://crrev.com/1958093002 [4] http://crrev.com/2637013002 [5] http://crrev.com/2651803007 Added isHandleVisible to |SelectionTemplate| BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 TEST=See changes in this patch CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2680943004 Cr-Original-Commit-Position: refs/heads/master@{#449928} Committed: https://chromium.googlesource.com/chromium/src/+/157413286770a7ac5a24c446a30c08f749738276 Review-Url: https://codereview.chromium.org/2680943004 Cr-Commit-Position: refs/heads/master@{#450280} [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/execCommand/crash-indenting-list-item.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/execCommand/crash-inserting-list.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/execCommand/remove_format_and_extract_contents.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/selection/character-data-mutation.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/selection/document-mutation.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_details_crash.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_iframe_crash.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/selection/selection_remove_children.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/style/justify-left-crash.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/editing/undo/redo-selection-modify-crash.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/fast/dom/delete-contents.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/fast/dom/shadow/selection-in-nested-shadow.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/fast/dynamic/move-node-with-selection.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/fast/events/drag_and_drop_into_removed_on_focus.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content-expected.txt [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/platform/mac/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/platform/win/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/LayoutTests/svg/foreignObject/viewport-foreignobject-crash-expected.html [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/FrameCaret.cpp [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/FrameCaret.h [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/FrameSelection.h [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/SelectionEditor.h [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/17c84b2b6519c821dc319e79f2a7a4508508e20f/third_party/WebKit/Source/core/page/FocusController.cpp
,
Feb 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f2a1baf12aa71d59d72e24583983b8a6b78340a commit 8f2a1baf12aa71d59d72e24583983b8a6b78340a Author: tyoshino <tyoshino@chromium.org> Date: Tue Feb 14 08:18:39 2017 Revert of Make FrameSelection to hold non-canonicalized positions (patchset #9 id:180001 of https://codereview.chromium.org/2680943004/ ) Reason for revert: See https://codereview.chromium.org/2680943004/#msg70 Original issue's description: > Make FrameSelection to hold non-canonicalized DOM positions > > This patch makes |FrameSelection| to hold non-canonicalized DOM positions in > |SelectionEditor| to align Selection API specification[1] for improving > interoperatbility[2]. > > Before this patch we holds selection as |VisibleSelection| as canonicalized > DOM positions. This behavior is not align with Selection API specification[1] > then the most complained issue of Blink from editing-tf@w3c. > > The heart of this patch is holding selection as |SelectionInDOMTree| and > compute |VisibleSelection| on-demand with cache of computed |VisibleSelection|. > > |VisibleSelection| cache is invalidate each DOM tree change and style change > since canonicalization referes CSS style properties, e.g display, visibility, > -webkit-user-modify, etc, and layout dimension. > > |SelectionEditor| utilizes |SynchronousMutationObserver| to relocate > |m_selectionInDOMTree| instead of |FrameSelection|. Before this patch > |FrameSelection| relocates |VisibleSelection| with dirty layout tree then > sets |FrameSelection::setSelection()|. To void cyclic reference between > |FrameSelection| and |SelectionEditor|, we could not move relocation part to > |SelectionEditor|. > > This patch also updates > |FrameSelection::updatePostionAfterAdoptingTextNodesMerged()| to handle > |PositonAnchorType|. > > # Highlight of changes > ## FrameCaret > - Compute caret position after "layout clean" rather than each selection change > to align rendering pipeline. > > ## CharacterData > Changes timing of notifying character data update for ease of relocation of > positions. > > ## FrameSelection > - Move |m_isHandleVisible| to |SelectionInDOMTree| as follow-up of [5]. > - Move selection relocation to |SelectionEditor|; following patch will move > implementations to "SelectionEditor.cpp" > > ## SelecitonEdtior > - Make it to hold |SelectionInDOMTree| with relocation at DOM mutation. > - Caching |VisibleSelection| > > # Brief description of test expectation changes: > ## ImeTest.java: > This patch gets rid of redundant selection change event from > - |testImePaste|, > - |testContentEditableEvents_DeleteSurroundingText| > - |testInputTextEvents_DeleteSurroundingText| > > ## LayoutTests > Before this patch, Blink uses |VisibleSelection| when it sets even if style and > layout changed. This is wrong and unexpected behavior since positions in > |VisibleSelection| can no longer be canonicalized positions. This patch changes > this behavior to return "sane" canonicalized positions with clean style and > layout tree. > > This patch is the result of many attempts. Previous changes can be found in > [3][4]. > > [1] https://www.w3.org/TR/selection-api/ W3C Selection API > [2] https://goo.gl/9v1zOK Improving Interoperatbility of Selection > [3] http://crrev.com/1958093002 > [4] http://crrev.com/2637013002 > [5] http://crrev.com/2651803007 Added isHandleVisible to |SelectionTemplate| > > BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 > TEST=See changes in this patch > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > > Review-Url: https://codereview.chromium.org/2680943004 > Cr-Original-Commit-Position: refs/heads/master@{#449928} > Committed: https://chromium.googlesource.com/chromium/src/+/157413286770a7ac5a24c446a30c08f749738276 > Review-Url: https://codereview.chromium.org/2680943004 > Cr-Commit-Position: refs/heads/master@{#450280} > Committed: https://chromium.googlesource.com/chromium/src/+/17c84b2b6519c821dc319e79f2a7a4508508e20f TBR=changwan@chromium.org,tkent@chromium.org,xiaochengh@chromium.org,yoichio@chromium.org,yosin@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 Review-Url: https://codereview.chromium.org/2691243002 Cr-Commit-Position: refs/heads/master@{#450291} [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/execCommand/crash-indenting-list-item.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/execCommand/crash-inserting-list.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/execCommand/remove_format_and_extract_contents.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/selection/character-data-mutation.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/selection/document-mutation.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_details_crash.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_iframe_crash.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/selection/selection_remove_children.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/style/justify-left-crash.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/editing/undo/redo-selection-modify-crash.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/fast/dom/delete-contents.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/fast/dom/shadow/selection-in-nested-shadow.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/fast/dynamic/move-node-with-selection.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/fast/events/drag_and_drop_into_removed_on_focus.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content-expected.txt [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/platform/mac/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/platform/win/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/LayoutTests/svg/foreignObject/viewport-foreignobject-crash-expected.html [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/FrameCaret.cpp [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/FrameCaret.h [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/FrameSelection.h [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/SelectionEditor.h [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/8f2a1baf12aa71d59d72e24583983b8a6b78340a/third_party/WebKit/Source/core/page/FocusController.cpp
,
Feb 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d892f9592860691ae9a782c12260c94ed6bd1a63 commit d892f9592860691ae9a782c12260c94ed6bd1a63 Author: yosin <yosin@chromium.org> Date: Tue Feb 14 15:56:00 2017 Make FrameSelection to hold non-canonicalized DOM positions This patch makes |FrameSelection| to hold non-canonicalized DOM positions in |SelectionEditor| to align Selection API specification[1] for improving interoperatbility[2]. Before this patch we holds selection as |VisibleSelection| as canonicalized DOM positions. This behavior is not align with Selection API specification[1] then the most complained issue of Blink from editing-tf@w3c. The heart of this patch is holding selection as |SelectionInDOMTree| and compute |VisibleSelection| on-demand with cache of computed |VisibleSelection|. |VisibleSelection| cache is invalidate each DOM tree change and style change since canonicalization referes CSS style properties, e.g display, visibility, -webkit-user-modify, etc, and layout dimension. |SelectionEditor| utilizes |SynchronousMutationObserver| to relocate |m_selectionInDOMTree| instead of |FrameSelection|. Before this patch |FrameSelection| relocates |VisibleSelection| with dirty layout tree then sets |FrameSelection::setSelection()|. To void cyclic reference between |FrameSelection| and |SelectionEditor|, we could not move relocation part to |SelectionEditor|. This patch also updates |FrameSelection::updatePostionAfterAdoptingTextNodesMerged()| to handle |PositonAnchorType|. # Highlight of changes ## FrameCaret - Compute caret position after "layout clean" rather than each selection change to align rendering pipeline. ## CharacterData Changes timing of notifying character data update for ease of relocation of positions. ## FrameSelection - Move |m_isHandleVisible| to |SelectionInDOMTree| as follow-up of [5]. - Move selection relocation to |SelectionEditor|; following patch will move implementations to "SelectionEditor.cpp" ## SelecitonEdtior - Make it to hold |SelectionInDOMTree| with relocation at DOM mutation. - Caching |VisibleSelection| # Brief description of test expectation changes: ## ImeTest.java: This patch gets rid of redundant selection change event from - |testImePaste|, - |testContentEditableEvents_DeleteSurroundingText| - |testInputTextEvents_DeleteSurroundingText| ## LayoutTests Before this patch, Blink uses |VisibleSelection| when it sets even if style and layout changed. This is wrong and unexpected behavior since positions in |VisibleSelection| can no longer be canonicalized positions. This patch changes this behavior to return "sane" canonicalized positions with clean style and layout tree. This patch is the result of many attempts. Previous changes can be found in [3][4]. [1] https://www.w3.org/TR/selection-api/ W3C Selection API [2] https://goo.gl/9v1zOK Improving Interoperatbility of Selection [3] http://crrev.com/1958093002 [4] http://crrev.com/2637013002 [5] http://crrev.com/2651803007 Added isHandleVisible to |SelectionTemplate| BUG= 139552 , 603684 , 605499 , 606499 , 625533 , 644648 , 679991 TEST=See changes in this patch CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2680943004 Cr-Original-Original-Commit-Position: refs/heads/master@{#449928} Committed: https://chromium.googlesource.com/chromium/src/+/157413286770a7ac5a24c446a30c08f749738276 Review-Url: https://codereview.chromium.org/2680943004 Cr-Original-Commit-Position: refs/heads/master@{#450280} Committed: https://chromium.googlesource.com/chromium/src/+/17c84b2b6519c821dc319e79f2a7a4508508e20f Review-Url: https://codereview.chromium.org/2680943004 Cr-Commit-Position: refs/heads/master@{#450370} [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/deleting/delete-br-001-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/deleting/delete-br-001.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/deleting/delete-character-003-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/deleting/delete-character-003.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/execCommand/crash-indenting-list-item.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/execCommand/crash-inserting-list.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/execCommand/format-block-multiple-paragraphs-in-pre.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/execCommand/remove_format_and_extract_contents.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/selection/character-data-mutation.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/selection/document-mutation.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_details_crash.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_iframe_crash.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/selection/selection_remove_children.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/style/justify-left-crash.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/editing/undo/redo-selection-modify-crash.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/external/wpt/selection/collapse-00-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/external/wpt/selection/collapse-30-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/external/wpt/selection/collapseToStartEnd-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/fast/dom/delete-contents.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/fast/dom/shadow/selection-in-nested-shadow.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/fast/dynamic/move-node-with-selection.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/fast/events/drag_and_drop_into_removed_on_focus.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/images/element-gcd-while-generating-alt-content-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/platform/mac/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/platform/win/fast/css/first-letter-rtc-crash-expected.txt [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/LayoutTests/svg/foreignObject/viewport-foreignobject-crash-expected.html [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/FrameCaret.cpp [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/FrameCaret.h [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/FrameSelection.h [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/SelectionEditor.h [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/frame/FrameView.cpp [modify] https://crrev.com/d892f9592860691ae9a782c12260c94ed6bd1a63/third_party/WebKit/Source/core/page/FocusController.cpp |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by wangxianzhu@chromium.org
, Apr 14 2016