Update layout in WebInputMethodController::finishComposingText

Update layout in WebInputMethodController::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from
WebInputMethodController::finishComposingText. This patch fixes the issue
by adding a layout update call there.

BUG=663645
TEST=webkit_unit_tests --gtest_filter=All/WebViewTest.FinishComposingTextDoesNotCrash/*

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

[Xiaocheng, 2016-11-10 07:50:15]

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

[commit-bot: I haz the power, 2016-11-10 07:50:34]

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

[Xiaocheng, 2016-11-10 07:51:11]

Description was changed from

==========
Update layout in WebViewImpl::finishComposingText

BUG=
==========

to

==========
Update layout in WebViewImpl::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from WebViewImpl::finishComposingText.
This patch fixes the issue by adding a layout update call there.

BUG=663645
==========

[Xiaocheng, 2016-11-10 08:52:11]

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

[Xiaocheng, 2016-11-10 08:52:11]

PTAL.

[commit-bot: I haz the power, 2016-11-10 09:10:40]

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

[commit-bot: I haz the power, 2016-11-10 09:10:41]

Dry run: This issue passed the CQ dry run.

[aelias_OOO_until_Jul13, 2016-11-10 19:31:48]

Can you write a unit test for this?  I'd suggest doing it as a C++ unit test
(not layout test) since this might be timing dependent.  Probably in
InputMethodControllerTest (although WebFrameTest/WebViewTest is OK if that
doesn't fit there) .  You could load up some representative HTML and just
perform the call sequence; you don't necessarily need any EXPECT statements
since you just want to verify that no assertion is fired.

[yosin_UTC9, 2016-11-11 03:47:43]

On 2016/11/10 at 19:31:48, aelias wrote:
> Can you write a unit test for this?  I'd suggest doing it as a C++ unit test
(not layout test) since this might be timing dependent.  Probably in
InputMethodControllerTest (although WebFrameTest/WebViewTest is OK if that
doesn't fit there) .  You could load up some representative HTML and just
perform the call sequence; you don't necessarily need any EXPECT statements
since you just want to verify that no assertion is fired.

Could you add DCHECK(!document.needsLayoutTreeUpdate()) at entry of
InputMethodController::finishComposingText()?

I think a test should be in WebFrameTest or WebViewTest since this is
WebViewImpl.cpp issue rather than IMC; though DCHECK() is missing...

[Xiaocheng, 2016-11-11 05:14:40]

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

[commit-bot: I haz the power, 2016-11-11 05:14:53]

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

[Xiaocheng, 2016-11-11 05:15:46]

Description was changed from

==========
Update layout in WebViewImpl::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from WebViewImpl::finishComposingText.
This patch fixes the issue by adding a layout update call there.

BUG=663645
==========

to

==========
Update layout in WebViewImpl::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from WebViewImpl::finishComposingText.
This patch fixes the issue by adding a layout update call there.

BUG=663645
TEST=webkit_unit_tests
--gtest_filter=All/WebViewTest.FinishComposingTextDoesNotCrash/*
==========

[Xiaocheng, 2016-11-11 05:16:41]

Description was changed from

==========
Update layout in WebViewImpl::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from WebViewImpl::finishComposingText.
This patch fixes the issue by adding a layout update call there.

BUG=663645
TEST=webkit_unit_tests
--gtest_filter=All/WebViewTest.FinishComposingTextDoesNotCrash/*
==========

to

==========
Update layout in WebInputMethodController::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from
WebInputMethodController::finishComposingText. This patch fixes the issue
by adding a layout update call there.

BUG=663645
TEST=webkit_unit_tests
--gtest_filter=All/WebViewTest.FinishComposingTextDoesNotCrash/*
==========

[Xiaocheng, 2016-11-11 05:25:26]

Updated with a WebViewTest. PTAL.

Note that the relevant web/ level functions were recently moved to
WebInputMethodController by https://codereview.chromium.org/2333813002.

We should add DCHECK(!frame()->document()->needsLayoutTreeUpdate()) to functions
of InputMethodController in a separate patch.

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

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

[commit-bot: I haz the power, 2016-11-11 06:32:30]

Dry run: This issue passed the CQ dry run.

[aelias_OOO_until_Jul13, 2016-11-11 20:26:52]

Source/web lgtm

[aelias_OOO_until_Jul13, 2016-11-11 20:27:32]

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

https://codereview.chromium.org/2491643004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebViewTest.cpp:901: TEST_P(WebViewTest,
FinishComposingTextDoesNotCrash) {
There's never been a crash here, please rename the test to
FinishComposingTextDoesNotAssert

[Xiaocheng, 2016-11-14 03:14:05]

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

[commit-bot: I haz the power, 2016-11-14 03:14:18]

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-11-14 04:34:11]

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

[commit-bot: I haz the power, 2016-11-14 04:34:12]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-11-14 06:29:12]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2016-11-14 06:29:13]

Thanks for the review.

Committing with the test name changed.

[Xiaocheng, 2016-11-14 06:29:13]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org
Link to the patchset: https://codereview.chromium.org/2491643004/#ps40001
(title: "Change test name")

[commit-bot: I haz the power, 2016-11-14 06:29:21]

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-11-14 06:34:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-14 06:36:18]

Description was changed from

==========
Update layout in WebInputMethodController::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from
WebInputMethodController::finishComposingText. This patch fixes the issue
by adding a layout update call there.

BUG=663645
TEST=webkit_unit_tests
--gtest_filter=All/WebViewTest.FinishComposingTextDoesNotCrash/*
==========

to

==========
Update layout in WebInputMethodController::finishComposingText

InputMethodController::finishComposingText requires clean layout, but
it may be called with dirty layout from
WebInputMethodController::finishComposingText. This patch fixes the issue
by adding a layout update call there.

BUG=663645
TEST=webkit_unit_tests
--gtest_filter=All/WebViewTest.FinishComposingTextDoesNotCrash/*

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

[commit-bot: I haz the power, 2016-11-14 06:36:18]

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