I thought that this is a regression issue caused by https://codereview.chromium.org/1909343003

dglazkov@ was this caused by your change as indicated in #1?

Thank you for the report.

Hmmm... The DCHECK seems legit. It looks like updating style+layout caused a tree mutation.

Do you know what the full DCHECK stack trace looks like?

Here's the stack trace:

[53664:54787:0822/113646:8270007556390:FATAL:EphemeralRange.cpp(103)] Check failed: isValid(). 
0   libbase.dylib                       0x000000011483e4ae _ZN4base5debug10StackTraceC2Ev + 30
1   libbase.dylib                       0x000000011483e515 _ZN4base5debug10StackTraceC1Ev + 21
2   libbase.dylib                       0x00000001148ccff0 _ZN7logging10LogMessageD2Ev + 80
3   libbase.dylib                       0x00000001148caba5 _ZN7logging10LogMessageD1Ev + 21
4   libblink_core.dylib                 0x0000000121e38fee _ZNK5blink22EphemeralRangeTemplateINS_16EditingAlgorithmINS_13NodeTraversalEEEE13startPositionEv + 254
5   libblink_core.dylib                 0x0000000121f17e18 _ZN5blinkL15createPlainTextINS_16EditingAlgorithmINS_13NodeTraversalEEEEEN3WTF6StringERKNS_22EphemeralRangeTemplateIT_EEj + 168
6   libblink_core.dylib                 0x0000000121f17d62 _ZN5blink9plainTextERKNS_22EphemeralRangeTemplateINS_16EditingAlgorithmINS_13NodeTraversalEEEEEj + 34
7   libblink_web.dylib                  0x000000011e82b733 _ZN5blink11WebViewImpl13textInputInfoEv + 499
8   libcontent.dylib                    0x000000010e089e5e _ZN7content12RenderWidget20UpdateTextInputStateENS_7ShowImeENS_12ChangeSourceE + 494
9   libcontent.dylib                    0x000000010e088f8b _ZN7content12RenderWidget24WillBeginCompositorFrameEv + 267
10  libcontent.dylib                    0x000000010de2031a _ZN7content22RenderWidgetCompositor18WillBeginMainFrameEv + 26
11  libcc.dylib                         0x000000011883cdc3 _ZN2cc13LayerTreeHost18WillBeginMainFrameEv + 67
12  libcc.dylib                         0x000000011895683c _ZN2cc9ProxyMain14BeginMainFrameENSt3__110unique_ptrINS_28BeginMainFrameAndCommitStateENS1_14default_deleteIS3_EEEE + 4844
13  libcc.dylib                         0x00000001189937c1 _ZN4base8internal13FunctorTraitsIMN2cc9ProxyMainEFvNSt3__110unique_ptrINS2_28BeginMainFrameAndCommitStateENS4_14default_deleteIS6_EEEEEvE6InvokeIRKNS_7WeakPtrIS3_EEJS9_EEEvSB_OT_DpOT0_ + 529
14  libcc.dylib                         0x0000000118993529 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN2cc9ProxyMainEFvNSt3__110unique_ptrINS4_28BeginMainFrameAndCommitStateENS6_14default_deleteIS8_EEEEERKNS_7WeakPtrIS5_EEJSB_EEEvOT_OT0_DpOT1_ + 105
15  libcc.dylib                         0x00000001189933d0 _ZN4base8internal7InvokerINS0_9BindStateIMN2cc9ProxyMainEFvNSt3__110unique_ptrINS3_28BeginMainFrameAndCommitStateENS5_14default_deleteIS7_EEEEEJNS_7WeakPtrIS4_EENS0_13PassedWrapperISA_EEEEEFvvEE7RunImplIRKSC_RKNS5_5tupleIJSE_SG_EEEJLm0ELm1EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE + 160
16  libcc.dylib                         0x0000000118992cec _ZN4base8internal7InvokerINS0_9BindStateIMN2cc9ProxyMainEFvNSt3__110unique_ptrINS3_28BeginMainFrameAndCommitStateENS5_14default_deleteIS7_EEEEEJNS_7WeakPtrIS4_EENS0_13PassedWrapperISA_EEEEEFvvEE3RunEPNS0_13BindStateBaseE + 44
17  libbase.dylib                       0x00000001147fc8ee _ZNK4base8CallbackIFvvELNS_8internal8CopyModeE1EE3RunEv + 46
18  libbase.dylib                       0x000000011483ffe4 _ZN4base5debug13TaskAnnotator7RunTaskEPKcRKNS_11PendingTaskE + 676
19  libblink_platform.dylib             0x000000011c9ef2f5 _ZN5blink9scheduler16TaskQueueManager24ProcessTaskFromWorkQueueEPNS0_8internal9WorkQueueEPNS2_13TaskQueueImpl4TaskE + 1589
20  libblink_platform.dylib             0x000000011c9ebb23 _ZN5blink9scheduler16TaskQueueManager6DoWorkEN4base9TimeTicksEb + 1187
21  libblink_platform.dylib             0x000000011c9f3e67 _ZN4base8internal13FunctorTraitsIMN5blink9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEvE6InvokeIRKNS_7WeakPtrIS4_EEJRKS5_RKbEEEvS7_OT_DpOT0_ + 199
22  libblink_platform.dylib             0x000000011c9f3c80 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN5blink9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbERKNS_7WeakPtrIS6_EEJRKS7_RKbEEEvOT_OT0_DpOT1_ + 128
23  libblink_platform.dylib             0x000000011c9f3bfa _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEJNS_7WeakPtrIS5_EES6_bEEEFvvEE7RunImplIRKS8_RKNSt3__15tupleIJSA_S6_bEEEJLm0ELm1ELm2EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE + 138
24  libblink_platform.dylib             0x000000011c9f35fc _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvNS_9TimeTicksEbEJNS_7WeakPtrIS5_EES6_bEEEFvvEE3RunEPNS0_13BindStateBaseE + 44
25  libbase.dylib                       0x00000001147fc8ee _ZNK4base8CallbackIFvvELNS_8internal8CopyModeE1EE3RunEv + 46
26  libbase.dylib                       0x000000011483ffe4 _ZN4base5debug13TaskAnnotator7RunTaskEPKcRKNS_11PendingTaskE + 676
27  libbase.dylib                       0x0000000114916f6d _ZN4base11MessageLoop7RunTaskERKNS_11PendingTaskE + 877
28  libbase.dylib                       0x0000000114917554 _ZN4base11MessageLoop21DeferOrRunPendingTaskENS_11PendingTaskE + 68
29  libbase.dylib                       0x0000000114917fcd _ZN4base11MessageLoop6DoWorkEv + 669
30  libbase.dylib                       0x0000000114929118 _ZN4base24MessagePumpCFRunLoopBase7RunWorkEv + 104
31  libbase.dylib                       0x000000011492909c ___ZN4base24MessagePumpCFRunLoopBase13RunWorkSourceEPv_block_invoke + 28
32  libbase.dylib                       0x00000001148d104a _ZN4base3mac15CallWithEHFrameEU13block_pointerFvvE + 10
33  libbase.dylib                       0x0000000114928615 _ZN4base24MessagePumpCFRunLoopBase13RunWorkSourceEPv + 101
34  CoreFoundation                      0x00007fff884f9881 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
35  CoreFoundation                      0x00007fff884d8fbc __CFRunLoopDoSources0 + 556
36  CoreFoundation                      0x00007fff884d84df __CFRunLoopRun + 927
37  CoreFoundation                      0x00007fff884d7ed8 CFRunLoopRunSpecific + 296
38  libbase.dylib                       0x000000011492977c _ZN4base20MessagePumpCFRunLoop5DoRunEPNS_11MessagePump8DelegateE + 76
39  libbase.dylib                       0x0000000114928dfa _ZN4base24MessagePumpCFRunLoopBase3RunEPNS_11MessagePump8DelegateE + 122
40  libbase.dylib                       0x000000011491677a _ZN4base11MessageLoop10RunHandlerEv + 298
41  libbase.dylib                       0x00000001149e47d5 _ZN4base7RunLoop3RunEv + 85
42  libbase.dylib                       0x0000000114ade137 _ZN4base6Thread3RunEPNS_7RunLoopE + 215
43  libbase.dylib                       0x0000000114adef7d _ZN4base6Thread10ThreadMainEv + 2525
44  libbase.dylib                       0x0000000114aba2f1 _ZN4base12_GLOBAL__N_110ThreadFuncEPv + 705
45  libsystem_pthread.dylib             0x00007fff8d87799d _pthread_body + 131
46  libsystem_pthread.dylib             0x00007fff8d87791a _pthread_body + 0
47  libsystem_pthread.dylib             0x00007fff8d875351 thread_start + 13

