Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Robert O'Callahan]

On Tue, Dec 1, 2015 at 4:26 AM, Ehsan Akhgari 
wrote:

Should we finally bite the bullet and force a flush in reftests/crashtests?


You mean force a flush in the load event handler in the test harness,
before the test's load event handler runs?


> After thinking about this for a while, I haven't been able to come up with
> something specific that we'd risk breaking on the Web without our
> reftests/crashtests catching it...


How about a bug where we're missing a necessary FlushPendingNotifications,
and someone writes a reftest/crashtest load event handler that would
trigger the bug if not for the harness flush? That might not be worth
worrying about though.

I think we should just grind through reftest/crashtests and add an explicit
flush to every onload/load event handler. Mind numbing-work but would take
at most a week based on my count.

Rob
-- 
lbir ye,ea yer.tnietoehr  rdn rdsme,anea lurpr  edna e hnysnenh hhe uresyf
toD
selthor  stor  edna  siewaoeodm  or v sstvr  esBa  kbvted,t
rdsme,aoreseoouoto
o l euetiuruewFa  kbn e hnystoivateweh uresyf tulsa rehr  rdm  or rnea
lurpr
.a war hsrer holsa rodvted,t  nenh hneireseoouot.tniesiewaoeivatewt sstvr
esn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Ehsan Akhgari]

On 2015-11-28 1:00 AM, Boris Zbarsky wrote:

On 11/27/15 2:15 AM, Axel Hecht wrote:

I wonder, how much of the web could rely on this, given our tests do?


Our test failures were mostly along the lines of "we expected an
assertion here and now we don't get one".  I doubt that would much
affect the web.


Should we finally bite the bullet and force a flush in 
reftests/crashtests?  After thinking about this for a while, I haven't 
been able to come up with something specific that we'd risk breaking on 
the Web without our reftests/crashtests catching it...


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Boris Zbarsky]

On 11/30/15 4:09 PM, Robert O'Callahan wrote:

I think we should just grind through reftest/crashtests and add an explicit
flush to every onload/load event handler.


There may be mochitests affected too.  At least the ones using window 
screenshots, but possibly some other ones that aim to not trigger fatal 
assertions...


Maybe there are few enough that we can just look at the ones doing 
screenshotting and ignore the others.


-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Vivien Nicolas]

Thanks for the information.

On Sat, Nov 28, 2015 at 7:02 AM, Boris Zbarsky  wrote:

> On 11/27/15 5:33 AM, Vivien Nicolas wrote:
>
>> But don't we need at least a Flush_Style to make sure the CSS that tries
>> to
>> use background-image will start triggering images decoding and block the
>> onload event ?
>>
>
> Yes, we do.  That's handled in nsDocLoader::DocLoaderIsEmpty.
>
> -Borsi
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Boris Zbarsky]

On 11/27/15 3:16 AM, L. David Baron wrote:

(If that is a problem, we could still flush style instead of
layout.)


We always flush style before onload, because otherwise we wouldn't block 
onload on things like background images specified in CSS.


-Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Boris Zbarsky]

On 11/27/15 2:15 AM, Axel Hecht wrote:

I wonder, how much of the web could rely on this, given our tests do?


Our test failures were mostly along the lines of "we expected an 
assertion here and now we don't get one".  I doubt that would much 
affect the web.


-Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Boris Zbarsky]

On 11/27/15 5:33 AM, Vivien Nicolas wrote:

But don't we need at least a Flush_Style to make sure the CSS that tries to
use background-image will start triggering images decoding and block the
onload event ?


Yes, we do.  That's handled in nsDocLoader::DocLoaderIsEmpty.

-Borsi
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[L. David Baron]

On Friday 2015-11-27 08:15 +0100, Axel Hecht wrote:
> I wonder, how much of the web could rely on this, given our tests do?

What about starting of CSS transitions?  Could pages be relying on
making a style change in onload starting a transition?  Or do other
browsers not guarantee that?

(If that is a problem, we could still flush style instead of
layout.)

-David

-- 
턞   L. David Baron http://dbaron.org/   턂
턢   Mozilla  https://www.mozilla.org/   턂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Vivien Nicolas]

