'haveImportsLoaded' shouldn't block rendering, if ignoringPendingStylesheets.

'haveImportsLoaded' shouldn't block rendering, if ignoringPendingStylesheets is true.

BUG=629236
Review-Url: https://codereview.chromium.org/2729493002
Cr-Commit-Position: refs/heads/master@{#454826}
Committed: https://chromium.googlesource.com/chromium/src/+/fd74982786ba429438c10f2804e0a3cd16fb8bbf

[cathiechentx, 2017-03-01 05:05:50]

Description was changed from

==========
'haveImportsLoaded' shouldn't block rendering, if ignoringPendingStylesheets.

BUG=629236
==========

to

==========
'haveImportsLoaded' shouldn't block rendering, if
ignoringPendingStylesheets==true.

BUG=629236
==========

[cathiechentx, 2017-03-01 05:06:24]

Description was changed from

==========
'haveImportsLoaded' shouldn't block rendering, if
ignoringPendingStylesheets==true.

BUG=629236
==========

to

==========
'haveImportsLoaded' shouldn't block rendering, if ignoringPendingStylesheets is
true.

BUG=629236
==========

[cathiechentx, 2017-03-01 05:12:18]

cathiechen@tencent.com changed reviewers:
+ atotic@chromium.org, dgrogan@chromium.org, glebl@chromium.org,
morrita@chromium.org

[cathiechentx, 2017-03-01 05:13:32]

cathiechen@tencent.com changed reviewers:
+ jamesxwei@tencent.com

[cathiechentx, 2017-03-01 05:48:28]

Hey guys,

I looked into issue 629236 recently:)

When come to 'div.getBoundingClientRect()', ignoringPendingStylesheets will be
true. Then it could calcstyle, build layout tree and rendering. But it'll be
blocked by haveImportsLoaded(). 

I think we shouldn't block rendering, if ignoringPendingStylesheets is true.

If I misunderstood about anything, just point it out. Thanks very much!

[atotic, 2017-03-01 20:32:04]

atotic@chromium.org changed reviewers:
- atotic@chromium.org

[dgrogan, 2017-03-01 21:33:12]

We usually select 1-2 people who know this code well to review, not 4. Finding
those 1-2 is not always trivial, you might have to look at git blame to see who
touched your functions the most.

I regrettably don't know anything about loading, so I'm not one of the 1-2.

https://codereview.chromium.org/2729493002/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp
(right):

https://codereview.chromium.org/2729493002/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/tests/DocumentLoadingRenderingTest.cpp:412:
EXPECT_TRUE(rect->width() > 0.f && rect->height() > 0.f);
nit: call EXPECT_GT twice instead of EXPECT_TRUE once

[Gleb Lanbin, 2017-03-01 21:42:04]

glebl@chromium.org changed reviewers:
- glebl@chromium.org

[cathiechentx, 2017-03-02 03:01:31]

On 2017/03/01 21:33:12, dgrogan wrote:
> We usually select 1-2 people who know this code well to review, not 4. Finding
> those 1-2 is not always trivial, you might have to look at git blame to see
who
> touched your functions the most.
> 
> I regrettably don't know anything about loading, so I'm not one of the 1-2.
> 

Ah... Got it! I'm really sorry about that! I'll find someone more relevant.

[cathiechentx, 2017-03-02 03:16:14]

cathiechen@tencent.com changed reviewers:
+ esprehn@chromium.org
- dgrogan@chromium.org

[cathiechentx, 2017-03-02 07:02:16]

Hi esprehn and Morrita,
Do you have time to look at this?

When import is added dynamically by javascript, it won't block javascript, but
block render.
If getBoundingClientRect() by javascript when import blocking render,
getBoundingClientRect will return an empty rect.

getBoundingClientRect will set ignoringPendingStylesheets true, which won't work
at import.
Is it resonable if ignoringPendingStylesheets is true, import shouldn't block
render?

Thanks:)

[esprehn, 2017-03-03 21:36:24]

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

[commit-bot: I haz the power, 2017-03-03 21:36:36]

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

[esprehn, 2017-03-03 21:37:28]

FYI: morrita@ doesn't work on Chrome anymore.

lgtm, this change looks right to me.

[esprehn, 2017-03-03 21:37:36]

On 2017/03/03 at 21:37:28, esprehn wrote:
> FYI: morrita@ doesn't work on Chrome anymore.
> 
> lgtm, this change looks right to me.

btw thanks for the great test!

[commit-bot: I haz the power, 2017-03-03 23:09:31]

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

[commit-bot: I haz the power, 2017-03-03 23:09:31]

Dry run: This issue passed the CQ dry run.

[cathiechentx, 2017-03-06 02:45:08]

On 2017/03/03 21:37:36, esprehn wrote:
> On 2017/03/03 at 21:37:28, esprehn wrote:
> > FYI: morrita@ doesn't work on Chrome anymore.
> > 
> > lgtm, this change looks right to me.
> 
> btw thanks for the great test!

Thanks for reviewing:)

[cathiechentx, 2017-03-06 02:45:15]

The CQ bit was checked by cathiechen@tencent.com

[commit-bot: I haz the power, 2017-03-06 02:45:30]

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, 2017-03-06 04:43:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488768315245580,
"parent_rev": "9a71b6540c7dce7068c4abe0317e740f504986fc", "commit_rev":
"fd74982786ba429438c10f2804e0a3cd16fb8bbf"}

[commit-bot: I haz the power, 2017-03-06 04:44:14]

Description was changed from

==========
'haveImportsLoaded' shouldn't block rendering, if ignoringPendingStylesheets is
true.

BUG=629236
==========

to

==========
'haveImportsLoaded' shouldn't block rendering, if ignoringPendingStylesheets is
true.

BUG=629236

Review-Url: https://codereview.chromium.org/2729493002
Cr-Commit-Position: refs/heads/master@{#454826}
Committed:
https://chromium.googlesource.com/chromium/src/+/fd74982786ba429438c10f2804e0...
==========

[commit-bot: I haz the power, 2017-03-06 04:44:16]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/fd74982786ba429438c10f2804e0...