The problem here is that createPlainText takes in an EphemeralRange, and the updating of the style+layout invalidates it.

The fix is to move the updateStyleAndLayoutIgnorePendingStylesheets call up to the call stack, until just before the EphemeralRange is constructed. Then, you'll have the right order of events.

For an example of this moving-up-the-call-stack see https://codereview.chromium.org/2003793002 and https://codereview.chromium.org/2144673003.

If you're up to this work, I would be happy to review it. Let me know.

Hello dglazkov,

At first, I thought about the solution, "moving-up-the-call-stack", which means that calling updateStyleAndLayoutIgnorePendingStylesheets() before the EphemeralRange is constructed.

The EphemeralRange is argument passed by createPlainText()'s callers. 
I thought 2 options. 
 - calling updateStyleAndLayoutIgnorePendingStylesheets() before createPlainText(). 
But so many callers.... 
 - in createPlainText(), reContruct the EphemeralRange after updateStyleAndLayout..() 
   I think it's the best.
   But we have no enough information. 
   In this case, (youtube),  we need specific element. 
   WebViewImpl::textInputInfo() is caller of createPlainText(). 

{code}
    info.value = plainText(EphemeralRange::rangeOfContents(*element), TextIteratorEmitsObjectReplacementCharacter);
{code}

in history, before https://codereview.chromium.org/1909343003 is committed,
we had right order of event.
Because updateStyleAndLayoutIgnorePendingStylesheets() is called in TextIteratorAlgorithm contructor and range.startPosition() / endPosition() is called before the TextIteratorAlgorithm constructor. 
The https://codereview.chromium.org/1909343003 changed the call-stack. 

