Re: The War on Warnings
On Thu, Jun 4, 2015 at 1:48 PM, Luke Wagner lwag...@mozilla.com wrote: In addition to judging noisiness by volume over a whole test run, can we also include any warning that happens on normal browser startup, new tab, and other vanilla browser operations? This has always annoyed me. Indeed. Here's an attempt at establishing common ground that hopefully almost everyone can agree with... - There should be no warnings when starting and immediately closing desktop Firefox on a new profile. Or maybe a very small number (e.g. less than 5) on some configurations... certainly not dozens. - There should be no warnings that trigger 1000s of times while running standard test suites. Because this is just silly: 65959 [N] WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 83 Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: wiki page for platform-specific defines
On Fri, Jun 5, 2015 at 1:22 AM, kgu...@mozilla.com wrote: On Wednesday, June 3, 2015 at 9:16:46 PM UTC-4, Xidorn Quan wrote: I guess it is probably better to add different color on true and false, which should improve the readability. Or probably just remove all false? Looks like Mike and Adam cleaned it to be much more readable, thanks! I also added a second table for prefs files (although I'm not sure where b2g.js is used so I left in some question marks in that one). I didn't even know there are other pref files... Thanks for that. I saw some prefs which we probably should move out from all.js. - Xidorn ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 6/4/15 6:14 PM, Ehsan Akhgari wrote: They are both equally slow, but fatal assertions happen only once, by definition. ;-) Except that for some of our tests we restart after a crash (e.g. for web platform tests). So in that test harness, fatal assertions that are hit are much slower than non-fatal ones, fwiw. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Extra commit metadata on hg.mozilla.org
hg.mozilla.org now displays extra metadata on changeset pages. e.g. https://hg.mozilla.org/mozilla-central/rev/dc4023d54436. Read more at http://gregoryszorc.com/blog/2015/06/04/changeset-metadata-on-hg.mozilla.org/ If you notice anything wonky, including performance issues, please speak up. I'm sure there are corner cases where the parsers aren't picking up everything they should. Please link new bugs to bug 1171321. Changes to hg.mozilla.org's styling are within our control to make (unlike say 3rd party hosted services like GitHub) and are generally low hanging fruit to implement. If there is something you want the site to do to make your life easier, please ask. gps ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Per-test chaos mode now available, use it to help win the war on orange!
On 6/4/15 11:32 AM, kgu...@mozilla.com wrote: I just landed bug 1164218 on inbound, which adds the ability to run individual mochitests and reftests in chaos mode. (For those unfamiliar with chaos mode, it's a feature added by roc a while back that makes already-random things more random; see [1] or bug 955888 for details). The idea with making it available per-test is that new tests should be written and tested locally/on try with chaos mode enabled, to flush out possible intermittent failures faster. Ideally we should also land them with chaos mode enabled. At this time we're still not certain if this will provide a lot of value (i.e. if chaos-mode-triggered failures are representative of real bugs) so it's not mandatory to make your tests run in chaos mode, but please do let me know if you try enabling it on your test and are either successful or not. We need to collect more data on the usefulness of this to see where we should take it. If it does turn out to be valuable, my hope is that we can start making pre-existing tests chaos-mode enabled as well, and eventually reduce the intermittent failure rate. Will chaos mode enabled tests run on Try and release branches? We don't know if chaos mode test failures are representative of real bugs, but could chaos mode hide bugs that only reveal themselves when users run without chaos mode? See [2] for an example of how to enable chaos mode in your tests. Basically you can add chaos-mode to the reftest.list file for reftests, or call SimpleTest.testInChaosMode() for mochitests. If you do run into intermittent failures, the best way to debug them is usually to grab a recording of the failure using rr [3] and then debug the recording to see what was going on. This only works on Linux (and has some hardware requirements as well) but it's a really great tool to have. Cheers, kats [1] http://robert.ocallahan.org/2014/03/introducing-chaos-mode.html [2] https://hg.mozilla.org/integration/mozilla-inbound/rev/89ac61464a45 [3] http://rr-project.org/ or https://github.com/mozilla/rr/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 06/04/2015 01:18 PM, smaug wrote: More likely we need to change a small number of noisy NS_ENSURE_* macro users to use something else, and keep most of the NS_ENSURE_* usage as it is. I agree -- I posted about switching to something opt-in, like MOZ_LOG, for some of the spammier layout NS_WARNINGS, too: https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.tech.layout/YXauN50HDhI ~Daniel ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Thursday, June 4, 2015 at 11:14:59 AM UTC-7, Martin Thomson wrote: On Jun 4, 2015 10:27 AM, Daniel Holbert dholb...@mozilla.com wrote: Also: in layout, there are various warnings related to coordinate wraparound/overflow, where we're basically throwing up our hands and warning that broken layout is likely to occur because the page is millions of pixels tall. These can be useful hints, when debugging page brokenness. I know that it's more work, but isn't that what the console is best suited for? We have some spammy warnings there too, of course (try looking for rc4 warnings if you have gmail open). This does seem more appropriate for warnings targeted at web developers. Most end users probably will not be using debug builds of Firefox (particularly considering we market a dev edition, you know, for web devs). ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 06/05/2015 12:06 AM, Daniel Holbert wrote: On 06/04/2015 01:18 PM, smaug wrote: More likely we need to change a small number of noisy NS_ENSURE_* macro users to use something else, and keep most of the NS_ENSURE_* usage as it is. I agree -- I posted about switching to something opt-in, like MOZ_LOG, for some of the spammier layout NS_WARNINGS, too: https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.tech.layout/YXauN50HDhI ~Daniel There is also DEBUG_foo and then using it --with-debug-label=foo There are couple of #ifdef DEBUG_smaug checks, but it isn't really nice to pollute the code with developer specific ifdefs. However for your case DEBUG_layout might work. -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Per-test chaos mode now available, use it to help win the war on orange!
Very interesting, thank you! Would there be a way to add an environment variable or harness flag to run all tests in chaos mode? On Thu, Jun 4, 2015 at 5:31 PM, Chris Peterson cpeter...@mozilla.com wrote: On 6/4/15 11:32 AM, kgu...@mozilla.com wrote: I just landed bug 1164218 on inbound, which adds the ability to run individual mochitests and reftests in chaos mode. (For those unfamiliar with chaos mode, it's a feature added by roc a while back that makes already-random things more random; see [1] or bug 955888 for details). The idea with making it available per-test is that new tests should be written and tested locally/on try with chaos mode enabled, to flush out possible intermittent failures faster. Ideally we should also land them with chaos mode enabled. At this time we're still not certain if this will provide a lot of value (i.e. if chaos-mode-triggered failures are representative of real bugs) so it's not mandatory to make your tests run in chaos mode, but please do let me know if you try enabling it on your test and are either successful or not. We need to collect more data on the usefulness of this to see where we should take it. If it does turn out to be valuable, my hope is that we can start making pre-existing tests chaos-mode enabled as well, and eventually reduce the intermittent failure rate. Will chaos mode enabled tests run on Try and release branches? We don't know if chaos mode test failures are representative of real bugs, but could chaos mode hide bugs that only reveal themselves when users run without chaos mode? See [2] for an example of how to enable chaos mode in your tests. Basically you can add chaos-mode to the reftest.list file for reftests, or call SimpleTest.testInChaosMode() for mochitests. If you do run into intermittent failures, the best way to debug them is usually to grab a recording of the failure using rr [3] and then debug the recording to see what was going on. This only works on Linux (and has some hardware requirements as well) but it's a really great tool to have. Cheers, kats [1] http://robert.ocallahan.org/2014/03/introducing-chaos-mode.html [2] https://hg.mozilla.org/integration/mozilla-inbound/rev/89ac61464a45 [3] http://rr-project.org/ or https://github.com/mozilla/rr/ ___ 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: The War on Warnings
On 2015-06-04 9:40 AM, David Rajchenbach-Teller wrote: On 04/06/15 14:30, Ehsan Akhgari wrote: There are very good reasons for warnings to not cause tests to fail. We have a lot of tests that are testing failure conditions that are expected to warn, because they are failure conditions. Well, that's why the patch linked above offers a whitelist API, so that tests can individually decide which families of warnings are either expected, or ok-for-the-time-being. Families of warnings? I don't really understand what you mean. Also, looking at the patch in the bug, your changes seem to affect Assert.jsm stuff, which is not related to NS_WARNING. There are also warnings that were actually meant to be assertions, and we should try to convert them to assertions, and that way they will fail tests. That being said, assertion report stack traces in debug builds, which is excruciatingly slow, so we have had to switch them to be warnings in at least some cases (see bug 756045 for example.) Are you talking about our weird non-fatal assertions or actual fatal assertions? They are both equally slow, but fatal assertions happen only once, by definition. ;-) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
In addition to judging noisiness by volume over a whole test run, can we also include any warning that happens on normal browser startup, new tab, and other vanilla browser operations? This has always annoyed me. On Thu, Jun 4, 2015 at 3:33 PM, Bobby Holley bobbyhol...@gmail.com wrote: On Thu, Jun 4, 2015 at 1:18 PM, smaug sm...@welho.com wrote: More likely we need to change a small number of noisy NS_ENSURE_* macro users to use something else, and keep most of the NS_ENSURE_* usage as it is. +1. ___ 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: what is new in talos, what is coming up
William Lachance writes: Hi Karl, On 2015-06-04 12:30 AM, Karl Tomlinson wrote: jma...@mozilla.com writes: We will deprecate those instances of compare-talos next quarter completely. The treeherder version seems to randomly choose which and how many of the results to load and so the comparison changes after reloads of the page. So this is a bug. :) Could you please file something here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Tree%20Managementcomponent=Perfherder For best results, include reproduction steps and comparison with the existing snarkfest interface. Thanks for the link. Knowing the right component is half the process of filing bugs. https://bugzilla.mozilla.org/show_bug.cgi?id=1171707 I don't know how to compare with snarkfest, but comparing treeherder with treeherder is sufficient to observe the bug. can we keep the snarkfest version running please until this is resolved? My main concern was because I inferred from the previous post that deprecation of snarkfest was scheduled on a timeline basis. Can we instead schedule on a when-its-ready basis, please? If I may sneak in a request or two, then a number of results or an estimate of standard error in the mean would be helpful to know how to interpret the standard deviations. Also, a way to see whether higher or lower is better even for those tests without enough data to detect a statistically significant change would be a bonus. https://bugzilla.mozilla.org/show_bug.cgi?id=1171694 https://bugzilla.mozilla.org/show_bug.cgi?id=1171703 I'm keen to find out what is in the Details links, but they currently just ask me to wait a minute. Likewise, that sounds like a bug. Clicking on details is supposed to open up a subtest summary. Here it seems to work fine for me: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-centraloriginalRevision=b2ad2f9f8e53newProject=mozilla-centralnewRevision=f21fac50a8cc That link is loading for me, and that info can be very helpful, thanks. However, others links are not. https://bugzilla.mozilla.org/show_bug.cgi?id=1171710 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Thursday, June 4, 2015 at 1:48:30 PM UTC-7, Luke Wagner wrote: In addition to judging noisiness by volume over a whole test run, can we also include any warning that happens on normal browser startup, new tab, and other vanilla browser operations? This has always annoyed me. Yes, this bothers me as well. Here's a rough look at a run I just did (this is simply opening the browser to a blank page and then closing it): 27 [N] WARNING: '!mMainThread', file /home/erahm/dev/mozilla-central/xpcom/threads/nsThreadManager.cpp, line 299 10 [N] WARNING: 'NS_FAILED(DebuggerOnGCRunnable::Enqueue(aRuntime, aDesc))', file /home/erahm/dev/mozilla-central/xpcom/base/CycleCollectedJSRuntime.cpp, line 743 5 [N] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /home/erahm/dev/mozilla-central/xpcom/base/nsTraceRefcnt.cpp, line 148 4 [N] WARNING: RemoveObserver() called for unregistered observer: file /home/erahm/dev/mozilla-central/hal/Hal.cpp, line 205 3 [N] WARNING: NS_ENSURE_TRUE(mDocShell) failed: file /home/erahm/dev/mozilla-central/embedding/browser/nsWebBrowser.cpp, line 363 3 [N] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111: file /home/erahm/dev/mozilla-central/docshell/base/nsDocShell.cpp, line 4592 2 [N] WARNING: Subdocument container has no frame: file /home/erahm/dev/mozilla-central/layout/base/nsDocumentViewer.cpp, line 2506 2 [N] WARNING: Re-registering a CID?: file /home/erahm/dev/mozilla-central/xpcom/components/nsComponentManager.cpp, line 551 2 [N] WARNING: 'NS_FAILED(rv)', file /home/erahm/dev/mozilla-central/dom/workers/ServiceWorkerManager.cpp, line 2529 2 [N] WARNING: NS_ENSURE_TRUE(domWindow) failed: file /home/erahm/dev/mozilla-central/embedding/browser/nsDocShellTreeOwner.cpp, line 83 2 [N] WARNING: NS_ENSURE_TRUE(aInBrowser) failed: file /home/erahm/dev/mozilla-central/embedding/browser/nsDocShellTreeOwner.cpp, line 79 2 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /home/erahm/dev/mozilla-central/dom/base/nsFrameLoader.cpp, line 267 2 [N] WARNING: Enabling vsync refresh driver: file /home/erahm/dev/mozilla-central/layout/base/nsRefreshDriver.cpp, line 859 2 [N] WARNING: '!BasePrincipal::IsCodebasePrincipal(aPrincipal)', file /home/erahm/dev/mozilla-central/dom/workers/ServiceWorkerManager.cpp, line 2591 1 [N] WARNING: nsWindow::GetNativeData not implemented for this type: file /home/erahm/dev/mozilla-central/widget/PuppetWidget.cpp, line 1162 1 [N] WARNING: NS_ENSURE_TRUE(domDoc target) failed: file /home/erahm/dev/mozilla-central/dom/base/nsContentUtils.cpp, line 3636 1 [N] WARNING: NS_ENSURE_TRUE(currentInner) failed: file /home/erahm/dev/mozilla-central/dom/base/nsGlobalWindow.cpp, line 9004 1 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /home/erahm/dev/mozilla-central/dom/base/nsContentUtils.cpp, line 3691 1 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /home/erahm/dev/mozilla-central/toolkit/xre/nsXREDirProvider.cpp, line 1374 1 [N] WARNING: No docshells for remote frames!: file /home/erahm/dev/mozilla-central/dom/base/nsFrameLoader.cpp, line 491 1 [N] WARNING: Hardware Vsync support not yet implemented. Falling back to software timers: file /home/erahm/dev/mozilla-central/gfx/thebes/gfxPlatform.cpp, line 2410 1 [N] WARNING: Enabling vsync compositor: file /home/erahm/dev/mozilla-central/gfx/layers/ipc/CompositorParent.cpp, line 679 1 [N] WARNING: '!editorRectEvent.mSucceeded', file /home/erahm/dev/mozilla-central/widget/PuppetWidget.cpp, line 800 1 [N] WARNING: Could not get disk status from nsIDiskSpaceWatcher: file /home/erahm/dev/mozilla-central/uriloader/prefetch/nsOfflineCacheUpdateService.cpp, line 317 1 [N] WARNING: Could not get disk information from DiskSpaceWatcher: file /home/erahm/dev/mozilla-central/dom/storage/DOMStorageIPC.cpp, line 320 1 [N] WARNING: '!compMgr', file /home/erahm/dev/mozilla-central/xpcom/glue/nsComponentManagerUtils.cpp, line 63 There's bug 1171716 for the !mMainThread warning. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Extra commit metadata on hg.mozilla.org
Gregory Szorc writes: hg.mozilla.org now displays extra metadata on changeset pages. e.g. https://hg.mozilla.org/mozilla-central/rev/dc4023d54436. Read more at http://gregoryszorc.com/blog/2015/06/04/changeset-metadata-on-hg.mozilla.org/ Thank you, Gregory. I'm sure that will be *very* useful. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: what is new in talos, what is coming up
Hi Karl, On 2015-06-04 12:30 AM, Karl Tomlinson wrote: jma...@mozilla.com writes: 2) compare-talos is in perfherder (https://treeherder.mozilla.org/perf.html#/comparechooser), other instances of compare-talos have a warning message at the top indicating you should use perfherder. We will deprecate those instances of compare-talos next quarter completely. This is very helpful to present PGO separated and to know that higher is better for canvasmark, but it is not yet ready to replacehttp://perf.snarkfest.net/compare-talos/index.html The treeherder version seems to randomly choose which and how many of the results to load and so the comparison changes after reloads of the page. So this is a bug. :) Could you please file something here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Tree%20Managementcomponent=Perfherder For best results, include reproduction steps and comparison with the existing snarkfest interface. upcoming work: 2) continue polishing perfherder graphs, compare-view Perhaps the above issue is already in this work and you know it will be addressed by next quarter, but, if not, can we keep the snarkfest version running please until this is resolved? Today, the treeherder version is not loading enough results to do a reasonable comparison, while the snarkfest version doesn't seem to have the problem and presents results almost instantly. If I may sneak in a request or two, then a number of results or an estimate of standard error in the mean would be helpful to know how to interpret the standard deviations. Also, a way to see whether higher or lower is better even for those tests without enough data to detect a statistically significant change would be a bonus. These sound like good ideas. Can't remember whether we've filed issues for them yet. Feel free to have a look at the perfherder bug list and file a feature request if something isn't open yet: https://wiki.mozilla.org/Auto-tools/Projects/Perfherder#Bug_Table I'm keen to find out what is in the Details links, but they currently just ask me to wait a minute. Likewise, that sounds like a bug. Clicking on details is supposed to open up a subtest summary. Here it seems to work fine for me: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-centraloriginalRevision=b2ad2f9f8e53newProject=mozilla-centralnewRevision=f21fac50a8cc Will ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Private members of ref counted classes and lambdas
On 06/04/2015 07:29 AM, Andrew Osmond wrote: Suppose I have some ref counted class Foo with the private member mBar. Normally with a lambda expression, [...] obviously the Foo object could get released before the dispatch completes You may be interested in this thread from a few months back: Proposal to ban the usage of refcounted objects inside C++ lambdas in Gecko https://groups.google.com/d/msg/mozilla.dev.platform/Ec2y6BWKrbM/xpHLGwJ337wJ (Not sure it arrived at a concrete conclusion, but you may run across some pitfalls/suggestions at least. I haven't used lambdas in C++, so I won't attempt to directly answer your question.) ~Daniel ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Private members of ref counted classes and lambdas
On Jun 4, 2015, at 11:17 AM, Daniel Holbert dholb...@mozilla.com wrote: You may be interested in this thread from a few months back: Proposal to ban the usage of refcounted objects inside C++ lambdas in Gecko https://groups.google.com/d/msg/mozilla.dev.platform/Ec2y6BWKrbM/xpHLGwJ337wJ (Not sure it arrived at a concrete conclusion, but you may run across some pitfalls/suggestions at least. I haven't used lambdas in C++, so I won't attempt to directly answer your question.) My impression was that the conclusion was “refcounted objects are not banned inside C++ lambdas” - i.e., no policy change from the status quo - but we need to be aware of the pitfalls”. The pitfalls are discussed pretty thoroughly in the thread. - Seth ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Linked Data and a new Browser API event
On 3 June 2015 at 19:42, Benjamin Francis bfran...@mozilla.com wrote: This is what I'd really like to get more of, particularly usage data. I've reached out to a few people at Yahoo, Google and a couple of universities and have managed to turn up a few studies with useful data [1][2][3][4]. My conclusions so far are: - Microformats are used on a large number of web sites but are limited by their case by case syntax and more fixed vocabulary and are less formally defined. - Microdata and RDFa are vocabulary agnostic which makes them inherently more extensible, they're increasing in popularity due to schema.org and consumption by major search engines, whilst the use of Microformats has remained relatively constant over time. - Microdata is a bit more concise than RDFa but doesn't allow for the mixing of vocabularies. - Open Graph is a simplistic form of RDFa with a limited vocabularly and limited usefulness in comparison to other formats, but is very widely used due to Facebook and Twitter being major consumers. - Microformats is used by more websites (domains) but Microdata is used by more web pages (more URLs, more typed entities and more triples) and is growing the fastest. Microformats has the breadth, but Microdata has the depth. In our case I think what we care about is the latter - the amount of pinnable content. - JSON-LD is the newest format, the main difference being that it isn't intended to be embedded in with HTML markup, but is included separately in a script tag. It's also useful as a canonical JSON-based format to represent all of the other formats. That leads me to recommend that we do the following: - Parse Microdata and RDFa (including Open Graph) from web pages in Gecko - Expose all of this data to Gaia via a single getLinkedData() or getStructuredData() method on the Browser API which returns a Promise that resolves with the data in a canonical JSON-LD format - Also consider supporting JSON-LD directly as no parsing is required, we just need to detect a script tag If anyone finds any more usage data, or has a different interpretation of the data below, then please do share. Thanks Ben 1. Web Data Commons website based on Common Crawl corpus (2009-2014) http://webdatacommons.org/ 2. Web Data Commons Paper based on Common Crawl Corpus (2009-2012) http://events.linkeddata.org/ldow2012/papers/ldow2012-inv-paper-2.pdf 3. Yahoo post based on Yahoo corpus (2011) https://tripletalk.wordpress.com/2011/01/25/rdfa-deployment-across-the-web/ 4. Yahoo paper based on Bing corpus (2012) http://events.linkeddata.org/ldow2012/papers/ldow2012-inv-paper-1.pdf ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Jun 4, 2015 10:27 AM, Daniel Holbert dholb...@mozilla.com wrote: Also: in layout, there are various warnings related to coordinate wraparound/overflow, where we're basically throwing up our hands and warning that broken layout is likely to occur because the page is millions of pixels tall. These can be useful hints, when debugging page brokenness. I know that it's more work, but isn't that what the console is best suited for? We have some spammy warnings there too, of course (try looking for rc4 warnings if you have gmail open). ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 06/04/2015 05:30 AM, Ehsan Akhgari wrote: There are very good reasons for warnings to not cause tests to fail. We have a lot of tests that are testing failure conditions that are expected to warn, because they are failure conditions. Also: in layout, there are various warnings related to coordinate wraparound/overflow, where we're basically throwing up our hands and warning that broken layout is likely to occur because the page is millions of pixels tall. These can be useful hints, when debugging page brokenness. We have a lot of tests that exercise how we behave around this coordinate-overflow threshold [mostly that we don't crash], and it's expected that such tests would trigger these warnings. One thing that might help for noise from these: Jesse filed a bug[1] a while back on adding a flag to detect huge sizes suppress some assertions when we do; we could conceivably use this flag to suppress warnings [perhaps just displaying a single detected huge sizes warning], as well. ~Daniel [1] https://bugzilla.mozilla.org/show_bug.cgi?id=765861 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Per-test chaos mode now available, use it to help win the war on orange!
I just landed bug 1164218 on inbound, which adds the ability to run individual mochitests and reftests in chaos mode. (For those unfamiliar with chaos mode, it's a feature added by roc a while back that makes already-random things more random; see [1] or bug 955888 for details). The idea with making it available per-test is that new tests should be written and tested locally/on try with chaos mode enabled, to flush out possible intermittent failures faster. Ideally we should also land them with chaos mode enabled. At this time we're still not certain if this will provide a lot of value (i.e. if chaos-mode-triggered failures are representative of real bugs) so it's not mandatory to make your tests run in chaos mode, but please do let me know if you try enabling it on your test and are either successful or not. We need to collect more data on the usefulness of this to see where we should take it. If it does turn out to be valuable, my hope is that we can start making pre-existing tests chaos-mode enabled as well, and eventually reduce the intermittent failure rate. See [2] for an example of how to enable chaos mode in your tests. Basically you can add chaos-mode to the reftest.list file for reftests, or call SimpleTest.testInChaosMode() for mochitests. If you do run into intermittent failures, the best way to debug them is usually to grab a recording of the failure using rr [3] and then debug the recording to see what was going on. This only works on Linux (and has some hardware requirements as well) but it's a really great tool to have. Cheers, kats [1] http://robert.ocallahan.org/2014/03/introducing-chaos-mode.html [2] https://hg.mozilla.org/integration/mozilla-inbound/rev/89ac61464a45 [3] http://rr-project.org/ or https://github.com/mozilla/rr/ ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Thu, Jun 4, 2015 at 1:18 PM, smaug sm...@welho.com wrote: More likely we need to change a small number of noisy NS_ENSURE_* macro users to use something else, and keep most of the NS_ENSURE_* usage as it is. +1. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Thu, Jun 4, 2015 at 2:49 AM, Jonas Sicking jo...@sicking.cc wrote: FWIW, I suspect it'll be hard to put a dent in the number of warnings that we emit unless we either change all instances of NS_ENSURE_SUCCESS(rv, rv) to use some other macro which doesn't warn, or unless we change NS_ENSURE_SUCCESS(rv, rv) to not warn. As you can see the from the list, the distribution of these warnings is very uneven. It is not necessary to solve the entire warning issue to make huge reductions in the total volume of warnings we see. For instance, in bug 1170642 I noticed that there were about 16MB of warnings in e10s M2, produced by a single line of code (which Eric fixed by removing the warning). Andrew It feels like right now we have three incompatible desires: * Test lots of failure cases. * Make errors warn in debug builds on all/most frames as the failure is propagated up the callstack. * Don't warn a lot when testing debug builds. It seems like we can only pick two out of these three. / Jonas On Wed, Jun 3, 2015 at 6:14 PM, Eric Rahm er...@mozilla.com wrote: We emit a *lot* of runtime warnings when running debug tests. I inadvertently triggered a max log size failure during a landing this week which encouraged me to take a look at what all is being logged, and what I found was a ton of warnings (sometimes accompanied by stack traces). Most of these should probably be removed (of course if they're real issues they should be fixed, but judging by the frequency most are probably non-issues). I'm currently cleaning up some of these, but if you happen to see something in the following list and are feeling proactive I would appreciate the help. There's even a meta bug for tracking these: https://bugzilla.mozilla.org/show_bug.cgi?id=765224 I generated this list by grabbing the logs for a recent m-c linux64 debug run, normalizing out PIDs and timestamps and then doing some sort/uniq-fu to get counts of unique lines. This is roughly the top 40 offenders: 65959 [N] WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 83 63460 [N] WARNING: NS_ENSURE_TRUE(piTarget) failed: file gdom/events/EventDispatcher.cpp, line 469 20039 [N] WARNING: 'NS_FAILED(rv)', file gdom/workers/ServiceWorkerManager.cpp, line 2529 20039 [N] WARNING: '!BasePrincipal::IsCodebasePrincipal(aPrincipal)', file gdom/workers/ServiceWorkerManager.cpp, line 2591 17784 [N] WARNING: Subdocument container has no frame: file glayout/base/nsDocumentViewer.cpp, line 2506 16322 JavaScript warning: file:///builds/slave/test/build/tests/jsreftest/tests/js1_8/extensions/regress-476427.js, line 1: JavaScript 1.6's for-each-in loops are deprecated; consider using ES6 for-of instead 14159 [N] WARNING: NS_ENSURE_TRUE(mMutable) failed: file gnetwerk/base/nsSimpleURI.cpp, line 264 14087 [N] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111: file gdocshell/base/nsDocShell.cpp, line 4592 11315 [N] WARNING: '!mMainThread', file gxpcom/threads/nsThreadManager.cpp, line 299 10574 [N] WARNING: No docshells for remote frames!: file gdom/base/nsFrameLoader.cpp, line 491 9201 [N] WARNING: have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'psd-mIEnd != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 884 9155 [N] WARNING: have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'psd-mIEnd != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 3058 9130 [N] WARNING: have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'aISize != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 160 8844 [N] WARNING: Someone passed native anonymous content directly into frame construction. Stop doing that!: file glayout/base/nsCSSFrameConstructor.cpp, line 6559 7599 [N] WARNING: NS_ENSURE_TRUE(mDocShell) failed: file gembedding/browser/nsWebBrowser.cpp, line 363 7454 [N] WARNING: anonymous nodes should not be in child lists (bug 439258): file glayout/base/RestyleManager.cpp, line 1440 6544 [N] WARNING: Graph thread slowdown?: 'std::abs(framePosition - CurrentDriver()-StateComputedTime()) MillisecondsToMediaTime(5)', file gdom/media/MediaStreamGraph.cpp, line 1195 6126 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file gnetwerk/base/nsFileStreams.cpp, line 492 6126 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file gnetwerk/base/nsFileStreams.cpp, line 205 5637 [N] WARNING: No outer window available!: file gdom/base/nsGlobalWindow.cpp,
Re: Survey on code review quality
Hi Gijs, Sorry for late reply (for some reason we've never received a notification of your post)! Our much earlier study was about the lifecycle of Firefox patches: The Secret Life of Patches: A Firefox Case Study (https://cs.uwaterloo.ca/~obaysal/wcre2012-baysal.pdf) We then worked on Martin Best's Bugzilla Anthropology project (https://wiki.mozilla.org/Bugzilla_Anthropology) where we analyzed the interview data that he collected. The tech report is available online (http://www.cs.uwaterloo.ca/research/tr/2012/CS-2012-10.pdf). In this study, we developed a situational awareness tool organized around custom developer dashboards. Pre-prints of the published work are also publicly available: No Issue Left Behind: Reducing Information Overload in Issue Tracking (https://cs.uwaterloo.ca/~obaysal/fse14-baysal.pdf) and DASHboards: Enhancing Developer Situational Awareness (https://cs.uwaterloo.ca/~obaysal/icse14_kononenko_preprint.pdf) Currently we are looking into Mozilla's code review process and investigating the factors that affect its quality. Olga and Oleksii ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR 2.0 staged. Feedback please!
I am wondering, how close are we to be able to index IDL/WebIDL files, and navigate through JS and C++ callers and implementations of those attributes/methods? That is currently the biggest reason why I have to use MXR from time to time, and it would be nice to see DXR growing support for this... With the ability to implement languages as plugins and have them cooperate on individual files, I don't think we're far. I do have a ticket open (https://bugzilla.mozilla.org/show_bug.cgi?id=1109804), but I know next to nothing about those IDLs. Would you be willing to spend a few minutes giving me a tour of what you'd like sometime after 2.0 lands? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR 2.0 staged. Feedback please!
It looks like finding of overrides of virtual methods is missing from DXR 2.0. Is this intentional? -Jeff On Wed, Jun 3, 2015 at 3:10 PM, Erik Rose e...@mozilla.com wrote: DXR 2.0 is about to land! This is a major revision touching every part of the system, swapping out SQLite for elasticsearch, and replacing many hard-coded C++ assumptions with a language-independent plugin interface. Please take it for a spin on the staging server at http://dxr.allizom.org/, and see if you find any regressions from the production version at dxr.mozilla.org. You can file them directly at https://bugzilla.mozilla.org/enter_bug.cgi?product=Webtoolscomponent=DXRstatus_whiteboard=es or just reply. Barring showstoppers, we plan to put it into prod within a few weeks. What's new? * Improved C/C++ analysis * Multi-language support—Python and Rust, for starters, soon to be enabled for moz-central * All queries are fast—and will be even faster in prod, once our webheads and elasticsearch servers are colocated * Browsing of images * Listing of binary files * Result counts (so jrudermann can have googlefights) * Independent tree indexing, so one build failure won't scuttle updates for the rest of the trees. (This will help us get all the trees currently under MXR indexed.) * Parallel indexing so we can set the DC on fire * New plugin architecture so we can add new languages, query types, and cross references easily (https://dxr.readthedocs.org/en/es/development.html#writing-plugins) This is really a backend-focused release, but you can see some of the new possibilities start to leak out. I'm enthusiastic about the features this will enable next: better surfacing of symbols without having to know their type ahead of time, faceted drill-down, context for search results, and permalinks (our last major blocker to decommissioning MXR). Thanks for helping test it out! Erik Rose DXR Lead ___ 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
Private members of ref counted classes and lambdas
Suppose I have some ref counted class Foo with the private member mBar.
Normally with a lambda expression, you can easily access the private
members by passing the this pointer:
void Foo::pokeBar()
{
nsCOMPtrnsIRunnable r = NS_NewRunnableFunction[this] () - void
{
mBar.poke();
});
NS_DispatchToMainThread(r);
}
But obviously the Foo object could get released before the dispatch
completes. I could capture an nsRefPtr to this in the lambda:
nsresult Foo::pokeBar()
{
nsRefPtrFoo self = this;
nsCOMPtrnsIRunnable r = NS_NewRunnableFunction[self] () - void
{
self-mBar.poke();
});
NS_DispatchToMainThread(r);
}
But now it won't let me access the private members. I can add *both* and it
appears to work (at least on gcc, haven't tested clang/msvc):
nsresult Foo::pokeBar()
{
nsRefPtrFoo self = this;
nsCOMPtrnsIRunnable r = NS_NewRunnableFunction[this, self] () - void
{
self-mBar.poke();
});
NS_DispatchToMainThread(r);
}
Since I'm using the self pointer, I don't see the compiler optimizing the
capture of it out, and the addition of this opens up the scope to what I
need. (Although maybe explicitly using self is overkill? Can I eliminate
that safely?)
But this is my first time using lambda expressions in C++, so I'm not
entirely sure if there is a good reason for *not* doing what I am above :).
In the few examples of lambdas that I found in our code, it looks like we
just made everything public. My other attempts at fixing this was to make
an nsRefPtr to mBar inside Foo::pokeBar and pass *that* to the lambda,
which is fine but my real world uses have multiple private members and
functions I would like to access :). Is there anybody who can chime in on
how best to use lamdas with private members?
The only guidance I could find on the wiki was this, which I think I am
following (e.g. don't use [=] to suck down everything):
https://developer.mozilla.org/en-US/docs/Mozilla/C++_Portability_Guide#Use_C.2B.2B_lambdas.2C_but_with_care
Thanks,
Andrew
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: DXR 2.0 staged. Feedback please!
It looks like finding of overrides of virtual methods is missing from DXR 2.0. Is this intentional? Hmm, no. The tests seem to pass (https://github.com/mozilla/dxr/blob/es/dxr/plugins/clang/tests/test_overrides.py). Where are you seeing it? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Process-level mitiagtions are being turned on for the Windows content process sandbox
Hi all, The next Nightly should have certain process-level mitigations turned on for the Windows content process sandbox. These are Chromium sandbox features that ensure that things like DEP, ASLR and SEHOP are turned on for the content process when available. If you are running Nightly on Windows with e10s enabled, this will be on by default. Please file any regressions this causes to block bug 119. You can test by: * using a weaker sandbox by setting security.sandbox.content.level to 0 and restarting * turning off the content sandbox altogether by setting an environment variable MOZ_DISABLE_CONTENT_SANDBOX to 1 before starting Firefox. Thanks, Bob ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
Nicholas Nethercote writes: Do warnings (as opposed to NS_ASSERTION) do anything in tests? I don't think they do. If that's right, a warning is only useful if a human looks at it and acts on it, and that's clearly not happening for a lot of these. Warnings in tests don't do anything but log that a problem has occurred. But, when the warnings are not noisy, this is often useful for tracking the cause of intermittent failures. Similarly, warnings about real problems are helpful when running debug builds, but we are being spammed with so many warnings that it is hard to imagine they can all be real problems. If they are, then we are doing a poor job of keeping up with them. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Linked Data and a new Browser API event
On 04/06/2015 12:34 , Benjamin Francis wrote: On 4 June 2015 at 03:27, Michael[tm] Smith m...@w3.org wrote As came up in some off-list discussion with Anne, is the “Manifest for a web application” spec at https://w3c.github.io/manifest/ not relevant here? (Nothing to reverse engineer, since it has an actual spec—with defined processing requirements—and at least one other browser-engine project is also contributing to it and implementing it.) Yes, we already support W3C web app manfiests in our prototype, and it's a key part of the implementation. A manifest provides metadata for a website as a whole, whereas Linked Data provides metadata to a particular web page. When you pin a whole site we use the manifest (and fall back to other metadata when not available), and when you pin a page we use Linked Data (and fall back to other metadata when not available). Pinning based on Linked Data also makes a lot of sense because it's already massively deployed (to millions of domains), whereas manifest isn't. To reinforce Benjamin's point imagine a flight booking site. The manifest would get you to pin the site as an app with which you could book flights in the future; LD would allow you to pin a ticket you've bought in such a way that it displays just the essential time/location information you want to see at a glance. The use cases are totally different. -- Robin Berjon - http://berjon.com/ - @robinberjon ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Linked Data and a new Browser API event
On 4 June 2015 at 03:27, Michael[tm] Smith m...@w3.org wrote As came up in some off-list discussion with Anne, is the “Manifest for a web application” spec at https://w3c.github.io/manifest/ not relevant here? (Nothing to reverse engineer, since it has an actual spec—with defined processing requirements—and at least one other browser-engine project is also contributing to it and implementing it.) Yes, we already support W3C web app manfiests in our prototype, and it's a key part of the implementation. A manifest provides metadata for a website as a whole, whereas Linked Data provides metadata to a particular web page. When you pin a whole site we use the manifest (and fall back to other metadata when not available), and when you pin a page we use Linked Data (and fall back to other metadata when not available). Ben ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Private members of ref counted classes and lambdas
Turns out my original problem was some other mistake I made. Using just self works (thanks botond for the poke on IRC about that). I remember reading the linked thread, although I had since forgotten about it -- thanks for the reminder. My impression was that using raw pointers for ref counted objects with lambdas is something to be discouraged, rightfully so. That's why I use nsRefPtr, just like I would if I explicitly used a nested class inheriting from nsRunnable instead of a lambda. I'll review the thread again though (I *did* forgot about using Move, perhaps other bits as well). On Thu, Jun 4, 2015 at 2:24 PM, Seth Fowler s...@mozilla.com wrote: On Jun 4, 2015, at 11:17 AM, Daniel Holbert dholb...@mozilla.com wrote: You may be interested in this thread from a few months back: Proposal to ban the usage of refcounted objects inside C++ lambdas in Gecko https://groups.google.com/d/msg/mozilla.dev.platform/Ec2y6BWKrbM/xpHLGwJ337wJ (Not sure it arrived at a concrete conclusion, but you may run across some pitfalls/suggestions at least. I haven't used lambdas in C++, so I won't attempt to directly answer your question.) My impression was that the conclusion was “refcounted objects are not banned inside C++ lambdas” - i.e., no policy change from the status quo - but we need to be aware of the pitfalls”. The pitfalls are discussed pretty thoroughly in the thread. - Seth ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 06/04/2015 01:07 PM, David Rajchenbach-Teller wrote: Part of my world domination plans are to turn warnings into something that causes test to actually fail (see bug 1080457 co). Would you like to join forces? Turning warnings into failures sounds odd (but bug 1080457 seems to be actually something different). Warnings aren't anything which should cause tests to fail. Warnings, especially those generated by NS_ENSURE_* macros are things which are super useful for debugging - they often point rather exactly where to start looking for an issue. And since there is NS_ENSURE_*, there isn't necessarily any problem or bug, just an unusual state, which is then handled by the NS_ENSURE_* macro. -Olli Cheers, David On 04/06/15 03:14, Eric Rahm wrote: We emit a *lot* of runtime warnings when running debug tests. I inadvertently triggered a max log size failure during a landing this week which encouraged me to take a look at what all is being logged, and what I found was a ton of warnings (sometimes accompanied by stack traces). Most of these should probably be removed (of course if they're real issues they should be fixed, but judging by the frequency most are probably non-issues). I'm currently cleaning up some of these, but if you happen to see something in the following list and are feeling proactive I would appreciate the help. There's even a meta bug for tracking these: https://bugzilla.mozilla.org/show_bug.cgi?id=765224 I generated this list by grabbing the logs for a recent m-c linux64 debug run, normalizing out PIDs and timestamps and then doing some sort/uniq-fu to get counts of unique lines. This is roughly the top 40 offenders: ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Thu, Jun 4, 2015 at 5:38 AM, Robert O'Callahan rob...@ocallahan.org wrote: Usually I use NS_WARNING to mean something weird and unexpected is happening, e.g. a bug in Web page code, but not necessarily a browser bug. Sometimes I get useful hints from NS_WARNING spew leading up to a serious failure. Yup. I think this is a quite common, and quite useful, usage of NS_WARNING. But testing runs a lot of weird and unexpected things. That's a good thing because those tend to be things that often regress. But it also leads to a lot of warnings. but for those usages could probably be switched to PR_LOG without losing much. I think this would mean changing most NS_ENSURE_SUCCESS(rv, rv), and likely many other NS_ENSURE_* to macros that call PR_LOG instead. So like I said, we'll have to either change a huge number of NS_ENSURE_* macros to use something else, or change what NS_ENSURE_* does. / Jonas Rob -- oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo owohooo osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o'oRoaocoao,o'o oioso oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo owohooo osoaoyoso,o o'oYooouo ofolo!o'o owoiololo oboeo oiono odoaonogoeoro ooofo otohoeo ofoioroeo ooofo ohoeololo. ___ 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: Private members of ref counted classes and lambdas
On Thu, Jun 4, 2015 at 2:24 PM, Seth Fowler s...@mozilla.com wrote: My impression was that the conclusion was “refcounted objects are not banned inside C++ lambdas” - i.e., no policy change from the status quo - but we need to be aware of the pitfalls”. The pitfalls are discussed pretty thoroughly in the thread. Capturing of raw pointers to refcounted objects was in fact banned - a static analysis forbidding it was introduced in bug 1153304. (Note that, as I discovered recently, the static analysis was over-eager and was also complaining about raw pointers to refcounted object being taken as an argument or used as a local variable inside a lambda - I'm about to land a fix for that in bug 1170388). Cheers, Botond ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On Thu, Jun 4, 2015 at 2:49 AM, Jonas Sicking jo...@sicking.cc wrote: It feels like right now we have three incompatible desires: * Test lots of failure cases. * Make errors warn in debug builds on all/most frames as the failure is propagated up the callstack. * Don't warn a lot when testing debug builds. It seems like we can only pick two out of these three. Do warnings (as opposed to NS_ASSERTION) do anything in tests? I don't think they do. If that's right, a warning is only useful if a human looks at it and acts on it, and that's clearly not happening for a lot of these. Nick ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
FWIW, I suspect it'll be hard to put a dent in the number of warnings that we emit unless we either change all instances of NS_ENSURE_SUCCESS(rv, rv) to use some other macro which doesn't warn, or unless we change NS_ENSURE_SUCCESS(rv, rv) to not warn. It feels like right now we have three incompatible desires: * Test lots of failure cases. * Make errors warn in debug builds on all/most frames as the failure is propagated up the callstack. * Don't warn a lot when testing debug builds. It seems like we can only pick two out of these three. / Jonas On Wed, Jun 3, 2015 at 6:14 PM, Eric Rahm er...@mozilla.com wrote: We emit a *lot* of runtime warnings when running debug tests. I inadvertently triggered a max log size failure during a landing this week which encouraged me to take a look at what all is being logged, and what I found was a ton of warnings (sometimes accompanied by stack traces). Most of these should probably be removed (of course if they're real issues they should be fixed, but judging by the frequency most are probably non-issues). I'm currently cleaning up some of these, but if you happen to see something in the following list and are feeling proactive I would appreciate the help. There's even a meta bug for tracking these: https://bugzilla.mozilla.org/show_bug.cgi?id=765224 I generated this list by grabbing the logs for a recent m-c linux64 debug run, normalizing out PIDs and timestamps and then doing some sort/uniq-fu to get counts of unique lines. This is roughly the top 40 offenders: 65959 [N] WARNING: Overflowed nscoord_MAX in conversion to nscoord width: file ../../dist/include/nsRect.h, line 83 63460 [N] WARNING: NS_ENSURE_TRUE(piTarget) failed: file gdom/events/EventDispatcher.cpp, line 469 20039 [N] WARNING: 'NS_FAILED(rv)', file gdom/workers/ServiceWorkerManager.cpp, line 2529 20039 [N] WARNING: '!BasePrincipal::IsCodebasePrincipal(aPrincipal)', file gdom/workers/ServiceWorkerManager.cpp, line 2591 17784 [N] WARNING: Subdocument container has no frame: file glayout/base/nsDocumentViewer.cpp, line 2506 16322 JavaScript warning: file:///builds/slave/test/build/tests/jsreftest/tests/js1_8/extensions/regress-476427.js, line 1: JavaScript 1.6's for-each-in loops are deprecated; consider using ES6 for-of instead 14159 [N] WARNING: NS_ENSURE_TRUE(mMutable) failed: file gnetwerk/base/nsSimpleURI.cpp, line 264 14087 [N] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) failed with result 0x80040111: file gdocshell/base/nsDocShell.cpp, line 4592 11315 [N] WARNING: '!mMainThread', file gxpcom/threads/nsThreadManager.cpp, line 299 10574 [N] WARNING: No docshells for remote frames!: file gdom/base/nsFrameLoader.cpp, line 491 9201 [N] WARNING: have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'psd-mIEnd != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 884 9155 [N] WARNING: have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'psd-mIEnd != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 3058 9130 [N] WARNING: have unconstrained width; this should only result from very large sizes, not attempts at intrinsic width calculation: 'aISize != NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 160 8844 [N] WARNING: Someone passed native anonymous content directly into frame construction. Stop doing that!: file glayout/base/nsCSSFrameConstructor.cpp, line 6559 7599 [N] WARNING: NS_ENSURE_TRUE(mDocShell) failed: file gembedding/browser/nsWebBrowser.cpp, line 363 7454 [N] WARNING: anonymous nodes should not be in child lists (bug 439258): file glayout/base/RestyleManager.cpp, line 1440 6544 [N] WARNING: Graph thread slowdown?: 'std::abs(framePosition - CurrentDriver()-StateComputedTime()) MillisecondsToMediaTime(5)', file gdom/media/MediaStreamGraph.cpp, line 1195 6126 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file gnetwerk/base/nsFileStreams.cpp, line 492 6126 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file gnetwerk/base/nsFileStreams.cpp, line 205 5637 [N] WARNING: No outer window available!: file gdom/base/nsGlobalWindow.cpp, line 3915 5109 [N] WARNING: NS_ENSURE_TRUE(domWindow) failed: file gembedding/browser/nsDocShellTreeOwner.cpp, line 83 5085 [N] WARNING: NS_ENSURE_TRUE(aInBrowser) failed: file gembedding/browser/nsDocShellTreeOwner.cpp, line 79 4856 [N] WARNING: zero axis length: file gdom/svg/nsSVGLength2.cpp, line 124 4708 [N] WARNING: Shouldn't call SchedulePaint in a detached pres context: file
Re: The War on Warnings
On 2015-06-04 6:07 AM, David Rajchenbach-Teller wrote: Part of my world domination plans are to turn warnings into something that causes test to actually fail (see bug 1080457 co). Would you like to join forces? There are very good reasons for warnings to not cause tests to fail. We have a lot of tests that are testing failure conditions that are expected to warn, because they are failure conditions. In my experience, in some cases where we have a particularly spammy warning, the underlying reason is that there is an error condition that we know how to handle properly, but we still warn about it. Or alternatively, there are warnings which indicate that the author of the code did not think at the time that can happen, but a lot of them can be ignored. There are also warnings that were actually meant to be assertions, and we should try to convert them to assertions, and that way they will fail tests. That being said, assertion report stack traces in debug builds, which is excruciatingly slow, so we have had to switch them to be warnings in at least some cases (see bug 756045 for example.) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 2015-06-04 5:49 AM, Jonas Sicking wrote: FWIW, I suspect it'll be hard to put a dent in the number of warnings that we emit unless we either change all instances of NS_ENSURE_SUCCESS(rv, rv) to use some other macro which doesn't warn, or unless we change NS_ENSURE_SUCCESS(rv, rv) to not warn. Do we really have to do that? It seems like all we need to do is to investigate the highly occurring warnings and silence them somehow. NS_ENSURE_SUCCESS warning is super useful when debugging things such as intermittent test failures, or big complicated web pages that are triggering an issue somehow -- they can often give you a starting point on where you need to look. It feels like right now we have three incompatible desires: * Test lots of failure cases. * Make errors warn in debug builds on all/most frames as the failure is propagated up the callstack. * Don't warn a lot when testing debug builds. It seems like we can only pick two out of these three. It seems to me that if we change #3 to mean fixing highly frequently occurring warnings, we can have all three of the above together. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 06/04/2015 09:52 PM, Jonas Sicking wrote: On Thu, Jun 4, 2015 at 5:38 AM, Robert O'Callahan rob...@ocallahan.org wrote: Usually I use NS_WARNING to mean something weird and unexpected is happening, e.g. a bug in Web page code, but not necessarily a browser bug. Sometimes I get useful hints from NS_WARNING spew leading up to a serious failure. Yup. I think this is a quite common, and quite useful, usage of NS_WARNING. But testing runs a lot of weird and unexpected things. That's a good thing because those tend to be things that often regress. But it also leads to a lot of warnings. but for those usages could probably be switched to PR_LOG without losing much. I think this would mean changing most NS_ENSURE_SUCCESS(rv, rv), and likely many other NS_ENSURE_* to macros that call PR_LOG instead. So like I said, we'll have to either change a huge number of NS_ENSURE_* macros to use something else, or change what NS_ENSURE_* does. More likely we need to change a small number of noisy NS_ENSURE_* macro users to use something else, and keep most of the NS_ENSURE_* usage as it is. -Olli / Jonas Rob -- oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo owohooo osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o'oRoaocoao,o'o oioso oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo owohooo osoaoyoso,o o'oYooouo ofolo!o'o owoiololo oboeo oiono odoaonogoeoro ooofo otohoeo ofoioroeo ooofo ohoeololo. ___ 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: The War on Warnings
Usually I use NS_WARNING to mean something weird and unexpected is happening, e.g. a bug in Web page code, but not necessarily a browser bug. Sometimes I get useful hints from NS_WARNING spew leading up to a serious failure, but for those usages could probably be switched to PR_LOG without losing much. Rob -- oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo owohooo osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o oioso oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo owohooo osoaoyoso,o o‘oYooouo ofolo!o’o owoiololo oboeo oiono odoaonogoeoro ooofo otohoeo ofoioroeo ooofo ohoeololo. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA: wiki page for platform-specific defines
On Wednesday, June 3, 2015 at 9:16:46 PM UTC-4, Xidorn Quan wrote: I guess it is probably better to add different color on true and false, which should improve the readability. Or probably just remove all false? Looks like Mike and Adam cleaned it to be much more readable, thanks! I also added a second table for prefs files (although I'm not sure where b2g.js is used so I left in some question marks in that one). ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR 2.0 staged. Feedback please!
This is great to see Erik! Thanks everyone for their hard work! I am wondering, how close are we to be able to index IDL/WebIDL files, and navigate through JS and C++ callers and implementations of those attributes/methods? That is currently the biggest reason why I have to use MXR from time to time, and it would be nice to see DXR growing support for this... Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The War on Warnings
On 04/06/15 14:30, Ehsan Akhgari wrote: There are very good reasons for warnings to not cause tests to fail. We have a lot of tests that are testing failure conditions that are expected to warn, because they are failure conditions. Well, that's why the patch linked above offers a whitelist API, so that tests can individually decide which families of warnings are either expected, or ok-for-the-time-being. [...] There are also warnings that were actually meant to be assertions, and we should try to convert them to assertions, and that way they will fail tests. That being said, assertion report stack traces in debug builds, which is excruciatingly slow, so we have had to switch them to be warnings in at least some cases (see bug 756045 for example.) Are you talking about our weird non-fatal assertions or actual fatal assertions? -- David Rajchenbach-Teller, PhD Performance Team, Mozilla ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
