Audit the use of updateStyleAndLayoutIgnorePendingStylesheets in SearchBuffer.

Audit the use of updateStyleAndLayoutIgnorePendingStylesheets in SearchBuffer.

BUG=590369
R=yosin,esprehn

Committed: https://crrev.com/40fa5a0e328cf9b03798438c6d6e02738d786f4f
Cr-Commit-Position: refs/heads/master@{#407212}

[dglazkov, 2016-07-12 18:58:23]

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

[commit-bot: I haz the power, 2016-07-12 18:59:48]

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-07-12 19:18:40]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 19:18:41]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[dglazkov, 2016-07-12 19:38:43]

Whoops, let's maybe compile first.

[dglazkov, 2016-07-12 19:38:46]

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

[commit-bot: I haz the power, 2016-07-12 19:41:17]

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-07-12 22:08:01]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 22:08:02]

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

[dglazkov, 2016-07-13 13:48:15]

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

[commit-bot: I haz the power, 2016-07-13 13:48:35]

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-07-13 15:00:30]

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

[commit-bot: I haz the power, 2016-07-13 15:00:31]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-07-13 22:40:11]

Still working on this, just comments for myself...

https://codereview.chromium.org/2144673003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2144673003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:1238:
frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
This can go up one more, to EditorCommand and LocalDOMWindow.

https://codereview.chromium.org/2144673003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:1258:
frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
This can go up one more as well, to WebLocalFrameImpl::find. Actually, that
method needs to be re-considered. The division of responsibilities is pretty
arbitrary.

https://codereview.chromium.org/2144673003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/TextFinder.cpp (right):

https://codereview.chromium.org/2144673003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/TextFinder.cpp:305:
searchStart.document()->updateStyleAndLayoutIgnorePendingStylesheets();
Goes up to WebLocalFrameImpl::scopeStringMatches and
to TextFinder::callScopeStringMatches, last one with a stern comment on how we
should not be flushing pipeline on a timeout.

[dglazkov, 2016-07-14 22:18:59]

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

[commit-bot: I haz the power, 2016-07-14 22:20:00]

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-07-14 23:19:33]

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

[commit-bot: I haz the power, 2016-07-14 23:19:34]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dglazkov, 2016-07-15 18:06:38]

Make sure the tests correctly flush the pipeline.

[dglazkov, 2016-07-15 18:06:39]

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

[commit-bot: I haz the power, 2016-07-15 18:07:32]

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-07-15 19:33:45]

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

[commit-bot: I haz the power, 2016-07-15 19:33:46]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-07-15 19:40:27]

PTAL

[esprehn, 2016-07-22 05:57:10]

lgtm, I suspect most callers here just want updateLayout(), I can't imagine
editing needs us to update the paint invalidation and display list unless we're
doing a hit test.

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

https://codereview.chromium.org/2144673003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1965: if (!frame() ||
!frame()->page())
I wonder how you get a frame without a page? Is this just checking
isLocalFrame() ?

https://codereview.chromium.org/2144673003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/TextFinderTest.cpp (right):

https://codereview.chromium.org/2144673003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/TextFinderTest.cpp:367:
document().view()->updateAllLifecyclePhases();
I bet almost all of these only need updateLayout() and don't need the paint
invalidation and recording logic.

[dglazkov, 2016-07-22 16:40:50]

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

https://codereview.chromium.org/2144673003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1965: if (!frame() ||
!frame()->page())
On 2016/07/22 at 05:57:10, esprehn wrote:
> I wonder how you get a frame without a page? Is this just checking
isLocalFrame() ?

I think that check it bogus. The only time you have Frame::page return nullptr
is after the frame is detached. I hope we don't do something silly like trying
to search a detached iframe. Let me just replace it with a DCHECK.

https://codereview.chromium.org/2144673003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/TextFinderTest.cpp (right):

https://codereview.chromium.org/2144673003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/TextFinderTest.cpp:367:
document().view()->updateAllLifecyclePhases();
On 2016/07/22 at 05:57:10, esprehn wrote:
> I bet almost all of these only need updateLayout() and don't need the paint
invalidation and recording logic.

Yeah, you're right. Let me replace.

[dglazkov, 2016-07-22 16:56:52]

Feedback addressed.

[dglazkov, 2016-07-22 16:56:53]

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

[commit-bot: I haz the power, 2016-07-22 16:57: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-07-22 18:22:14]

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

[commit-bot: I haz the power, 2016-07-22 18:22:14]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-07-22 18:32:11]

The CQ bit was checked by dglazkov@chromium.org

[dglazkov, 2016-07-22 18:32:11]

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/2144673003/#ps80001
(title: "Feedback addressed.")

[commit-bot: I haz the power, 2016-07-22 18:32:39]

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-07-22 18:37:48]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-22 18:39:31]

Description was changed from

==========
Audit the use of updateStyleAndLayoutIgnorePendingStylesheets in SearchBuffer.

BUG=590369
R=yosin,esprehn
==========

to

==========
Audit the use of updateStyleAndLayoutIgnorePendingStylesheets in SearchBuffer.

BUG=590369
R=yosin,esprehn

Committed: https://crrev.com/40fa5a0e328cf9b03798438c6d6e02738d786f4f
Cr-Commit-Position: refs/heads/master@{#407212}
==========

[commit-bot: I haz the power, 2016-07-22 18:39:32]

Patchset 5 (id:??) landed as
https://crrev.com/40fa5a0e328cf9b03798438c6d6e02738d786f4f
Cr-Commit-Position: refs/heads/master@{#407212}