I made the patch keeping the current status because I'm not sure updateStyleAndLayoutIgnorePendingStylesheets() should be called before the EphemeralRnage constructor. 

Could you suggest the better idea? 
Welcome any ideas. Thanks in advance.

This is not as difficult as you might imagine. Here's the CL that fixes the crash:

https://codereview.chromium.org/2272793002

The updateStyleAndLayoutIgnorePendingStylesheets is (in 99% of cases) idempotent, so the fix here is to simply call it earlier.

Also, the discussion about the order:

No, the order was not correct prior to https://codereview.chromium.org/1909343003. It was just papering over the problem. The whole point of the checks that we hit is that an EphemeralRange has to operate on the version of tree that it was created on. Forcing a pipeline flush (updateStyleAndLayout) sometimes causes the mutation, which means that the range would become invalid. There were just not checks to catch that.

https://codereview.chromium.org/2272793002 was my first option.
But it solves only these case, isn't it?

I mean there are so many callers of plainText()
for example, 
{code}
in TextCheckingHelper::findFirstMisspellingOrBadGrammar()
String paragraphString = plainText(EphemeralRange(paragraphStart, paragraphEnd));

in SelectionController::selectClosestWordFromHitTestResult()
const String& str = plainText(range, TextIteratorEmitsObjectReplacementCharacter);
. . . 
{code}

I didn't find or test the test case using this code yet. 
But logically in code, I thought it's possible to be failed on asssert inside plainText().
Range is passed by caller and updateStyle() is called inside plainText(). 

https://codereview.chromium.org/2272793002 remains these possible cases. 

Because https://codereview.chromium.org/1909343003 broke the problem's cover, I thought we need to check all plainText()'s caller or better idea.

Right. I think you're agreeing with my comment 5.

If you'd like, you can try to submit a CL that audits all other cases. I'll think on the test case puzzle that esprehn@ gave me :)

@Github @zendezk @appveyor @Hektve87 @VSTeam @nuget

Functions having DeprecatedScheduleStyleRecalcDuringLayout causes dirty layout tree
after layout() calls, e.g. LayoutTextTrackContainer.

How about calling updateStyleAndLayoutIfNeededRecursive() again in FrameView::FrameView::updateLifecyclePhasesInternal()?

void FrameView::updateLifecyclePhasesInternal(...) {
...
    updateStyleAndLayoutIfNeededRecursive();
    DCHECK(lifecycle().state() >= DocumentLifecycle::LayoutClean);
    // Even if lifecycle >= LayoutClean, functions having DeprecatedScheduleStyleRecalcDuringLayout
    // makes layout tree dirty.
    if (m_frame->document()->needsLayoutTreeUpdate())
        updateStyleAndLayoutIfNeededRecursive();
...
}

I would like for us to try and remove the remaining uses of DeprecatedScheduleStyleRecalcDuringLayout instead (see  bug 370457 ).

BTW, this particular bug is not related to the issue you've mentioned. This one is just about proper ordering of things.

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

commit f202a1b8dc8660efa8b173efcaab00675b922daa
Author: dglazkov <dglazkov@chromium.org>
Date: Fri Aug 26 21:59:38 2016

Make WebViewImpl::textInputInfo update layout before working on ranges.

EphemeralRange is supposed to operate on the same version of the tree
it was created. Unfortunately, the updateStyleAndLayout call in
plainText was causing a DOM mutation in some cases, thus
invalidating the range. Let's not do that.

BUG= 639728 
R=esprehn

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

[modify] https://crrev.com/f202a1b8dc8660efa8b173efcaab00675b922daa/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/f202a1b8dc8660efa8b173efcaab00675b922daa/third_party/WebKit/Source/web/tests/WebViewTest.cpp