mozilla :: #content

17 Mar 2017
00:00mccr8all this talk of shaking reminds me of the mozvibrate exploit we had.
00:00* bkelly doesn't feel as confident in the security of our product...
00:00ehsanhaha
00:00ehsanmccr8++
00:00smaughow is ehsan's review queue?
00:00ehsanshoot
00:00ehsannot great
00:00* smaug has a patch coming for a security bug ;)
00:00ehsanok
00:00ehsansecurity bugs are always welcome
00:01ehsancrap, speaking of which
00:01* ehsan forgot to review another one today :(
00:01qDotmozvibrate wouldn't have been exploitable if they'd have let someone with domain knowledge write it. Just saying.
00:02ehsansmaug: is yours a difficult review?
00:02smaugno
00:02ehsanok
00:02smaugthis is trivial. But I need to add still similar checks in couple of other places
00:05bkellyugh... I hate the random crap lenovo installed on this PC
00:05bkellydo I trust "CyberLink ISO Viewer"? probably not
00:05ehsanbkelly: I filed a servicenow today for a machine with ubuntu preinstalled :P
00:05ehsansorry to rub it in ;)
00:06bkellyehsan: well, they told me this would come with 1TB SSD, but it didn't... so good luck
00:06ehsanuh oh
00:06bkellyehsan: also, I have the ubuntu install dvd sitting on my desk
00:06ehsanbkelly: that pleasure was short lived!
00:06ehsanlol
00:06ehsanbkelly: good one
00:06ehsanI deserved that!
00:06bkellybut that is destined for my old windows pc
00:06ehsanoh
00:06ehsanwhat's the bigger goal?
00:07bkellyI want to use the fast machine for my main desktop... I like windows for day-to-day
00:07bkellymy old desktops are my icecc cluster
00:07bkellyand a linux VM running on my windows PC is also in the icecc (not all the processors, though)
00:08qDotbkelly: Does the linux vm icecc still help much? I've got windows on home theater PCs around the house...
00:08ehsanI see
00:08ehsanyou got a pretty nice setup going on!
00:08ehsannow that I will run linux as a primary OS, I can also start investing into making a real development environment
00:09bkellyqDot: I haven't tried it on my new faster PC... but I got 4 jobs out of it in my old setup... seems to drop my clobber times noticeably... from 20+ mins to ~14min
00:09ehsanlike with connecting to the icecc cluster at the office when I'm there
00:09ehsan(and even also maybe when I'm not)
00:09qDotbkelly: My new desktop should show up next week, might give that a shot. Thanks!
00:09bkellyI actually use one of the older desktops as my main development environment... I ssh in and use remote X when I need it
00:09ehsanwe'll see about latency
00:10bkellyI can launch my main desktop VM if I want to add it to the icecc... or shut it down if I want to do a simultantous windows build
00:10ehsanbkelly: these days I also ssh into my linux desktop machine at the office a lot similarly
00:10bkellyI plan to ssh into my box while on travel in a few weeks... have to see how that goes
00:10ehsanrunning vnc as needed
00:10ehsan(and oftentimes no vnc!)
00:11bkellyehsan: I almost never need an actual display... xvfb-run ftw
00:11ehsanbkelly: what does that do?
00:11bkellyehsan: xvfb-run?
00:11ehsanyeah
00:11* ehsan is an x n00b
00:12bkellyehsan: "X virtual frame buffer" it creates a fake X environment... all our automation uses it
00:12bkellyso you can run headless
00:12ehsanhrm
00:12ehsanI run headless without it
00:12bkellyehsan: you may actually have an X environment on your machine?
00:12ehsanyeah
00:12ehsanI have a normal ubuntu desktop
00:12ehsanoh
00:12ehsanI do export DISPLAY=:1
00:13ehsanso I get a firefox window on my desktop monitor
00:13ehsanwhich nobody sees!
00:13bkellyehsan: if you want to try xvfb-run, the command line I use is "xvfb-run -a -s '-screen 0 1600x1200x24'"
00:13* ehsan copies that
00:13ehsanbkelly: can you see that virtual buffer somehow?
00:13bkellyehsan: mach started requiring the manual screen size setting... and some tests in our suite fail if its not 1600x1200x24
00:13ehsanwow!
00:13bkellyehsan: I don't think so
00:14ehsanwhy can't mach pass the right thing?!
00:14tbsaundewell, mach is running as a child of xvfb
00:15ehsanah
00:15ehsanof course!
00:15tbsaundeand presumably for some reason mach has expectations about the screen in use
00:15bkellytbsaunde: it used to work without that for some reason... but then started depending on explicit site at one point
00:15ehsanbkelly: so I think my setup has the slight advantage of making vnc possible on the off chance that you need it
00:15tbsaundeI don't know what the default is, bug 640x480 wouldn't suprise me, and I can believe modern stuff doesn't expect that
00:16bkellyehsan: sure... I have been using x windows over ssh tunneling for that
00:16ehsaneven though I often don't need to
00:16ehsanbkelly: oh right yes you mentioned that
00:16tbsaundeehsan: yeah, you can do both depending on situation though ;)
00:16bkellyehsan: works well with vcxsrv on windows... xquartz on mac seems to suck, though
00:16ehsanbkelly: yeah xquartz is really bad :(
00:17ehsanbkelly: that vcxsrv is good is actually really good news
00:17tbsaundeI like being able to use my desktop for actual browsing, but some tests don't like xvfb so sometimes one or the other
00:17bkellyehsan: well, by "good" I mean its doesn't crash repeatedly like xming
00:17* ehsan wouldn't run using a windows machine semi-regularly with that capability
00:18ehsanbkelly: quartz just doesn't start at all half the times...
00:18ehsanerr
00:18ehsanxquartz
00:26bkellynext pwn2own against firefox should complete soon I think
00:26bkellysupposed to have started about 25 minutes ago
00:31ehsanyeah
00:31ehsanend of the hour
00:32mccr8it was successful. we're waiting for the test case.
00:33* qDot not sure whether to "yay" or not.
00:40bkellyI just want someone to say its svg so I can go to bed
00:41mccr8service verkers
00:42bkellyreally?
00:42mccr8no
00:42mccr8I was trying to come up with an acronym for SVG but got lazy before I came up with anything for G.
00:42bkellybecause, I would believe it
00:42mccr8Yeah, it is a complicated feature that is fairly new.
00:42bkellyservice verker garbage
00:42mccr8There you go.
00:43* bkelly sees lenovo spams windows notification advertisements periodically...
00:44tbsaundereally svg again? I guess it is complicated and not that often used
00:44bkellytbsaunde: we don't know yet
00:45tbsaundeah, thought mccr8's there you go was reffering to having filed something
00:45mccr8tbsaunde: No, sorry, just joking around.
00:46tbsaundeno worries
00:50bkellyvisual studio installer is just sittintg there using no cpu/disk io or anything
00:50bkellywtf is it doing
00:58bkellytantalizing pwn2own bugmail
01:02ehsansmaug: r+
01:02ehsanbkelly: what is it?
01:02smaugthanks
01:03bkellyehsan: I dunno... its private now
01:07ehsangfx
01:07ehsanerr
01:18bz_dinnerhmm. Failure on the 4pm thing....
01:18bz_dinnerBut doesn't mean there isn't a bug....
01:21smaugthis bug is silly
01:22* qDot kinda wishing most of his work didn't deal with outside drivers that are just asking for this sort of thing.
01:22ehsanyeah
01:22ehsaneasy
01:22ehsanwe had a static analysis bug about it :(((
01:24smaugwe did?
01:26ehsansmaug: yes
01:26ehsan1279569
01:27bz_dinnerehsan: care to cc me on 1279569 ?
01:27smaugI don't have access to that
01:27ehsan(I haven't made sure it would have found this oarticular one...)
01:27ehsantry again
01:27ehsansorry
01:27ehsanlet me get to fixing the bug :)
01:33bz_dinnerehsan: :(
02:38bz_dinnerhmm
02:39bz_dinnerSo this pwn2own sure had a lot of integer overflow bugs
02:41heycambz_dinner: hooray for more rust usage then?
02:41ehsanbz_dinner: you here?
02:41ehsanI need a DOM peer
02:42ehsannot having dinner earlier was a mistake...
03:02overholtehsan: do you need me to order some food to your place?
03:02ehsanoverholt: heh, no
03:02ehsanand most places are closed already!
03:02ehsanthanks though
03:02* ehsan should be done soon
03:03overholtNew Ho King is open all night, I think
03:03overholtnot that I've eaten there in a very long time
03:05ehsanI'll be making yummy dinner after this :)
03:05overholtehsan: ttyl
03:05ehsanyep
03:07bkellyI think my new desktop is mostly setup
03:07bkellywindows build seems only a few minutes faster unfortunately
03:07bkellymaybe thats because I was doing other things...
03:08ehsanbkelly: hey
03:08ehsando you remember how to get a JSContext* in a worker?
03:08bkellyehsan: we have a method for that...
03:08ehsanfor the current caller
03:08ehsanyeah
03:08* ehsan doesn't remember what it's called
03:09bkellyjust a sec
03:09ehsanfound it!
03:09ehsanbkelly: nm
03:09bkellyehsan: https://dxr.mozilla.org/mozilla-central/source/dom/workers/RuntimeService.cpp#1479
03:10bkellyehsan: note that will assert on main thread, though
03:10ehsanok
03:10ehsanthanks
03:10bzehsan: here now
03:10bzehsan: Still need something?
03:10ehsangreat
03:10ehsanbz: a review
03:10bzOK
03:10bzIn 1348168 ?
03:11ehsanbz: yeah
03:11ehsanbuilding still
03:11bzOK
03:11ehsancan you join the security channel?
03:11ehsanbz: ^
03:11* bz mutters about the bug that added this thing having so much mozreview noise it was impossible to tell that what landed didn't match what was reviewed
03:11bzchecking
03:12bz#security?
03:12mccr8no
03:12bzThen "probably not"
03:12* bz can't find the page with the pin
03:13* bz goes to find his ssl IRC client
03:17bz_nosslehsan: done
03:28bkellywoah.... I excluded some more directories for the virus scanner and my build is running twice as fast
03:28bkellyexcluding my entire user directory from virus scanner is probably not the best, though
10:13annevkbkelly: are virus scanners actually useful?
10:18jgrahambz: That mirroring is now fixed btw. w3c-test.org periodically runs out of inodes or disk space
10:18jgrahamEven though we supposedly clean up after PRs are closed
10:18annevkThat it runs out of space is fixed?
10:19jgrahamNo, the fact that some PRs were unmirrored is fixed. The underlying problem isn't
10:20annevkGood times
12:54smaugfarre: I'm a bit lost with the patches now
12:55farresmaug: right.
12:55smaugfarre: like https://bug1318720.bmoattachments.org/attachment.cgi?id=8848451
12:55firebothttps://bugzil.la/1318720 NEW, afarre@mozilla.com Prevent chained requestIdleCallbacks' idle callbacks from being executed during the same idle period
12:55smaugit talks about "Don't allow calling rIC on detached"
12:56smaugbut seems to be doing something with throttled callbacks
12:56farresmaug: right. it fixes the leak that we talked about. it could drop those patches and take them through their own bug if it helps
12:57smaugfarre: the throttled callback counter thing is the leak fix?
12:57farresmaug: the counter thing isn't the leak fix. but it drastically cuts down on the number of IdleRequestExecutorTimeoutHandlers
12:58farresmaug: down to one per executor
12:59farrethe actual leak fix is the change to IdleRequestExecutor::IsCancelled
12:59farrei.e. don't schedule the executor if we've freed inner objects
13:00smaugfarre: could you upload a new version of that patch with -U 8
13:00smaugit has so little context that reviewing is hard
13:00farreright, 0004?
13:00smaugfarre: ok, the leak fix was in different patch
13:01farresmaug: or the interdiff?
13:01smaugfarre: right, https://bug1318720.bmoattachments.org/attachment.cgi?id=8848451 has only 3 lines context
13:01smaugor maybe 4
13:02smaugfarre: so https://bug1318720.bmoattachments.org/attachment.cgi?id=8848453 is the leak fix, right?
13:02farrenope. the leak-fix is in 4:
13:02farre- bool IsCancelled() const { return !mWindow; }
13:02farre+ bool IsCancelled() const { return !mWindow || mWindow->AsInner()->InnerObjectsFreed(); }
13:03farreso 4 changes a line introduced in 1
13:05farresmaug: every change except ^ in 0004 doesn't fix the leak, but makes memory behaviour _much_ better
13:05farreand played a big part in me finding that one-liner
13:06smaugoh, we want the current inner window check there
13:06smaugI think
13:08farreright, how do I do that?
13:08smaugperhaps HasActiveDocument() call ?
13:09farreI'll try
13:09smaugI wonder
13:09smaugwith document.write
13:09smaugfarre: maybe it should be IsCurrentInnerWindow()
13:10smaugthinking
13:10smaugwhat does the spec say... are the callbacks bound to window or document
13:10farresmaug: good thing there is a wpt test to check against :)
13:10farreto window
13:11smaugfarre: ok, then IsCurrentInnerWindow
13:12farreand I have a bug in the test as well
13:13farreforgot to add a blank.html
13:14smaugfarre: oh, do you need that
13:14smaugfarre: couldn't you use about:blank ?
13:15bkellyannevk: I think Windows Defender is probably worth it
13:15smaugfarre: hmm
13:15bkellyI wouldn't install a 3rd party anti-virus
13:15smaugfarre: is cancelled only for the case when window really is gone?
13:15smaugfarre: since IsCurrentInnerWindow would return false in bfcache
13:16farresmaug: for Suspend we cancel the executor anyway
13:16farreand then mWindow in the executor will be null
13:16smaugfarre: right. But I wonder if we need IsCancelled at all
13:17smaugor just use IsCurrentInnerWindow in nsGlobalWindow code
13:18* farre is thinking
13:18smaugfarre: or perhaps we do need it, in case mWindow is null
13:18smaugso return !mWindow ||!mWindow->IsCurrentInnerWindow() ?
13:19smaugassuming that works still bfcache
13:19farretrying out now. fortunately there's wpt test for bfcache as well :)
13:26farresmaug: IsCurrentInnerWindow doesn't work
13:26smaugfarre: ok
13:27smaugfarre: so IsCurrentInnerWindow() returns true when InnerObjectsFreed() returns true?
13:27smaugsurprising
13:30farreyep. double checked
13:39smaugfarre: so I'm trying to understand how this mThrottledExecutor works
13:40smaugI would have expected it to live in nsGlobalWindow
13:42smaugfarre: and how is mIdleCallbackTimeoutCounter kept in sync with mThrottledIdleCallbackTimeoutCounter so that same id isn't used with many callbacks?
13:43farresmaug: it definetely could, and IdleRequestExecutor could also actually have been the actual TimeoutHandler as well. and really the only reason is that at some point I'm going to move IdleRequestExecutor out of nsGlobalWindow and then it would be nice to keep them together.
13:43farresmaug: mThrottledIdleCallbackTimeoutCounter is used internally only
13:43smaugthis is rather confusing
13:44bkellythat would be a good motto
13:44bkellyGecko: Rather Confusing
13:44farreoh, wait. mIdleCallbackTimeoutCounter is also only used internally.
13:44farreright
13:44farresorry. unecessary change then
13:44smaugbkelly: s/Gecko/Web/
13:45bkellysmaug: yea, someone debating a web spec issue said it was a gross oversimplification
13:45bkellykept thinking we should just put that as a subtitle on html spec...
13:45bkellyHTML Specifiction: A Gross Oversimplifcation
13:45smaug:)
13:45farrebkelly: :D
13:45smaughopefully without typos
13:46farresmaug: I'll remove the new counter. it is bogus
13:46smaugfarre: thanks
13:47smaugI need to still figure out where we get the actual id for the callbacks then
13:52smaugit is confusing if TimeoutManager::GetTimeoutId isn't the method to generate the id
13:52smaugoh, but that is for timeouts only I guess
13:52farreyes
13:53farreand there are two timeouts for rIC. the first that is externally controlled by the options object passed to rIC
13:53smaughmm, searchfox has been behaving less well recently
13:53smaugit doesn't find things
13:53farrethe other is for throttling, both for background and chaining
13:55smaugoh silly me, searchfox tends to forget the case-sensitive setting
13:57padenothow can I set two prefs during a crashtest ?
14:15bkellykind of sad this 16 core system still takes 18 minutes to do a clobber build on windows
14:18froydnjthat's...pretty bad'
14:19bkellythats even with disabling anti virus compeltely
14:19froydnj20 cores spread through icecc does a linux build in half that time here
14:19bkellyit seems we have some multi-minute stalls at the end building the js lib
14:19bkellyand linking firefox of course takes minutes
14:20froydnjthat seems like we're doing something wrong
14:20froydnjlinking on linux doesn't take nearly that long
14:21bkellyfroydnj: there are a bunch of bugs about the visual studio linker
14:21bkellyfroydnj: although the js lib multi-minute pause is in a single cl.exe, not link.exe
14:42smaugfarre: you need to explain mThrottledExecutor usage for mme
14:42smaugme
14:42farresure
14:43smaughow is that related to the normal executor
14:43smaugcan they both be active at the same time?
14:43farreno
14:43farrethey can't
14:43farrethe normal executor is the runnable that runs on the nsThread (on the idle queue)
14:44farreand that's for the case when we want to run as soon as possible
14:44farremThrottledExecutor is for as soon as possible, but not before some time t
14:44farrewhere t in the window in the background case is the same time that we throttle regular setTimeouts to
14:45farrein the foreground mThrottledExecutor is used when we've executed all idle callbacks that can execute in the current idle period
14:45smaugwhat?
14:46smaugI didn't expect that at all
14:46smaughow is that throttling
14:46farrewell. yess
14:47farreI can agree on the name not being a 100% here. in some sense we're throttling idle callbacks when we disallow them from running during the current period (due to reasons)
14:47farrewould mDelayedExecutorDispatcher be a more reasonable name?
14:47smaugbut we use "throttling" in very different meaning normally
14:48smaugit is confusing to use the same thing for foreground and background, I guess
14:48smaugsince those are totally different cases
14:48smaugmDelayedExecutorDispatcher might be a tad better yes
14:49bkellyfroydnj: my 18-core icecc does a full linux clobber in 10.5 minutes
14:49bkellyI have another 4 cores I can add to that once I find time to install linux on my old desktop
14:49farresmaug: but an important thing to remember, whether on the idle queue or in the list of timeouts, the executor is still considered to be dispatched in both
14:50farreso there can only ever be one mDelayedExecutorDispatcher, and if the executor is on the idle queue then mDelayedExecutorDispatcher isn't in the list of timeouts and vice versa
14:52smaugfarre: does it cause any issues if we have active mDelayedExecutorDispatcher from foreground and then switch tab to background?
14:54farresmaug: that should behave ok I think. the executor will still be considered to be dispatched, and we'll handle dispatching it in the same way as we handle dispatching timeouts for that case, which should be acceptable
14:55farreand for bfcache, the timeout handler checks if the executor is cancelled, and returns without scheduling it
14:55farreso that should also be ok
14:56smaugfarre: another question, why ThrottledDispatch takes now signed int?
14:58farreI worried about aDelayUntil < now, and that I&#39;d get a wrap
14:58farreso by changing to int I can assert that it is > 0
14:59farre>= I should say
14:59smaugdoes the assert help anything?
14:59smaugI mean, it just catches issues in debug builds
15:00farreI can&#39;t happen now either
15:00farrethere&#39;s a check in MaybeDispatch for that
15:00smaugwhat happens is someone passes real high timout value to requestIdleCallback?
15:00farreso yes. I&#39;ll change it back to uint
15:01farreand I&#39;ll rename ThrottledDispatch to DelayedDispatch while I&#39;m at it
15:02smaugfarre: would be worth to test different browsers when max_int32_t + 1 is passed as timeout value
15:04jdmhow did pwn2own go last night?
15:04AutomatedTester1/2 expliots worked
15:04AutomatedTesterwell... in the time alloted
15:08mccr8ehsan++
15:08farresmaug: ~600h worth of timeout :)
15:09farresmaug: I&#39;ll start the test now and come back with the result in april
15:09smaugfarre: well, just ensuring we don&#39;t end up handling the value as int32
15:09smaugbut uint32
15:09smaugand test should be done in debug build
15:09farresmaug: and somehow block any idle periods I guess
15:18farresmaug: since again we use setTimeout we at least won&#39;t accept negative values
15:19smaugfarre: sure, but wouldn&#39;t you patch have asserted in that case?
15:19farrenope, those timeouts are set throut nsGlobalWindow::RequestIdleCallback
15:19smaugaha
15:22farreand this is also a really good reason for moving all rIC related (with the exception of nsGlobalWindow::RequestIdleCallback of course) stuff into IdleRequestExecutor.{h, cpp}. will be a lot easier to get an overview of what&#39;s going on
15:31tbsaundemystor: from the department of evil ideas generating nsresult.rs with cpp from ErrorList.h probably isn&#39;t that hard
15:31mystortbsaunde: the c preprocessor adds #line directives which would have to be stripped
15:32mystortbsaunde: /me may have tried this before and decided it was too annoying to do
15:32tbsaundemystor: ah yeah arg
15:32tbsaundestill that file might be pretty easily parsed with python which is less terrible anyway?
15:32mystorBut yeah, if we did cpp -> sed script
15:33farresmaug: thanks for the review!
15:33mystortbsaunde: What I was thinking of doing was writing a python script which created the nsresults in python, then wrote out 2 side-by-side files, one C++ header and one .rs file
15:34tbsaundemystor: yes, that would be the sanest ;)
15:39froydnjusing that .rs would be a little awkward, I think; might have to resort to &quot;check in the generated file and ensure that it doesn&#39;t change unexpectedly&quot; like we do for several other files
15:39froydnjor stick everything in a JSON or something and have a build.rs file take that apart
15:42tbsaundethat would basically be s/python/build.rs/ or just run python from build.rs
15:43froydnjbuild.rs gets run at the wrong time to generate a C header for the rest of the build to depend on, though
15:44tbsaundeeasy enough to run the script twice though
15:44tbsaunde--c++ and --rust or some such
15:50bkelly8 minutes for a clobber build with that extra machine in the mix
15:50padenotbkelly, on windows ?
15:51bkellypadenot: no... linux icecc with 26 jobs available... my new 16 core system does windows clobber in 18 minutes
15:51padenotyeah
15:51padenotyeah I&#39;ve following
15:51padenotbeen
15:51bkellypadenot: probably 5 minutes of that its mostly idle in single core usage
15:51padenotbkelly, you mean the linux build ?
15:51padenotlld helps a ton, I&#39;m at 3:30 now
15:51bkellypadenot: no... windows build... there are many minutes where its not getting any paralellism
15:52bkellypadenot: and its not all link.exe
15:52padenotyeah, I hear that everywhere
15:52bkellyanyway... its an improvement over before... so I&#39;ll take it
15:52padenotsure thing
15:52bkellyand I might be able to turn off the heater in my office
15:53bkellywith 3 machines running in high perf power profile
15:53padenottry to compile chromium once, it gives you a sense of perspective
15:53padenot(1h30 on my 12 core xeon)
15:54bkellyit amazing they get anything done at all
15:55bkellypadenot: are they doing PGO on every build or something?
15:55bkellyI saw on blink-dev they are thinking of trying unified build like we do
15:55padenotno they use goma
15:55padenotas in, googlers do
15:55bkellygoma?
15:56padenotthey ridiculous google-scale distributed compilation stuff
15:56bkellyah
15:56padenottheir*
15:56bkellysteps to contribute... install google-scale compute cluster
15:57padenotI do prefer our approach here for sure
15:59bkellypadenot: I may be able to speed my linux situation slightly... 8 of more cores are running in a VM that I can likely move to linux install on the bare metal
15:59bkellyprobably marginal gain, though
15:59padenotlld is awesome though
15:59padenot4 second link
15:59padenotseconds
15:59bkellypadenot: are their instructions on using lld in our build? any downsides?
16:00padenotbkelly, Sylvestre has a patch, but he needs to find time to finish and land it
16:00padenotit would be something like --enable-lld in mozconfig
16:00padenotno downsides I found
16:00bkellypadenot: do I have to install lld separately I assume?
16:01bkellypadenot: this bug? https://bugzilla.mozilla.org/show_bug.cgi?id=1336978
16:01padenotbkelly, yeah, he also has prepared packages for debian based systems, not sure what you use
16:01firebotBug 1336978 NEW, sledru@mozilla.com Add --enable-lld to link with lld instead of ld
16:01padenothe&#39;s the maintainer of llvm stuff for debian
16:01bkellyI&#39;m on ubuntu all around
16:01padenotbkelly, yep that&#39;s the one
16:01padenotbkelly, same, it&#39;s like a three liner copy/paste to install it cleanly
16:02padenothttp://apt.llvm.org/
16:02bzOh, ffs
16:02bkellypadenot: thanks for the tips... I&#39;ll have to try that some time... probably burned enough time fiddling with my config today, though
16:03padenotbkelly, yeah, also it&#39;s going to be real simple when he&#39;s done, I was just being curious when I tried it
16:03* bz curses wpt manifest
16:03bzjgraham: We really need to do something about this...
16:04jgrahambz: :/ Which failure mode did you hit this time?
16:04bzjgraham: person A lands something without updating all the fiddly hash bits (how did they edit the manifest?)
16:05bzjgraham: person B uses some form of manifest-update and lands on autoland
16:05bzjgraham: person C uses some form of manifest-update and lands on inbound
16:05bzjgraham: sheriffs back out person C when they go to merge
16:08jgrahambz: Do you have any ideas beyond &quot;don&#39;t do the hash stuff&quot; and &quot;rewrite everything in Rust and put it in the build system&quot;?
16:09bkellyjgraham: maybe just update the hashes when you do the upstream/downstream merge?
16:09bzSo what use case are the hashes solving?
16:09bkellyhashing things is fun!
16:09bzI&#39;d like to undestand that before I can say something intelligent.
16:09jgrahamThe use case is to make incremental updates fast
16:10jgrahamSo that you don&#39;t have to reexamine files that didn&#39;t change
16:10bzincremental updates like when someone does wpt-manifest-update?
16:10jgrahamYeah
16:10jgrahamOr |mach wpt -manifest-update|
16:10bzok
16:10bzsure
16:11* bz makes the mistake of loading MANIFEST.json in searchfox, watches it hang his browser
16:12jgrahamOnce we have the CSS bits everything will be even worse (because there will be many more files)
16:12bkellybz: probably the browsers fault there...
16:12bzbkelly: well, yes
16:12* bz should measure what&#39;s going on there
16:12bzjgraham: So is there some way we can prevent people committing, or pushing, with bogus wpt manifests?
16:13* bz pokes around
16:13jgrahambz: So the lint is supposed to do that, but it doesn&#39;t check the hashes
16:13jgrahamBecause it was way too annoying when it did
16:13bzSo https://reviewboard.mozilla.org/r/117728/diff/5#index_header
16:13bzThis _renamed_ a bunch of stuff in the manifest, but not in the hash bits....
16:14bzjgraham: Here&#39;s a question
16:14bzjgraham: for manifest-building purposes, the one part we care about changing or not is &quot;is this file still there and still a test file of the same type&quot;, right?
16:15jgrahamYeah, that sounds roughly right, although some metadata like whether it has a long timeout also goes in the manifest
16:17bzhmm
16:17bzSo I was considering recording the hash of just the first N bytes
16:17bzTo reduce hash churn
16:17bzBut I guess that might be fragile....
16:18bzIn an autoland workflow, as long as we do commit rewriting (e.g. on the message)
16:18bzwe could run manifest-update as part of the autolanding, right?
16:18bkellybz: jgraham: could we store the hashes in local temp storage instead of in tree... so repeated manifest updates are fast, but first one takes full time
16:19jgrahambkelly: I was just wondering the same
16:19jgrahambz: That could also work
16:19bzbkelly&#39;s suggestion might be nice...
16:19jgraham(note that &quot;full time&quot; is probably minutes once we have css tests)
16:19bkellyreally its derivative data... belongs in obj or auto-generated on land it seems
16:19bzhrm
16:20bz&quot;minutes&quot; is annoying for clobber build time purposes. :(
16:20bkellyjgraham: tree could have a slightly stale version of the hashes that are updated separately from main manifest
16:20jgrahamYeah, it&#39;s not ideal to put any of it in-tree. But it&#39;s slow to generate
16:20bkellyre-gen the hashes in the tree once a day
16:20bkellyor download them as an artifact
16:20asuthdoesn&#39;t git or hg already know the hashes?
16:21jgrahamInteresting thought, but you still need to store the last processed hash somewhere
16:21jgrahamIt&#39;s not generating the hashes that&#39;s slow per-se it&#39;s generating the other manifest data
16:21bzwell
16:22jgrahamWhich the hashes let you skip
16:22bzIf we were willing to tie this to a VCS
16:22bzwe could simply store the last rev as of when the manifest was known up-to-date.
16:22bzBut there are problems.
16:22bzIn the sense that we don&#39;t actually know that the manifest is up-to-date.
16:23jgrahambz: That&#39;s sort of how it used to work, except not very well
16:23bkellyI kind of feel that making this like an &quot;artifact build&quot; is the way to go
16:24bkellypull the hashes from last parent rev in taskcluster to prime local copy
16:24bzThis is pretty silly
16:24bzClearly the manifest was updated by someone _yesterday_ given the merge conflicts
16:24bzI just ran manifest-update and there are more updates that should be there but are not in-tree. :(
16:24bkellyor... get rid of manifest updating and make people manually add an entry in a file like we do with mochitests
16:25asuthI like that idea. I understand updating .ini files. WPT updates fill me with dread.
16:25bzbkelly: The entries are not that simple to add
16:26jgrahambkelly: I think I like the idea where TC generates the manifest and uploads it, local copies download that manifest into the objdir if they don&#39;t already have one, or generates one from scratch if there&#39;s no network
16:27jgrahamasuth: :/ Avoiding people having to maually update ini files was really the goal since the test files contain all the information about what&#39;s a test. I&#39;m not sure that updating ini files is so well understood outside Mozilla
16:29jgrahamYou aren&#39;t really supposed to do anything by hand. Just running the test harness could update the manifest for you these days (since the hash stuff makes it reasonably fast)
16:29asuthcould the manifest be sharded out like the .ini files are so that conflicts are potentially reduced?
16:30asuthOr rather, I can avoid committing changes to things I know aren&#39;t from my patch?
16:30jgrahamThat makes some sense, but there&#39;s other infrastructure reading these files so I&#39;m not sure how easy it is in practice
16:32asuthAh well, I like the idea of autoland just being in charge of generating fix-up commits that it tacks onto the push. So someday!
16:36jgrahambkelly: Added your suggestion to bug 1333433
16:36firebothttps://bugzil.la/1333433 NEW, nobody@mozilla.org Consider making the wpt manifest a product of the build system
16:36bkellythanks!
16:37bobowensmaug: ping over bug 1344465
16:37firebothttps://bugzil.la/1344465 ASSIGNED, bobowencode@gmail.com Can&#39;t submit form using post method form WebExtensions or file:// page
16:37smaugbobowen: pong
16:38bobowensmaug: my knowledge in this area is a little hazy, can you upload files directly from the form post?
16:39smaugbobowen: stuff like https://www.w3schools.com/php/php_file_upload.asp ?
16:41bobowensmaug: hmm OK maybe the test thing I&#39;m posting to isn&#39;t playing back the data
16:55bzbobowen: which test thing are you using?
16:55bzbobowen: you should probably use http://software.hixie.ch/utilities/cgi/test-tools/echo fwiw
16:59bobowensmaug: ah, I was missing the enctype=&quot;multipart/form-data&quot;, thanks
16:59bobowenbz: will do thanks
17:00smaugbz: well, not in a mochtest ;)
17:01bobowensmaug: :-) ... I&#39;ve not tried it yet, but it looks like the sjs I&#39;m currently using will work
17:28* smaug kicks MozReview
17:28smaugreally hard to review patches which remove files
17:49mccr8yeah I don&#39;t know why MozReview can&#39;t show the file that was deleted
18:06bz&quot;r-, please attach this patch to a bug&quot;?
18:06bzThough note that mozreview does expose the raw diff
18:06bzIs there a bug on it not showing those?
18:37jgrahambkelly: BTW I think the data about serviceworker in https://wptdashboard.appspot.com/t/ is relatively up to date (not sure how old or stable vs nightly)
18:38bkellyjgraham: those don&#39;t seem to mach our ini exceptions to tests: https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/meta/service-workers/service-worker
18:38bkellyfor example, all the clients* tests
18:40jgrahambkelly: Hmm, like I say I&#39;m not sure what build it is and it won&#39;t have special prefs set
18:41bkellyjgraham: we don&#39;t set any special prefs for most of those
18:41jgrahamI&#39;m hoping we can encourage the Google people to generate this for nightly, but that may take a while
18:41bkellyand at least in FF52 we don&#39;t have any exceptions for those client* tests https://dxr.mozilla.org/mozilla-release/source/testing/web-platform/meta/service-workers/service-worker
18:42jgrahambkelly: OK, I&#39;ll let the people working on this know
18:43bkellythanks
18:59smaugso in this new structure, where layout lives?
18:59smaugthat is like one of the most critical parts
19:03bzsmaug: yeah, that part was non-obvious, but I assume Visuals
19:03* bz mails mayo for clarification
19:03* smaug sent email to Mark
19:04* bz too, already
19:04mccr8The email has a full org chart attached to it so you can more detail.
19:04smaugmccr8: bah, I need to login to google and all ;)
19:04bzmccr8: you mean linked, right?
19:04mccr8I just clicked. :P
19:05bkellythe re-orgs will continue until morale improves
19:05smaugisn&#39;t re-org the normal in all the companies
19:06overholtsmaug: bz: my understanding is layout->visuals as well
19:06bzI mean, that would make sense
19:06bzIt&#39;s just the mail did not say
19:06bzhmm
19:06bzpeterv is under visuals, alright
19:06* bz shrugs
19:07bzThis mostly tells me that the org chart is whatever
19:07smaugand that DOM module owner isn&#39;t in runtime team
19:07smaugnot that MoCo teams need to map to Gecko modules at all
19:08bkellyI hope media team is called &quot;Audibles&quot; now
19:08* smaug proposes bz to become a new DOM module owner
19:08bzsmaug: That&#39;s peterv
19:08bzbkelly: media team should be called lolcats, obviously
19:09bkellybz: maybe next re-org
19:09bzsmaug: oh, I see what you meant. Nah, having the org chart map to moduler ownership is bunk
19:09bzsmaug: bholley is in &quot;visuals&quot;, but xpconnect is totally &quot;runtime&quot;. ;)
19:09smaugindeed
19:09bzsmaug: I don&#39;t think it&#39;s worth even worrying about trying to align the two in any sane way
19:10smaugtrue
19:10* bz is a style system peer and pretty sure style system is under visuals....
19:11smaugoh, masayuki is in visuals, even though he really does DOM only
19:11bzsmaug: IME is visual, right? ;)
19:11smaugeh, um.. sure if event handling is visuals
19:12smaug:)
19:12bzEvent planning can be all about the visuals. And the organization.
19:12bzHas masayuki wanted to do event planning?
19:13smaugha, he has planned events in Japan
19:31smaugand I almost thought enough reviews for this week... one more
19:33smaugbtw, the pwn2own was a rather bad case where splitting review to two different reviewers somehow missed a critical part. I totally blame myself, but it isn&#39;t clear to me how to deal with that
19:34smaugI don&#39;t expect &quot;webidl reviewers&quot; should review lots of the implementation too, but perhaps I should look at the implementation some more in the future
19:34jdminteresting
19:45smaug(the actual security bug wasn&#39;t near webidl layer)
19:53* bz_bbiab pretty much always ends up looking at the impl when asked for webidl review
19:54bz_bbiabbecause it&#39;s so common for people to misuse the stuff they get from bindings
19:54smaugsure
19:55bzThe review in that case was complicated by multiple rounds and a huge patch queue
19:55bzThere is no good way to make that work.
19:55smaugwell, actually it isn&#39;t too common, since in normal cases bindings just give sane C++ to use
19:56smaugbut yeah, lesson learnt here
19:56* bz does a double-take every time he sees people blog about PCSK9
19:58bzNot to be confused with PCKS9
19:58bzer, PKCS9!
19:58* bz can&#39;t spell his confusingly-similar acronyms right
20:01bobowensmaug, mystor: it looks like the reading actually happens in the parent for the file upload case, but I&#39;ll add more tests anyway
20:07smaugbobowen: btw, baku|away has been looking at all the ipc + inputstream handling recently, so he might want to take a look at the patch too
20:10bobowensmaug: yes, I was just chatting to him about a slightly related thing
20:11bobowensmaug: he confirmed what I was seeing, that the read happens in the parent
20:21bzfroydnj: ping
20:22froydnjbz: pong
20:22bzfroydnj: so I&#39;d like to make the &quot;are we doing a prefix match&quot; thing explicit for pref callbacks
20:23bzfroydnj: Would you prefer just making the last arg to RegisterCallback required, or having a RegisterCallback/RegisterPrefixCallback API?
20:23bzfroydnj: Figured I would get that sorted before I change a bunch of callsites. ;)
20:23* bz is more in the RegisterCallback/RegisterPrefixCallbac<