It may be a stupid question as my understanding of this part of the code is
flaky.

But don't we need at least a Flush_Style to make sure the CSS that tries to
use background-image will start triggering images decoding and block the
onload event ?

I just want to make sure it won't create flickering effects when app author
tries to reveal their page content after the onload event.


On Fri, Nov 27, 2015 at 8:29 AM, Xidorn Quan  wrote:

> On Fri, Nov 27, 2015 at 6:15 PM, Axel Hecht  wrote:
> > On 11/27/15 4:09 AM, Robert O'Callahan wrote:
> >>
> >> On Fri, Nov 27, 2015 at 3:59 PM, Boris Zbarsky 
> wrote:
> >>
> >>> On 11/26/15 9:24 PM, Robert O'Callahan wrote:
> >>>
>  We've always done it, but I can't think of any good reasons.
> 
> >>>
> >>> I've tried to fix this in the past and ran into two problems.
> >>>
> >>> The first problem was that some tests failed as a result.  This is
> >>> somewhat minor, really.
> >>>
> >>> The second problem, pointed out by the first, is that some tests
> stopped
> >>> testing what they mean to be testing, because all of our reftests and
> >>> crashtests assume layout gets flushed onload, so they can test dynamic
> >>> behavior by doing stuff after that.
> >>>
> >>> See https://bugzilla.mozilla.org/show_bug.cgi?id=581685 for details.
> I
> >>> haven't had a chance to get back and really figure this out, though we
> >>> should.
> >>
> >>
> >>
> >> Mmmm. This could be a significant win!
> >>
> >> Rob
> >>
> >
> > I wonder, how much of the web could rely on this, given our tests do?
>
> I don't think the web would be affected by this change, because if the
> page do something in onload which requires a layout, the layout would
> always happen no matter whether we do this in advance.
>
> Our tests relying on this is probably because certain bugs are only
> detectable when we apply content/style change after a layout flush.
>
> - Xidorn
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Why do we flush layout in nsDocumentViewer::LoadComplete?

[Robert O'Callahan]

We've always done it, but I can't think of any good reasons. I seem to
recall that one reason was we want onload to be usable to measure
page-load-and-layout time, but that would be a bad reason.

If we didn't do that, we could run onload scripts earlier and maybe do less
layout if they update the page content. Also that layout flush is
non-interruptible; if we didn't do it, we might get more value from our
interruptible layout.

Rob
-- 
lbir ye,ea yer.tnietoehr  rdn rdsme,anea lurpr  edna e hnysnenh hhe uresyf
toD
selthor  stor  edna  siewaoeodm  or v sstvr  esBa  kbvted,t
rdsme,aoreseoouoto
o l euetiuruewFa  kbn e hnystoivateweh uresyf tulsa rehr  rdm  or rnea
lurpr
.a war hsrer holsa rodvted,t  nenh hneireseoouot.tniesiewaoeivatewt sstvr
esn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Boris Zbarsky]

On 11/26/15 9:24 PM, Robert O'Callahan wrote:

We've always done it, but I can't think of any good reasons.


I've tried to fix this in the past and ran into two problems.

The first problem was that some tests failed as a result.  This is 
somewhat minor, really.


The second problem, pointed out by the first, is that some tests stopped 
testing what they mean to be testing, because all of our reftests and 
crashtests assume layout gets flushed onload, so they can test dynamic 
behavior by doing stuff after that.


See https://bugzilla.mozilla.org/show_bug.cgi?id=581685 for details.  I 
haven't had a chance to get back and really figure this out, though we 
should.



I seem to recall that one reason was we want onload to be usable to measure
page-load-and-layout time, but that would be a bad reason.


I agree this is a non-reason.

-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Robert O'Callahan]

On Fri, Nov 27, 2015 at 3:59 PM, Boris Zbarsky  wrote:

