mozilla :: #ateam

12 Sep 2017
08:35whimbooted: so how can i get buildsymbols for artifact builds, does that work?
08:35whimbooif not we should error out
08:36whimbooi have a crash with a local artifact build but cannot use mozcrash to analyze it
08:36whimboomach buildsymbols returns immediately
09:47tedwe could probably make that work by just downloading the symbols from the existing build
09:47tedfile a bug?
11:17whimbooted: sure, i can file. so for now I would have to download it manually and place it to which location?
11:21whimboojgraham: so if runner.status() != Ok(None) doesnt work
11:22whimboobinary operation `!=` cannot be applied to type `std::result::Result<std::option::Option<std::process::ExitStatus>, std::io::Error>`
12:17whimbooted: updating mc to the 10.11 sdk seems to have caused issues with mouse gestures
12:18whimbooted: the double finger left and right swipe doesnt work for navigation
12:18whimbooas example
12:18whimboodoing a regressoin check right now
12:24jgrahamwhimboo: let status = runner.status(); if !(status.is_ok() && status.unwrap_or(None) == None) {} perhaps?
12:27whimbooted: so filed bug 1399086
12:27bugbotBug Event Handling, major, nobody, NEW , Mouse gestures no longer working since upgrade to MacOS 10.11 SDK
12:35atojgraham: When running &quot;./mach wpt&quot; with a non-optimised debug build, are the timeout values set accordingly?
12:36jgrahamato: They are supposed to be multiplied by the timeout multiplier which is >1 for a debug build
12:36jgrahamBut it&#39;s possible that your build isn&#39;t detected as debug (also, why non-optimised. Unless you have specific problems running a debugger, that&#39;s probably a bad idea)
12:38atoThe test Im running isnt marked with # META: timeout=long, so I think it is being multiplied.
12:39atoIn other words, I wouldve expected it to have timed out by this point. Something else must be wrong.
12:48whimboojgraham: thanks. i will play a bit with that
12:49whimboobut dealing with regressoins in Nightly first
13:00tedwhimboo: if you manually download the and unzip it under $objdir/dist/crashreporter-symbols things should just work
13:01whimbooted: ++ let me try that
13:01ted(that&#39;s what i&#39;d have the artifact build code do if we hooked that up)
13:01tedwhimboo: also re: your SDK bug, you&#39;ve got mstange and spohl on it, you don&#39;t need me :)
13:02whimbooted: yep. added them once it was clear... which was after the ping here
13:04whimbooted: so do i need the build for teh package-frontend or the task as referenced when downloading the file
13:05whimboointerestingly those are different changesets
13:05whimbooso i would assume the frontend one because it has the dmg
13:05tedwhimboo: if they both have a (i think that got fixed at some point) then either should work
13:06whimbooted: so there is and anything the full brings extra with it?
13:07tedwhimboo: it has the native debug symbols if you wanted to attach an actual debugger
13:07whimbooi think i will try without first
13:15whimbooted: hm, do I have to point mozcrash to that folder? so far no difference
13:15tedwhimboo: i would expect it to just work, but it&#39;s possible that something doesn&#39;t work right in artifact builds
13:15tedlike maybe it doesn&#39;t think the crashreporter is enabled?
13:16whimbooits enabled as listed in application.ini
13:16whimboomaybe i should check the real value when fx is open
13:19whimboomaybe i better download a full debug build from tc
13:57atoThe Minimize Window problems are annoying.
13:57jgrahamSo is the way that WebDriver deals with Windows
13:57atoAs I suspected, and this is something Ive noticed before, if I query the DOM property a couple of times it ends up in the expected state.
13:59atojgraham: Yeah, the API sucks.
14:01atoUnrelated, on a non-optimised debug build are using 364 out of the 600 wait-for-connection ticks in geckodriver waiting for Marionette to listen.
14:01atoNot a problem, but shows just how slow non-opt builds are.
14:04whimbooato: in geckodriver we have a delay of .1s, so that means 36.4s?
14:04atoProbably a bit more than that.
14:04whimboooverall the timeout is 60s
14:04whimboolet timeout = 60 * 1000;
14:04atoBecause of the compute time.
14:05whimboowell, i don&#39;t fight for 1-2s :)
14:05whimbooon tc workers we have more than 1min
14:05whimboofor Mn jobs, and other jobs using Marionette
14:06whimbooso 60s is kinda short
14:15atoThe basic problem is that document.hidden isnt updated synchronously. DOM properties get propagated at some point, most often quickly, but sometimes not.
14:15atoChromeWindow.windowState is more reliable and we can, empirically proven before we got rid of &quot;state&quot; on the window rect, rely on it.
14:16jgrahamato: I don&#39;t see what&#39;s wrong with the loop I suggested before
14:16atoInternally we wait for the sizemodechange event to fire, which in theory should be tied to document.hidden, but which in practice is tied to ChromeWindow.windowState.
14:17atoWell, hear me out.
14:17atoNow, Marionette _is_ returning after the window _has_ been iconified. The window is positively minimised by the point we return, but the next command that injects a command to check document.hidden property might fail because the DOM is slow.
14:18atoSo from the point of view of the specification we are conforming, but because window state isnt part of the window rect, the DOM query is our only alternative at testing if thats true.
14:19whimbooato: mind asking karlt for some feedback?
14:19atoSo there is an argument for making the document.hidden check a poll-wait, but there are other options too.
14:19atowhimboo: I dont think theres much help in doing that. I had a look at the mochitests testing this, and theyre all disabled on Linux due to the exact same problem.
14:20atowhimboo: This code path is entirely untested on Linux because of the inherent asynchronicity of X11.
14:21atoNow, other options include a Marionette-internal-poll wait on document.hidden, possibly with a timeout like jgraham suggested. I think thats safer than just a loop.
14:22jgrahamato: Just put the loop in the test. I expect other implemenations will have similar issues.
14:22atoThe wait-for-main-thread-to-be-idle + animation frame option isnt great in this case because a requestAnimationFrame hangs when a window is hidden.
14:24atoIf we wanted to make Marionette wait for document.hidden to be true, wed have to make an IPC call to the browser.
14:24atoIm not sure what the chance is that iconification would fail?
14:25atoIf it is somewhat reliable that iconification will never fail we could do a callback, otherwise wed need to poll which also isnt great.
14:31whimboojgraham: so that code for the process status works, but I don&#39;t really like the double-negation. I will update it slightly
14:32jgrahamwhimboo: OK
14:34atoChances are that an IPC call to the browser for checking document.hidden will incur enough overhead that the DOM property will have been updated.
14:36whimboojgraham: and the error code as written out is wrong :/ 0 instead of 1 for a crash of that specific type
14:39maja_zfjgraham: is there a way to run wdspec tests with jsdebugger enabled to set breakpoints in marionette code?
14:40atomaja_zf, jgraham: I guess thats technically possible if you extend the timeout sufficiently?
14:41jgrahammaja_zf: So, probably, but I&#39;m not sure how
14:41jgrahamThere is --debugger which is intended for gdb-type debuggers
14:42maja_zfContributor was asking me...
14:42jgrahamDoesn&#39;t look like jsdebugger is supported
14:42jgraham(maybe shoudl be?)
14:42atoGiven wptrunner doesnt destroy your process (on test timeout), Firefox will give you a prompt to start the tests and then break at &quot;debugger;&quot;.
14:42jgrahamActually I guess that doesn&#39;t work with wdpsec tests
14:43jgrahamBut you could alter the capabilites we pass in and hack to disable the timeout
14:43atoWhat would need to change in the session capabilities?
14:44atoI dont think -jsdebugger is a Firefox flag as such.
14:44atoI think it sets some pref or something.
14:44atoI cant quite remember.
14:44jgrahamSure, but you can pass it as an arg
14:44jgrahammaja_zf: for that
14:45atomaja_zf: And for the jsdebugger prefs.
14:45whimboojgraham: oh, so status.unwrap_or(None) moves ownership. So i tried as_ref() but that doesn&#39;t work here. any proposal?
14:45atomaja_zf: Could argue it would be useful to build those into geckodriver behind a flag there.
14:46jgrahamCould hack to disable timeouts
14:46atomaja_zf: So you could &quot;geckodriver --jsdebugger&quot; or whatever and it would override those prefs for you like it does for marionette.port in
14:46atomaja_zf: Then from wptrunner youd do &quot;--webdriver-arg=-jsdebugger&quot;.
14:46atomaja_zf: Or imply that somehow from its &quot;--debugger&quot; flag.
14:49maja_zfThat would be useful. I&#39;ll file a geckodriver bug.
14:49maja_zfto be clear, I&#39;m not implementing new tooling around wpt debugging now. I&#39;m going to distill your responses into a brief reply to a contributor&#39;s question about how to debug
14:52atoSounds useful.
15:13atoSo I tested polling document.hidden in the frame script and it seems a lot more reliable.
15:13atoI wonder however if the visibilitychange DOM event is more reliably linked to the document.hidden state.
15:31atojgraham, whimboo: If visibilitychange is something we can rely on we could add a DOM event listening service in the frame script.
15:31atoThat might be useful in the future too, and even could be something worth exposing.
15:32whimbooato: so we will also get this when switching tabs but that shouldn&#39;t harm us for teh window resizing methods
15:33whimboodo we enusre that switch_to_window is synchronous?
15:33whimbooif we return a bit too early it might interfere?
15:33atoHave we seen cases where switching tabs is problematic?
15:34atoI thought focus on the tab was a user-perceived characteristic and not something that would affect the behaviour of Marionette.
15:34whimbooswitching tabs means we also get this event
15:35whimbooThe visibilitychange event is fired when the content of a tab has become visible or has been hidden
15:36atoWell like an event listener youd be able to turn it off an on.
15:37atoStart listening for event type, await a message from the frame script with the event youre looking for, turn off the event listener.
15:37atoThen put all that in a promise that wouldnt return until a visibilitychange event is seen.
15:41jgrahamwhimboo: for as_ref() you need to s/None/&None/g
15:42whimboojgraham: the deref via *x doesn&#39;t help?
15:42whimbooor is as_ref().unwrap_or(&None) just shorter
15:42davehuntekyle: did you say there&#39;s a treeherder schema in activedata?
15:43ekyledavehunt: yes: {&quot;from&quot;:&quot;treeherder&quot;}
15:43jgrahamwhimboo: Oh that also works, hadn&#39;t seen your patch. I&#39;m not sure which is better
15:43davehuntekyle: fyi it&#39;s not listed under schemas on
15:44ekyledavehunt: thanks,I will add a link to that
15:45ekyledavehunt: ... and complete it; it does not appear finished.
15:45ekyledavehunt: ... so assume it is not the full truth, for now
15:46ekyledavehunt: You may notice duplication of jobs: When details on a job is updated, a new record is created, and the old one sticks around. I never go around to retiring the old record because I was the only one using this table.
15:47whimboojgraham: i would leave it for your judgement
15:47ekyledavehunt: but that will not affect the task cluster id to job number mapping
15:47davehuntekyle: no problem.. so I search based on the task id?
15:48ekyledavehunt: yes
15:55whimboojgraham: can you also review the 2nd patch which is the vendor patch
15:57jgrahamwhimboo: Done
15:59whimboojgraham: thanks! will do another try to be sure
16:03atojgraham, whimboo: From testing with a non-opt debug build, it looks like the only reliable solution is to query the DOM document. Im not super-thrilled about having to do IPC for this, but it appears to give consistent results.
16:06whimbooato: so this event does not work?
16:07atowhimboo: The sizemodechange event does work, but the DOM properties arent updated synchronously.
16:07whimbooato: maybe would be an alternative?
16:07whimbooso no polling
16:08atoSo when the next command does ExecuteScript(&quot;return document.hidden&quot;) that reports false, even though the window is hidden.
16:09atowhimboo: So for content, polling on document.hidden and visibilitychange works fine. They both give reliable results.
16:09whimbooMutationObserver would be cleaner IMO
16:09atoIf we have to go into the frame script, Id prefer the no-polling option.
16:09atoYou cant use a MutationObserver on document.
16:10atoIt only works for attributes on elements.
16:10atoAnd I dont see the advantage of a MutationObserver vs. visibilitychange.
16:14whimboovisibilitychange is an event
16:14whimboois more polling document.hidden vs using a MutationObserver
16:35atowhimboo: I dont understand your point.
16:35atowhimboo: Ive written a summary in
16:35bugbotBug 1397007: geckodriver, normal, nobody, NEW , Intermittent /webdriver/tests/ |
16:37jgrahamwhimboo: You can&#39;t detect document.hidden changes using a mutation observer because it doesn&#39;t mutate the document tree
16:37jgrahamwhimboo: events are not polling by definition, so I&#39;m not sure what you&#39;re saying there
16:37atoI think I said that earlier too.
17:11whimbooato, jgraham sorry missed that single line from Andreas earlier
17:25atowhimboo: What do you think about option 3 in
17:25bugbotBug 1397007: geckodriver, normal, nobody, NEW , Intermittent /webdriver/tests/ |
17:26atowhimboo: Its going to incur some complexity, so Id like buy-in from at least you or someone else before I start implementing that.
17:26atoBut Ive made a proof of concept that shows the intermittent is fixed, so its promising.
17:33atomaja_zf: Do you agree would relatively simple to fix?
17:33bugbotBug 1393831: Marionette, normal, nobody, NEW , pointerMove action calculates wrong default element centre point
17:33atomaja_zf: I was thinking about adding a good-first-bug flag to it.
17:34maja_zfato: yes
17:38atoIf I remember how to
17:38whimbooato: ask for ni? please. I have to head out soon
18:05bkellycan someone here help me get symbols like this in my MOZ_ASSERT stacks when running WPT locally?
18:05bkellyMINIDUMP_STACKWALK doesn&#39;t seem to work
18:07jgrahamWhilst I know I ought to be able to help, this is usually when you hear me run for cover, whilst shouting &quot;ted, someone asked for you&quot;
18:08tedbkelly: is this from a local build?
18:08bkellyted: what I linked is a try build...
18:08bkellyted: when I run it locally I don&#39;t get symbols in the stack
18:08tedbkelly: right, i mean, &quot;are you trying to do this in a local build&quot;
18:09tedso those are from assertions, and we have some python code that takes the assertion stacks that firefox prints and fixes the symbols
18:12bkellyok, I vaguely recall this... is there any page I can read on this?
18:13tedthe thing is the harness ought to be doing things such that this Just WOrks
18:13bkellythat would be ideal, yes
18:14tedlike mochitest does
18:15tedwpt does seem to do this:
18:15bkellyted: yea, seems I have to set stackfix-dir to something
18:15jgrahamYEs, I think that&#39;s right
18:15bkellyautomation sets it to: /builds/worker/workspace/build/tests/bin
18:16bkellyI don&#39;t know whats in there, though
18:16tedit has a sensible default
18:17bkellyit seems these stack fixup scripts are separately downloaded from m-c?
18:17bkellythis gives me nothing in my m-c dir: find . -name &#39;stackfix&#39;
18:18tedno, they live in tools/rb
18:18jgrahamIt&#39;s looking for something called fix_linux_stack
18:18tedthey get installed to dist/bin
18:18jgrahamOr fix_macos_stack
18:19jgrahamOr something else on Windows
18:19tedi think i see hte problem
18:19tedthere&#39;s no default for symbols_path
18:19tedso that `if self.symbols_path and stackfix_dir:` bit fails
18:20jgrahamIt could perhaps skip the if self.symbols_path check?
18:20bkellyted: I don&#39;t have a tools/rb
18:20jgrahamSeems like that might not be essential
18:20bkellyoh, I do
18:20tedjgraham: yeah, get_stack_fixer_function is OK with it being None
18:21tedbkelly: try changing this line locally to remove the symbols_path bit:
18:21tedi think that should make it work
18:21jgrahambkelly: Want to make a patch? You have the advantage that you can test it
18:22tedthe fix_{linux,macosx}_stack scripts use addr2line or whatever to symbolicate from local debug symbols
18:22bkellythat seems to fix it for me
18:22bkellyjgraham: sorry, but I&#39;m already in the middle of trying to fix something
18:23jgrahambkelly: OK :(
18:24bkellyjgraham: I mean, I don&#39;t have an upstream repo cloned, etc
18:24bkellyto do a PR
18:26bkellyjgraham: here is the patch I made locally:
18:28jgrahambkelly: You would just patch in mozilla-central
18:28jgrahamBut OK, I will try to remember to do this tomorrow
18:29bkellyjgraham: oh, I thought this was maintained upstream
18:29bkellyif its maintained in gecko I can write a bug and submit my patch
18:32bugbotBug 1399214: web-platform-tests, normal, bkelly, ASSIGNED , ./mach web-platform-tests does not fix assertion stacks locally
18:57jgrahambkelly: Thanks!!
18:58jgrahambkelly: This has tw way sync with upstream like the tests
18:59bkellyah, cool
19:56davehuntekyle: earlier it was suggested to use treeherder.machine.platform for filtering/display, but there are a lot of tests without a value here.. do you think it&#39;s okay to just exclude these (they&#39;ll be accessible by filtering to &quot;All&quot; platforms)
20:55ekyledavehunt: there are lots of tasks that are not tests. if you have a test with no &quot;treeherder&quot; info, please tell me about it
20:56ekyledavehunt: I can also work on ensuring the build.platform is filled with treeherder.machine.platform
20:56davehuntekyle: isn&#39;t everything in the unittest schema a test?
20:56ekyledavehunt: oh, yes, I thought you were looking in the task table
21:00ekyledavehunt: hmm, it would seem the buildbot test results do not have treeherder annotations
13 Sep 2017
No messages
Last message: 9 days and 13 hours ago