mozilla :: #fx-team

12 Jul 2017
01:47dolskehuh. that's weird. after my browser crashed, it restarted with Compact Light instead of Compact Dark. O_o
03:20gandalfMossop: do you want me to retest the regexp in a microbenchmark against xpcshell? I could switch getIdentifier to regexp for example
03:22Mossopgandalf: No, it's ok
13:23johannhmaybe this is a stupid question
13:23johannhbut this function is marked async
13:24johannhbut afaict most consumers call it without await
13:24johannhwhat does the await statement inside that function do then?
13:24johannhI'm asking because it's making my code crash
13:25johannhlike, in a way that I don't understand yet
13:27johannhJohn-Galt maybe knows but I guess he's not awake yet
13:30florianjohannh: it'll delay the execution of the lines after the await until that promise is resolved
13:30johannhflorian: oh
13:30makjohannh: if the question is just about not awaiting, what happens is that the async function awaits properly internally, the external function won't wait for it though, and as soon as the async function reaches await, the external function will continue. That means if the async function throws, and the external function is not awaiting nor .catch()-ing,
13:30makyou'll get an unhandled rejection error in tests
13:30johannhah, ok thanks
13:31makbasically, that's a broken error checking chain. It may be correct or not to not await, it depends on the code, but errors should always be handled (as well as promises)
13:32makI think Paolo is working on a way to make tests fail in case of unhandled promises
14:31pastI believe that work has been completed and tests should fail on unhandled rejections
14:47florianmikedeboer: Hi. I think I'm seeing SessionSaver's timer intermittently firing before the end of startup. It seems to be mostly (only?) with e10s disabled. 1. Is this possible. 2. How much effort would it be to avoid it?
14:57mikedeboerflorian: I don't know off-hand I'm afraid... but I think it's quite unlikely, except if SessionStore.setGlobalValue is called very early or something...
14:57mikedeboerflorian: I'd be interested to know which timer
14:57florianmikedeboer: lots of details in this log:
14:58floriansearch first for: IMPORTING resource:///modules/RecentWindow.jsm
14:59florianI think the first IMPORTING resource:///modules/sessionstore/SessionSaver.jsm is also interesting
15:00florianthe timer seems to be started from
15:02florianthe timer is
15:07mikedeboer...which is coming from the messageManager from content-sessionStore.js
15:45Gijsaswan: if I have an embedded webextension, how do I get a url for an icon / asset in the add-on from xpcom / bootstrap.js and similar ?
15:50jawsGijs: is aRequest.originalURI something that we can trust? or is there a different property that we should use to see what the actual URI that will be loaded is? i'm thinking of how URI fixup works
15:50Gijsjaws: uh, not sure what the context for this is
15:50Gijsso what does "we can trust" mean?
15:52jawslol yeah i realize i left that out :P
15:53jawsGijs: this is in the context of determining if we should show the "stop" button for local (about:) loads
15:53Gijsyeah, I am unhappy about that development
15:53Gijsfile: can be just as slow as other things in some situations (e.g. network drives)
15:53Gijsanyway, not my circus not my monkey
15:54Gijsjaws: so which request in particular have about: URIs and have different original vs. plain URIs ?
15:54Gijsjaws: I think about:feeds does this, not sure about anything else
15:54Gijsjaws: but in principle about:reader?url=foo is remote, but I don't think that has an originalURI
15:55Standard8mikedeboer: did we implement some sort of .only / .skip for add_task in mochitests? i.e. to temporary just run one test?
16:06jawsGijs: we're not doing this for all file:, just about: and chrome: (realize that the install could be over a network drive but that would be pretty terrible experience i think)
16:07jawsokay, i'll test about:reader
16:07Gijsjaws: yeah, so, originalURI I think helps with redirecting channels of some sort
16:07Gijsjaws: but I'm not sure it's more reliably what you want in this case than just plain URI
16:07* Gijs would have to dig into it himself
16:19aswanGijs: you can use `extension.getURL("/relative/path")`
16:19Gijsaswan: oh neat... is that the webextension object returned by LegacyExtensionsUtils?
16:19Gijsor something else?
16:19Gijsalso, does it return a string or a URI object?
16:19Gijs(are there docs? I looked but didn't find them)
16:20aswanlets see, the docs about embedded extensions are kind of sdk-centric from what i recall
16:21aswani think the thing you get from LegacyExtensionUtils is a container that holds the extension plus some other stuff
16:22aswanbut it looks like the thing that you can call getURL() on is in a property called extension
16:22aswanso something like `LegacyExtensionUtils.getEmbeddedExtensionFor(foo).extension.getURL(...)`
16:22aswangetURL() returns a string
16:23aswanand there aren't any good docs about Extension objects
16:23aswanwe have jsdoc comments for what its worth, i don't think anybody has ever really gotten good usable docs built from them
16:23aswanbut eg
16:25Gijscool, thanks!
16:29aswanno problem
17:30mikedeboerStandard8: I'm afraid we did not :( It's not hard to implement I think!
17:33Standard8mikedeboer: yeah, shouldnt be too hard, switching to mocha
17:33* Standard8 hides
17:42mikedeboer:D Now there's a project! Sink your teeth in that one... pfew!
19:52Mossopmikedeboer: ping?
19:52mikedeboerMossop: pong
19:52Mossopmikedeboer: How would you feel about disabling browser_windowRestore_perwindowpb.js in order to fix bug 1244904?
19:52firebot NEW, Intermittent browser_windowRestore_perwindowpb.js | application crashed [@ mozilla::::RunWatchdog]
19:53mikedeboerMossop: well, that's kinda OK, but it's really caused by something else
19:53mikedeboerit's not the most important test we have... it's only Linux32, right?
19:54Mossopmikedeboer: Yeah I know, and my preference is to find someone to fix it. But it will block activity stream being preffed on so we have to figure out some solution
19:56mikedeboerahyes that's true - I saw Mardak saying it'd start to permafail with act stream on by default
20:05perryjaws: ping
20:44jawsperry: pong
20:47perryjaws: I found another issue with the stop-reload bug where if you start loading a tab, open a new tab, and switch back to the loading tab, the stop-reload wont switch to the correct button since .switchToStop/.switchToReload's aRequest is null. Do you know any way to detect when going to a tab that's currently loading?
20:48perrySo for example, going to will have the stop button, opening a new tab brings in the reload button, and then switching back to the loading keeps the reload button
20:52jawsperry: if aRequest is null then this the switch is due to a tabswitch, and we should show the different icon in that case
20:53jawsperry: so _shouldSwitch should return true if !aRequest
20:55mkaplyRyanVM|afk: Ha. I had just bisected and found the same bug. Man I love mozregression
20:55RyanVM|afki have low hopes of that being uplifted, fwiw
20:56RyanVM|afktbh, not sure it'd be worth it anyway since it's a longstanding issue IIRC
20:56mkaplyRyanVM|afk: Agreed. I had someone contact me in a panic claiming it's new. I'm seeing if I can munge the content to make it print.
20:58mkaplyRyanVM|afk: And sure enough, removing the overflow-y: scroll on the page fixed it.
20:58perryjaws: I originally didn't return false on !aRequest but a linux test failed with aRequest being not defined when opening a tab with a normal URL. Log is here and the test is here
21:04jawsperry: that failure is in the test that you wrote
21:05jawsperry: we don't set the animate attribute when switching tabs if there is no aRequest
21:06jawshmmm i don't see anything wrong with your test
21:07jawsperry: you should always be checking if aRequest is defined before de-referencing it
21:08jawsalso that test failure is on an earlier version of the patch which had a bug with _shouldAnimate
21:24andymwho knows about privacy.resistfingerprinting and wouldn't mind being cc'd on a bug?
21:34dolskeandym: you probably want someone in selena's org.
22:19mythmonJohn-Galt: does this error mean anything to you? Normandy started getting it recently in taskcluster, and hg bisect tells me it is related to a changeset of yours that landed recently
22:19mythmonspecifically this changeset
22:25perryjaws: ping
22:27mythmonJohn-Galt: thanks
22:53jawsperry: pong
22:54perryjaws: The tests to check stop-reload animation won't pass without at least one assertion pass - I can put an Assert.ok to make sure that an animation happens, but I can't do the same for the ones that make sure no animation happens
22:55jawsperry: are you saying this in response to my comment about changing that last Assert.ok to info()
22:55perryjaws: yeah, sorry should have mentioned that
22:55jawsif there is indeed no other Assert.ok calls in that code path, then it is fine to keep it as Assert.ok for that test case
22:55jawsi think the other test cases all had Assert.ok in their codepath
22:55John-Galtmythmon: How did you manage to bisect that? Are you able to reproduce it locally?
22:56mythmonJohn-Galt: normandy's current test suite reproduces it
22:56mythmonit's busting our CI, actually, which is why I was looking at it
22:56perryjaws: well the only ones originally were the ones you asked to be changed to info(), but I can change the ones expecting animations to have an Assert.ok .then'd onto the animation promise
22:56John-Galtmythmon: Locally or only in CI? I tried to run those browser tests locally and they worked fine
22:57mythmonJohn-Galt: locally. i mean the tests we have in github. we haven't synced to m-c in a while
22:57jawsperry: it's ok to just change the last one in the tests (if there are already Assert.ok calls), no need to make big changes
22:57mythmonyeah, i'm not happy about it either.
22:57* John-Galt tries to figure out how to run normandy tests locally...
22:58mythmonJohn-Galt: our docs on the subject ^
22:58John-GaltThat sounds painful...
22:59mythmongit clone ...; npm install; ./bin/ /path/to/m-c; ./mach mochitest
23:01John-GaltApparently a mach build step is also required.
23:01mythmonoh, yes. sorry
23:03John-Galtmythmon: I can't reproduce it locally even after doing all that...
23:04mythmonJohn-Galt: which changeset are you on for m-c? maybe i've got something goofy here
23:05mythmonwhich hash? names like that don't mean much when trying to sync up
23:07John-GaltAh, actually, I can reproduce it with the latest inbound.
23:19John-Galtmythmon: Heh. So apparently it was miraculously fixed by the patch for bug 1378727 and broken by the backout...
23:19firebot NEW, Extension.jsm's promiseLocales method does main thread IO
23:20John-GaltWhich is odd
23:23mythmonJohn-Galt: that is odd. i have no idea how that could fix things
23:23John-GaltPossibly by accident because of a caching bug. Possibly just a timing issue.
23:28perryjaws: I don't understand what you were saying in your comment about
23:28perryThat isn't a local url so what would be the condition for it?
23:29jawsperry: that page is displayed using about:feeds
23:32John-Galtmythmon: Ah. So apparently it's because your tests are starting a new instance of an extension before the old instance has shutdown, and removing the old file too soon, or something. One of the failed startups causes the whole startup/shutdown chain to fail.
23:33perryjaws: Ok, is there any way to know if a page is displayed using about:feeds? I'm not able to find any
23:33jawsperry: i think it's okay to leave it out of your patch
23:33perryjaws: alright, thanks
13 Jul 2017
No messages
Last message: 73 days and 18 hours ago