> On 11/26/15 9:24 PM, Robert O'Callahan wrote:
>
>> We've always done it, but I can't think of any good reasons.
>>
>
> I've tried to fix this in the past and ran into two problems.
>
> The first problem was that some tests failed as a result.  This is
> somewhat minor, really.
>
> The second problem, pointed out by the first, is that some tests stopped
> testing what they mean to be testing, because all of our reftests and
> crashtests assume layout gets flushed onload, so they can test dynamic
> behavior by doing stuff after that.
>
> See https://bugzilla.mozilla.org/show_bug.cgi?id=581685 for details.  I
> haven't had a chance to get back and really figure this out, though we
> should.


Mmmm. This could be a significant win!

Rob
-- 
lbir ye,ea yer.tnietoehr  rdn rdsme,anea lurpr  edna e hnysnenh hhe uresyf
toD
selthor  stor  edna  siewaoeodm  or v sstvr  esBa  kbvted,t
rdsme,aoreseoouoto
o l euetiuruewFa  kbn e hnystoivateweh uresyf tulsa rehr  rdm  or rnea
lurpr
.a war hsrer holsa rodvted,t  nenh hneireseoouot.tniesiewaoeivatewt sstvr
esn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Xidorn Quan]

On Fri, Nov 27, 2015 at 6:15 PM, Axel Hecht  wrote:
> On 11/27/15 4:09 AM, Robert O'Callahan wrote:
>>
>> On Fri, Nov 27, 2015 at 3:59 PM, Boris Zbarsky  wrote:
>>
>>> On 11/26/15 9:24 PM, Robert O'Callahan wrote:
>>>
 We've always done it, but I can't think of any good reasons.

>>>
>>> I've tried to fix this in the past and ran into two problems.
>>>
>>> The first problem was that some tests failed as a result.  This is
>>> somewhat minor, really.
>>>
>>> The second problem, pointed out by the first, is that some tests stopped
>>> testing what they mean to be testing, because all of our reftests and
>>> crashtests assume layout gets flushed onload, so they can test dynamic
>>> behavior by doing stuff after that.
>>>
>>> See https://bugzilla.mozilla.org/show_bug.cgi?id=581685 for details.  I
>>> haven't had a chance to get back and really figure this out, though we
>>> should.
>>
>>
>>
>> Mmmm. This could be a significant win!
>>
>> Rob
>>
>
> I wonder, how much of the web could rely on this, given our tests do?

I don't think the web would be affected by this change, because if the
page do something in onload which requires a layout, the layout would
always happen no matter whether we do this in advance.

Our tests relying on this is probably because certain bugs are only
detectable when we apply content/style change after a layout flush.

- Xidorn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Axel Hecht]

On 11/27/15 4:09 AM, Robert O'Callahan wrote:

On Fri, Nov 27, 2015 at 3:59 PM, Boris Zbarsky  wrote:


On 11/26/15 9:24 PM, Robert O'Callahan wrote:


We've always done it, but I can't think of any good reasons.



I've tried to fix this in the past and ran into two problems.

The first problem was that some tests failed as a result.  This is
somewhat minor, really.

The second problem, pointed out by the first, is that some tests stopped
testing what they mean to be testing, because all of our reftests and
crashtests assume layout gets flushed onload, so they can test dynamic
behavior by doing stuff after that.

See https://bugzilla.mozilla.org/show_bug.cgi?id=581685 for details.  I
haven't had a chance to get back and really figure this out, though we
should.



Mmmm. This could be a significant win!

Rob



I wonder, how much of the web could rely on this, given our tests do?

Axel
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Why do we flush layout in nsDocumentViewer::LoadComplete?

[Robert O'Callahan]

On Fri, Nov 27, 2015 at 8:29 PM, Xidorn Quan  wrote:

> Our tests relying on this is probably because certain bugs are only
> detectable when we apply content/style change after a layout flush.
>

That's right. This change is likely to hide more bugs than it causes.

Rob
-- 
lbir ye,ea yer.tnietoehr  rdn rdsme,anea lurpr  edna e hnysnenh hhe uresyf
toD
selthor  stor  edna  siewaoeodm  or v sstvr  esBa  kbvted,t
rdsme,aoreseoouoto
o l euetiuruewFa  kbn e hnystoivateweh uresyf tulsa rehr  rdm  or rnea
lurpr
.a war hsrer holsa rodvted,t  nenh hneireseoouot.tniesiewaoeivatewt sstvr
esn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform