Re: Allowing web apps to delay layout/rendering on startup
On Sun, Aug 2, 2015 at 2:41 PM, Robert O'Callahan rob...@ocallahan.org wrote: On Mon, Aug 3, 2015 at 8:15 AM, Jonas Sicking jo...@sicking.cc wrote: Often times the chrome of a webapp is ready before various other things that are loaded during initial page load is loaded. OK then, I guess a new event is needed. The need is not for an event, no? But rather for an API which allows the page to tell the browser that it's ready for initial rendering? / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Allowing web apps to delay layout/rendering on startup
On Sun, Aug 2, 2015 at 11:58 AM, Zibi Braniecki zbigniew.branie...@gmail.com wrote: So maybe we would need another event that the website fires to indicate when it for paint/cache/screenshot. Assuming pages have an easy way to block the 'load' event until they're ready, can this just be the 'load' event? 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: Use of 'auto'
A new type of error 'auto' seems to cause, now seen on m-i, is auto foo = new SomeRefCountedFoo(); That hides that foo is a raw pointer but we should be using nsRefPtr. So please, consider again when about to use auto. It usually doesn't make the code easier to read, and it occasionally just leads to errors. In this case it clearly made the code harder to read so that whoever reviewed that patch didn't catch the issue. (So far the only safe case I've seen is using 'auto' on left side when doing *_cast on the right side. And personally I think even in that case auto doesn't help with readability, but I can live with use of auto there.) -Olli On 06/02/2015 10:58 PM, smaug wrote: Hi all, there was some discussion in #developers about use of 'auto' earlier today. Some people seem to like it, and some, like I, don't. The reasons why I try to avoid using it and usually ask to replace it with the actual type when I'm reviewing a patch using it are: - It makes the code harder to read * one needs to explicitly check what kind of type is assigned to the variable to see how the variable is supposed to be used. Very important for example when dealing with refcounted objects, and even more important when dealing with raw pointers. - It makes the code possibly error prone if the type is later changed. * Say, you have a method nsRefPtrFoo Foo(); (I know, silly example, but you get the point) Now auto foo = Foo(); makes sure foo is kept alive. But then someone decides to change the return value to Foo*. Everything still compiles just fine, but use of foo becomes risky and may lead to UAF. Perhaps my mind is too much on reviewer's side, and less on the code writer's. So, I'd like to understand why people think 'auto' is a good thing to use. (bz mentioned it having some use inside bindings' codegenerator, and sure, I can see that being rather valid case.) -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On Sun, Aug 2, 2015 at 2:56 AM, Hubert Figuière h...@mozilla.com wrote: On 02/08/15 04:55 AM, smaug wrote: A new type of error 'auto' seems to cause, now seen on m-i, is auto foo = new SomeRefCountedFoo(); That hides that foo is a raw pointer but we should be using nsRefPtr. So please, consider again when about to use auto. It usually doesn't make the code easier to read, and it occasionally just leads to errors. In this case it clearly made the code harder to read so that whoever reviewed that patch didn't catch the issue. Shouldn't we, instead, ensure that SomeRefCountedFoo() returns a nsRefPtr? How do you do that with a constructor? - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Running gaia unit tests with gdb
(Cross-post to dev-platform and dev-b2g, reply-to dev-b2g) Hi, sometimes we need to track crashes, and sometimes those are crashes happening with unit tests. So I just landed in bug 1177165 a change so that you can to run the test runner in gdb. You can find more information on MDN [1]. Please tell me if you have any suggestion about this :) [1] https://developer.mozilla.org/en-US/Firefox_OS/Automated_testing/Gaia_unit_tests#Launching_the_test_runner_with_gdb -- Julien signature.asc Description: OpenPGP digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Allowing web apps to delay layout/rendering on startup
On Sunday, August 2, 2015 at 2:03:28 AM UTC-7, Robert O'Callahan wrote: On Sun, Aug 2, 2015 at 11:58 AM, Zibi Braniecki wrote: So maybe we would need another event that the website fires to indicate when it for paint/cache/screenshot. Assuming pages have an easy way to block the 'load' event until they're ready, can this just be the 'load' event? you mean window.load or performance.mark fullyLoaded? If the former, I'm not sure how a page can control when the loaded is fired. If the latter, then it's not a good one. fullyLoaded is supposed to be fired when the page finished loading background data needed for operation. zb. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 08/02/2015 01:47 PM, Xidorn Quan wrote: On Sun, Aug 2, 2015 at 7:57 PM, Kyle Huey m...@kylehuey.com wrote: On Sun, Aug 2, 2015 at 2:56 AM, Hubert Figuière h...@mozilla.com wrote: On 02/08/15 04:55 AM, smaug wrote: A new type of error 'auto' seems to cause, now seen on m-i, is auto foo = new SomeRefCountedFoo(); That hides that foo is a raw pointer but we should be using nsRefPtr. So please, consider again when about to use auto. It usually doesn't make the code easier to read, and it occasionally just leads to errors. In this case it clearly made the code harder to read so that whoever reviewed that patch didn't catch the issue. Shouldn't we, instead, ensure that SomeRefCountedFoo() returns a nsRefPtr? How do you do that with a constructor? Probably we should generally avoid using constructor directly for those cases. Instead, use helper functions like MakeUnique() or MakeAndAddRef(), which is much safer. MakeAndAddRef would have the same problem as MakeUnique. Doesn't really tell what type is returned. And when you're dealing with lifetime management issues, you really want to know what kind of type you're playing with. I would just limit use of 'auto' to those cases which are safe for certain, given that it helps with readability in rather rare cases (this is of course very subjective). So, *_cast and certain forms of iteration, but only in cases when iteration is known to not call any may-change-the-view-of-the-world methods - so no calling to JS, or flushing layout or dispatching dom events or... -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 02/08/15 07:17 AM, smaug wrote: Probably we should generally avoid using constructor directly for those cases. Instead, use helper functions like MakeUnique() or MakeAndAddRef(), which is much safer. MakeAndAddRef would have the same problem as MakeUnique. Doesn't really tell what type is returned. makeSomeRefCountedFoo(), newSomeRefCountedFoo() or SomeRefCountedFoo::make() returning an nsRefPtrSomeRefCountedFoo. It is a matter of having an enforced convention for naming them. And when you're dealing with lifetime management issues, you really want to know what kind of type you're playing with. This is also part of why I'd suggest having an construction method that will return a smart pointer - preventing the use of raw pointers. So that there is no ambiguity in what we deal with and its ownership. This is probably not something trivial in our codebase. Hub ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 02/08/15 04:55 AM, smaug wrote: A new type of error 'auto' seems to cause, now seen on m-i, is auto foo = new SomeRefCountedFoo(); That hides that foo is a raw pointer but we should be using nsRefPtr. So please, consider again when about to use auto. It usually doesn't make the code easier to read, and it occasionally just leads to errors. In this case it clearly made the code harder to read so that whoever reviewed that patch didn't catch the issue. Shouldn't we, instead, ensure that SomeRefCountedFoo() returns a nsRefPtr? Hub ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 08/02/2015 02:34 PM, Hubert Figuière wrote: On 02/08/15 07:17 AM, smaug wrote: Probably we should generally avoid using constructor directly for those cases. Instead, use helper functions like MakeUnique() or MakeAndAddRef(), which is much safer. MakeAndAddRef would have the same problem as MakeUnique. Doesn't really tell what type is returned. makeSomeRefCountedFoo(), newSomeRefCountedFoo() or SomeRefCountedFoo::make() returning an nsRefPtrSomeRefCountedFoo. It is a matter of having an enforced convention for naming them. And when you're dealing with lifetime management issues, you really want to know what kind of type you're playing with. This is also part of why I'd suggest having an construction method that will return a smart pointer - preventing the use of raw pointers. So that there is no ambiguity in what we deal with and its ownership. Sure, static already_AddRefedClassFoo ClassFoo::Create() would make sense. (or returning nsRefPtrClassFoo ?) But that has nothing to do with auto. One should still see in the calling code what the type is in order to verify lifetime management is ok. This is probably not something trivial in our codebase. Hub ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 8/2/2015 10:21 AM, Boris Zbarsky wrote: On 8/2/15 7:34 AM, Hubert Figuière wrote: This is also part of why I'd suggest having an construction method that will return a smart pointer - preventing the use of raw pointers. Returning an already_AddRefed would prevent the use of raw pointers, but would leak if the caller used auto, right? Returning an nsRefPtr would not prevent the use of raw pointers, allowing a caller to write: I've discussed this several times, but with we added operator T*() = delete;, that would prevent the scenario you're talking about here. -- Joshua Cranmer Thunderbird and DXR developer Source code archæologist ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On Sun, Aug 2, 2015 at 7:57 PM, Kyle Huey m...@kylehuey.com wrote: On Sun, Aug 2, 2015 at 2:56 AM, Hubert Figuière h...@mozilla.com wrote: On 02/08/15 04:55 AM, smaug wrote: A new type of error 'auto' seems to cause, now seen on m-i, is auto foo = new SomeRefCountedFoo(); That hides that foo is a raw pointer but we should be using nsRefPtr. So please, consider again when about to use auto. It usually doesn't make the code easier to read, and it occasionally just leads to errors. In this case it clearly made the code harder to read so that whoever reviewed that patch didn't catch the issue. Shouldn't we, instead, ensure that SomeRefCountedFoo() returns a nsRefPtr? How do you do that with a constructor? Probably we should generally avoid using constructor directly for those cases. Instead, use helper functions like MakeUnique() or MakeAndAddRef(), which is much safer. - Xidorn ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 02/08/15 05:57 AM, Kyle Huey wrote: On Sun, Aug 2, 2015 at 2:56 AM, Hubert Figuière h...@mozilla.com wrote: On 02/08/15 04:55 AM, smaug wrote: A new type of error 'auto' seems to cause, now seen on m-i, is auto foo = new SomeRefCountedFoo(); That hides that foo is a raw pointer but we should be using nsRefPtr. So please, consider again when about to use auto. It usually doesn't make the code easier to read, and it occasionally just leads to errors. In this case it clearly made the code harder to read so that whoever reviewed that patch didn't catch the issue. Shouldn't we, instead, ensure that SomeRefCountedFoo() returns a nsRefPtr? How do you do that with a constructor? Uh oh, my bad. As suggested by Xidorn, having a construction method (class static) and making the constructor protected, is the right way to do it. Hub ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Use of 'auto'
On 8/2/15 7:34 AM, Hubert Figuière wrote: This is also part of why I'd suggest having an construction method that will return a smart pointer - preventing the use of raw pointers. Returning an already_AddRefed would prevent the use of raw pointers, but would leak if the caller used auto, right? Returning an nsRefPtr would not prevent the use of raw pointers, allowing a caller to write: Foo* foo = Foo:make(); // now we have a dangling pointer We could avoid the former by not using auto in situations like this. We could avoid the latter by _always_ using auto in situations like this and never using a raw pointer... We're currently much closer to the never use auto in this situation world than to the other, of course. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Allowing web apps to delay layout/rendering on startup
On Sun, Aug 2, 2015 at 2:03 AM, Robert O'Callahan rob...@ocallahan.org wrote: On Sun, Aug 2, 2015 at 11:58 AM, Zibi Braniecki zbigniew.branie...@gmail.com wrote: So maybe we would need another event that the website fires to indicate when it for paint/cache/screenshot. Assuming pages have an easy way to block the 'load' event until they're ready, can this just be the 'load' event? I think that's too late in many cases. Often times the chrome of a webapp is ready before various other things that are loaded during initial page load is loaded. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Allowing web apps to delay layout/rendering on startup
Are we expecting that this will reduce unwanted layout/paint cycles on the critical path and thus minimise the 'white flash of doom'? If not what are the options for killing this? If the user has to see anything before the new 'please-draw-me' event, we'd probably like to see the app's background painted with the primary theme color, or some kind of splash-screen image. Anything but the signature 'I'm a clunky old website' white flash :) *W I L S O N P A G E* Front-end Developer Firefox OS (Gaia) London Office Twitter: @wilsonpage IRC: wilsonpage On Mon, Aug 3, 2015 at 6:28 AM, Robert O'Callahan rob...@ocallahan.org wrote: On Mon, Aug 3, 2015 at 4:53 PM, Jonas Sicking jo...@sicking.cc wrote: The need is not for an event, no? But rather for an API which allows the page to tell the browser that it's ready for initial rendering? Right. Which gets turned into an event that's sent to the browser. 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 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Allowing web apps to delay layout/rendering on startup
On Mon, Aug 3, 2015 at 4:53 PM, Jonas Sicking jo...@sicking.cc wrote: The need is not for an event, no? But rather for an API which allows the page to tell the browser that it's ready for initial rendering? Right. Which gets turned into an event that's sent to the browser. 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
