Don't bother layout until first navigation is done.

Don't bother layout until first navigation is done.

Before this CL, null layout was triggered before new navigation is started, which delayed main resource request dispatch.

This CL avoids triggering those null layout synchronously against the blank document loaded before navigations.

BUG=540988

[kouhei (in TOK), 2015-10-13 04:36:12]

kouhei@chromium.org changed reviewers:
+ tzik@chromium.org

[kouhei (in TOK), 2015-10-13 04:36:12]

tzik-san: Would you measure the performance impact of this CL? Thanks.

[tzik, 2015-10-13 05:26:16]

On 2015/10/13 04:36:12, kouhei wrote:
> tzik-san: Would you measure the performance impact of this CL? Thanks.

Measured.
Previously, the layout task before first navigate delays following around 15ms,
that include request for the main resource.
And after this CL, the renderer requests the main resource 20ms earlier than
before.
.
before: https://drive.google.com/open?id=0BwpGjpkRI2N-TnRZVm5WRi1rY2c
after: https://drive.google.com/open?id=0BwpGjpkRI2N-b2Fkd3NrcWxROFU

[kouhei (in TOK), 2015-10-13 05:42:59]

kouhei@chromium.org changed reviewers:
+ tkent@chromium.org

[kouhei (in TOK), 2015-10-13 05:43:22]

tzik: Thanks!

tkent: Would you take a look?

[tkent, 2015-10-13 06:09:34]

Can you make a test?

[esprehn, 2015-10-19 06:11:19]

On 2015/10/13 at 06:09:34, tkent wrote:
> Can you make a test?

You should write a Sim* unit test see DocumentLoadingRenderingTest, also should
this be at a lower level in the engine? This seems like it might just be a game
of whackamole.

[kouhei (in TOK), 2015-10-20 04:19:36]

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

[commit-bot: I haz the power, 2015-10-20 04:19:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1397713004/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1397713004/50001

[commit-bot: I haz the power, 2015-10-20 07:07:53]

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

[commit-bot: I haz the power, 2015-10-20 07:07:54]

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

[kouhei (in TOK), 2015-10-26 03:48:43]

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

[commit-bot: I haz the power, 2015-10-26 03:48:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1397713004/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1397713004/70001

[commit-bot: I haz the power, 2015-10-26 06:44:39]

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

[commit-bot: I haz the power, 2015-10-26 06:44:40]

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

[kouhei (in TOK), 2015-10-27 05:31:03]

On 2015/10/26 06:44:40, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

It looks like Chrome for Android code is relying on this early layout for
BrandColor impl.
I spent two days investigating it but had no luck, so suspending this CL for
now.

[kouhei (in TOK), 2015-11-20 06:02:40]

tkent, tzik: Can we revisit this CL? Android issue seems to be now gone.

[tkent, 2015-11-20 06:22:51]

Can we just change the condition of FrameView::shouldThrottleRendering()?

[kouhei (in TOK), 2016-01-20 01:56:13]

resuming work on this CL.

On 2015/11/20 06:22:51, tkent wrote:
> Can we just change the condition of FrameView::shouldThrottleRendering()?

The method is meant for renderingPipelineThrottling feature:
https://code.google.com/p/chromium/issues/detail?id=487937
So I don't think it should be abused.

[tkent, 2016-01-20 04:19:58]

On 2016/01/20 at 01:56:13, kouhei wrote:
> > Can we just change the condition of FrameView::shouldThrottleRendering()?
> 
> The method is meant for renderingPipelineThrottling feature:
https://code.google.com/p/chromium/issues/detail?id=487937
> So I don't think it should be abused.

Can we introduce a function like shouldThrottleRendering() in core/, rather than
checking committedFirstRealDocumentLoad() and skipping layout in multiple
places?

[tkent, 2016-01-20 04:19:58]

On 2016/01/20 at 01:56:13, kouhei wrote:
> > Can we just change the condition of FrameView::shouldThrottleRendering()?
> 
> The method is meant for renderingPipelineThrottling feature:
https://code.google.com/p/chromium/issues/detail?id=487937
> So I don't think it should be abused.

Can we introduce a function like shouldThrottleRendering() in core/, rather than
checking committedFirstRealDocumentLoad() and skipping layout in multiple
places?

[kouhei (in TOK), 2016-01-21 02:00:32]

On 2016/01/20 04:19:58, tkent wrote:
> On 2016/01/20 at 01:56:13, kouhei wrote:
> > > Can we just change the condition of FrameView::shouldThrottleRendering()?
> > 
> > The method is meant for renderingPipelineThrottling feature:
> https://code.google.com/p/chromium/issues/detail?id=487937
> > So I don't think it should be abused.
> 
> Can we introduce a function like shouldThrottleRendering() in core/, rather
than
> checking committedFirstRealDocumentLoad() and skipping layout in multiple
> places?

PTAL. Added Document::isInitialEmptyDocument() per suggestion.

[tkent, 2016-01-21 02:19:32]

https://codereview.chromium.org/1397713004/diff/170001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1397713004/diff/170001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:912: if
(!document->isInitialEmptyDocument())
Can you move isInitialEmptyDocument() check to Document::updateLayoutTree(),
FrameView::layout(), or something deeper place?

[kouhei (in TOK), 2016-02-18 03:49:58]

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

[commit-bot: I haz the power, 2016-02-18 03:50:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1397713004/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1397713004/190001

[kouhei (in TOK), 2016-02-18 03:50:50]

tkent-san: PTAL

https://codereview.chromium.org/1397713004/diff/170001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1397713004/diff/170001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:912: if
(!document->isInitialEmptyDocument())
On 2016/01/21 02:19:32, tkent wrote:
> Can you move isInitialEmptyDocument() check to Document::updateLayoutTree(),
> FrameView::layout(), or something deeper place?
> 

Done.

[commit-bot: I haz the power, 2016-02-18 04:27:54]

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

[commit-bot: I haz the power, 2016-02-18 04:27:55]

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

[tkent, 2016-02-19 00:43:40]

The approach looks good.  Please address test failures.

https://codereview.chromium.org/1397713004/diff/190001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/1397713004/diff/190001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/WebViewImpl.cpp:1862: if
(!view->frame().document()->isInitialEmptyDocument())
Is this necessary?

[esprehn, 2016-02-26 21:23:53]

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

[esprehn, 2016-02-26 21:23:53]

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

[esprehn, 2016-02-26 21:23:54]

This doesn't seem like a good change, what's the actual improvement, your change
doesn't say why this is better? Layout on about:blank is super cheap.

We should just avoid pumping frames for the initial document not hack up the
lifecycle like this.

[esprehn, 2016-02-26 21:23:54]

This doesn't seem like a good change, what's the actual improvement, your change
doesn't say why this is better? Layout on about:blank is super cheap.

We should just avoid pumping frames for the initial document not hack up the
lifecycle like this.

[kouhei (in TOK), 2016-02-29 21:55:41]

On 2016/02/26 21:23:54, esprehn wrote:
> This doesn't seem like a good change, what's the actual improvement, your
change
> doesn't say why this is better? Layout on about:blank is super cheap.
> 
> We should just avoid pumping frames for the initial document not hack up the
> lifecycle like this.

The problem I'm trying to address in this CL is first layout -> StyleEngine init
-> parse UA stylesheet blocking resource fetches.
I'm happy as long as the resource fetches are not blocked by parse UA
stylesheet.

PS4 patches WebView{,Impl} initializations which end up triggering the first
layout on initial empty documents, and
PS17 is more generic version that avoids forced layout on initial empty
documents.
Alternative suggestions are welcomed.

[kouhei (in TOK), 2016-02-29 21:56:54]

On 2016/02/29 21:55:41, kouhei wrote:
> On 2016/02/26 21:23:54, esprehn wrote:
> > This doesn't seem like a good change, what's the actual improvement, your
> change
> > doesn't say why this is better? Layout on about:blank is super cheap.
> > 
> > We should just avoid pumping frames for the initial document not hack up the
> > lifecycle like this.
> 
> The problem I'm trying to address in this CL is first layout -> StyleEngine
init
> -> parse UA stylesheet blocking resource fetches.
> I'm happy as long as the resource fetches are not blocked by parse UA
> stylesheet.
> 
> PS4 patches WebView{,Impl} initializations which end up triggering the first
> layout on initial empty documents, and
> PS17 is more generic version that avoids forced layout on initial empty
> documents.
> Alternative suggestions are welcomed.

Trace of initial layout triggering UA stylesheet parse:
https://googledrive.com/host/0B5o-0Snm1_rFbTlKcjZkdGN3eEk