New issue
Advanced search Search tips

Issue 785344 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

First Contentful Paint triggered on a frame with just iframes

Project Member Reported by npm@chromium.org, Nov 15 2017

Issue description

What steps will reproduce the problem?
(1) Run the test in https://chromium-review.googlesource.com/c/chromium/src/+/769929/3 (the virtual version from virtual/paint-timing/external/wpt/).
(2) Test fails because bufferedEntries.length is 2: there is one first-paint and one fist-contentful-paint entry. 
(3) https://chromium-review.googlesource.com/c/chromium/src/+/769929/1 passes even though it's the same test, except using document.body.appendChild instead of <iframe>

What is the expected result?
Both of the tests should pass (they should behave equivalently).

Using fprintfs, I confirm that in both cases PaintTiming::MarkFirstTextPaint is being called on a main frame. In the former case, SetFirstContentfulPaintSwap is called after MarkFirstTextPaint whereas in the latter case it's called before.
 
Owner: npm@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 27 2017

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

commit fe04b007cb2bd834900c8d37e9292f690b6c7a17
Author: Nicolas Pena <npm@chromium.org>
Date: Mon Nov 27 23:49:43 2017

Add |contains_only_whitespace_| to LayoutText and TextRun

This CL takes advantage of the loop through text in LayoutText to store
a variable indicating whether text contains only 'whitespace' or not.
The main purpose of the loop is to compute widths but this seems like a
reasonable place to also store this new information, to avoid an extra
iteration over the text.

Now, TextRun has a boolean variable indicating whether the text to be
painted contains a whitespace or not. For now, the boolean is only
changed in InlineTextBox::ConstructTextRun to prevent Font::DrawText
from returning true. The return value of this method is only used to
determine whether SetTextPainted() is called in
GraphicsContext::DrawTextInternal.

PaintController::SetTextPainted is the method that triggers First
Contentful Paint (caused by text) on PaintTiming. We do not want it to
be triggered on trivial whitespace text rendering.

Test: Check whether FCP is observed on a document with just
<div>&nbsp</div>

Also, this fix should allow landing the test in:
https://chromium-review.googlesource.com/c/chromium/src/+/769929

Bug:  785344 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Id219eeaced4679eda9c26acde7fd4fdb0cb1e891
Reviewed-on: https://chromium-review.googlesource.com/786336
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519455}
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/layout/LayoutText.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/layout/LayoutText.h
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/layout/LayoutTextTest.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/layout/api/LineLayoutText.h
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/layout/line/InlineTextBox.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/paint/EllipsisBoxPainter.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/paint/ListMarkerPainter.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/core/paint/ng/ng_text_painter.cc
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/platform/fonts/Font.cpp
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/platform/fonts/Font.h
[modify] https://crrev.com/fe04b007cb2bd834900c8d37e9292f690b6c7a17/third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp

Comment 3 by npm@chromium.org, Nov 29 2017

Status: Fixed (was: Assigned)

Sign in to add a comment