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

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

Committed: https://crrev.com/f202a1b8dc8660efa8b173efcaab00675b922daa
Cr-Commit-Position: refs/heads/master@{#414826}

[dglazkov, 2016-08-23 18:58:55]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-23 18:59:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dglazkov, 2016-08-23 19:40:14]

PTAL.

[esprehn, 2016-08-23 20:47:28]

Can haz test?

[commit-bot: I haz the power, 2016-08-23 21:04:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-23 21:04:02]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-08-23 21:25:55]

On 2016/08/23 at 20:47:28, esprehn wrote:
> Can haz test?

Let me think about that.

[dglazkov, 2016-08-24 05:02:23]

First stab at the unit test.

[dglazkov, 2016-08-24 05:02:25]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-24 05:02:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 05:13:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-24 05:13:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[esprehn, 2016-08-25 23:55:16]

On 2016/08/24 at 05:02:23, dglazkov wrote:
> First stab at the unit test.

lgtm to me, but everything is red.

[dglazkov, 2016-08-26 05:00:03]

Actually working test added.

[dglazkov, 2016-08-26 05:00:04]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 05:00:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[esprehn, 2016-08-26 05:23:03]

https://codereview.chromium.org/2272793002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebViewTest.cpp (right):

https://codereview.chromium.org/2272793002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:738: WebViewImpl*
webViewImpl = m_webViewHelper.initialize(true, 0, &client);
I suspect this would be a pretty simple SimTest without all of this setup

https://codereview.chromium.org/2272793002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:748: "<svg height=\"100%\"
version=\"1.1\" viewBox=\"0 0 14 14\" width=\"100%\">"
if you use single quotes you can avoid the escapes all over

https://codereview.chromium.org/2272793002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:757:
mainFrameLocal->document()->body()->querySelector("path",
IGNORE_EXCEPTION)->setIdAttribute("foo");
don't ignore the exception, ASSERT_NO_EXCEPTION, which is the default, is what
you want.


Also:

webViewImpl->mainFrameImpl()->frame()->document() does what you want here, no
need to touch page()

[commit-bot: I haz the power, 2016-08-26 06:29:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 06:29:32]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-08-26 19:48:37]

Comments addressed.

[dglazkov, 2016-08-26 19:48:38]

The CQ bit was checked by dglazkov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 19:49:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 21:49:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 21:49:40]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-08-26 21:53:07]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2016-08-26 21:53:07]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2272793002/#ps60001
(title: "Comments addressed.")

[commit-bot: I haz the power, 2016-08-26 21:54:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 22:00:00]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-26 22:01:13]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/f202a1b8dc8660efa8b173efcaab00675b922daa
Cr-Commit-Position: refs/heads/master@{#414826}
==========

[commit-bot: I haz the power, 2016-08-26 22:01:14]

Patchset 4 (id:??) landed as
https://crrev.com/f202a1b8dc8660efa8b173efcaab00675b922daa
Cr-Commit-Position: refs/heads/master@{#414826}

[Xiaocheng, 2016-08-30 15:18:23]

xiaochengh@chromium.org changed reviewers:
+ xiaochengh@chromium.org

[Xiaocheng, 2016-08-30 15:18:24]

https://codereview.chromium.org/2272793002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/2272793002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebViewImpl.cpp:2445:
DocumentLifecycle::DisallowTransitionScope(focused->document()->lifecycle());
Pass-by: I think we need to have a named scope here, like what
crrev.com/2297503002 does. Otherwise the scope just gets destructed immediately.

[dglazkov, 2016-08-30 16:11:19]

On 2016/08/30 at 15:18:24, xiaochengh wrote:
>
https://codereview.chromium.org/2272793002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/WebViewImpl.cpp (right):
> 
>
https://codereview.chromium.org/2272793002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/WebViewImpl.cpp:2445:
DocumentLifecycle::DisallowTransitionScope(focused->document()->lifecycle());
> Pass-by: I think we need to have a named scope here, like what
crrev.com/2297503002 does. Otherwise the scope just gets destructed immediately.

Makes sense! Sorry about that. Will add.