mozilla :: #content

16 Mar 2017
01:25bzsmaug: ping
01:25smaugbz: pong
01:25bzsmaug: so I was looking at this setInnerHTML profile
01:25bzsmaug: And not seeing any of the things you filed bugs on...
01:26bzsmaug: at least so far
01:26smaugreally?
01:26smaugit was largely SetAttr stuff and stuff underneath it
01:26smaugin Zoom
01:26bzsmaug: yes, ok
01:26* bz hasn't dug deep into the SetAttr bits yet
01:26smaugI focused on SetInnerHTML only, not anything else
01:27bzright, me too
01:27bzI did find https://bugzilla.mozilla.org/show_bug.cgi?id=1347737
01:27firebotBug 1347737 NEW, nobody@mozilla.org nsHtml5HtmlAttributes::clear spends a bunch of time deallocating
01:27smaugyeah, bunch of html parser stuff
01:27bzalright
01:27* bz keeps reading
01:28smaugnothing was really "this is the issue to fix and all will be good"
01:28bznsHtml5Portability::newStringFromBuffer
01:28bzyeah
01:28bzWill have to do this the hard way
01:28bzWhich is both good and bad
01:29bzThe good part is that we're not being slow because we did some one dumb thing
01:29bzThe bad part is that we did a lot of dumb things. ;)
01:31smaugor had assumptions like hashtable lookup being fast
01:31smaugor allocation being fast
01:31bzheh
01:32bzhashtable lookups are fast compared to most alternatives.
01:32bzallocation is never fast. ;)
01:32smaugmost of the small things I saw were luckily quite easy to fix
01:33smaugs/were/are/
01:33bzThe parser really is super-malloc-happy. :(
01:34bznsHtml5TreeBuilder::endTag is freeing stuff too...
01:39bzMaking this stuff like 2x faster is going to be hard.
01:40bzThere just isn't that much overhead in here, apart from just "lots of stuff going on
01:41bzAh, here's the thing you filed those bits about image state stuff on
01:41bzI see
01:41bzOK, plus realloc for the attr and child array
01:42smaugreducing alloc/free might help quite a big
01:42smaugbit
01:42bzyeah
01:42bzI'm not quite sure how best to do that for attr storage
01:42bzRemoveScriptBlocker toes a TLS access
01:43bz MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());
01:43bzPresumably that
01:43bzOK, so at least that's nightly-only
01:43bzwell, or aurora
01:43bztons of this state stuff
01:44bzMaybe we can figure out a better way to do that once stylo lands
01:44smaugis there a plan when stylo might land?
01:44bzwell, 57
01:44bzis the current target
01:44bzWhether we hit that...
01:45bzbtw, thoughts on the two options for image loading content getting the nsIContent*?
01:47bzwhat
01:47bzwhy does HTMLInputElement update its type from ParseAttribute???
01:47smaugmight be worth to try adding virtual getter for nsIContent* and if that doesn't get optimized out well enough, then member variable
01:48bzOh, man
01:48bzThis is _karnaze_ code.
01:52bzI wonder whether we can just fix it....
01:52bzscary
01:53bzBut that's why it's still here
06:52freesamaelAnyone's using new cleopatra addon on windows? I often get this error message when trying to capture profile - [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIMessageSender.sendAsyncMessage]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js ->
06:52freesamaelresource://jid0-edalmuivkozlouyij0lpdx548bc-at-jetpack/index.js :: makeProfileAvailableToTab :: line 156" data: no]
13:23padenotwhat's the pref to disable the popup blocker, to use in a crashtest ?
13:25padenotor anything that allows window.open in a crashtest, really
13:25padenotit's returning `null` at the moment, I think it's because of the popup blocker, but I can't repro locally
13:30snorpfirebot: seen tn
13:30firebottn was last seen 1 year and 118 days ago, saying 'you convinced me just now that it was needed' in #apz.
13:45smaugsnorp: tnikkel perhaps?
13:47snorpsmaug: indeed
13:49froydnjhm, firefox up twice at pwn2own
13:52snorpfroydnj: yesterday, right?
13:52snorpdid they succeed
13:52snorpoh, no, today
13:53snorpcool
13:54smaugbz: ping
13:55smaugmaybe I should just read all the patches. That probably explains the UpdateState stuff
13:59overholtfreesamael: I don't have such an issue
14:08smaugbz: want to explain https://bug656197.bmoattachments.org/attachment.cgi?id=8847952
14:08firebothttps://bugzil.la/656197 NEW, bzbarsky@mit.edu Push state updates further out across beforesetattr/aftersetattr
14:09smaugwho is caring about the state atm? PresShell?
14:12* smaug guesses today's exploits will show issues in... svg and editor.
14:12jwattugh
14:13jwattmake another guess please :p
14:20padenotsmaug, no web assembly ?
14:26smaugI hope web assembly is self contained code enough and verified rather systematically. I could be wrong.
14:27smaugeditor and svg tend to be somewhat rarely used old code
14:33bkellysmaug: did they post the schedule?
14:34bkellyfor today
14:34smaugthey did... where is it
14:34padenothttps://www.zerodayinitiative.com/blog/2017/3/15/pwn2own-2017-day-two-schedule-and-results
14:34bkellythanks
14:35bkelly4pm and 5pm looks like
14:35bkellypacific
14:35AutomatedTesterfun times ahead :)
14:35smaugexactly
14:36bkellydo we know yet if the test machine will be running e10s config or not?
14:40smaugmccr8 said the machine might have touchscreen
14:41smaugwhich means no e10s :/
14:41bkellysmaug: only if 64-bit version in use! FF52 supports e10s with touchscreen in 32-bit build
14:41bkellynot confusing at all
14:41mccr8We have somebody there trying to work on it. So I'm not sure how it will end up.
14:42mccr8Well, they were using a 64-bit build so that could be a problem. (it is much better for ASLR)
14:42smaugbkelly: really? that sounds crazy
14:43bkellythat is what I've been told
14:43bkellysmaug: apparently 64-bit e10s touch screen support is in FF54
14:44bkellyin any case... non of it really inspires confidence
14:47bzsmaug: you there?
14:47bzsmaug: Does https://bugzilla.mozilla.org/show_bug.cgi?id=656197#c12 help?
14:47firebotBug 656197 NEW, bzbarsky@mit.edu Push state updates further out across beforesetattr/aftersetattr
15:02* bkelly loves the slight differences in spelling of cycle collection macros.
15:04* bkelly loves to complain.
15:06froydnjI hear we can fix spelling isues
15:07bkellyfroydnj: I can learn the difference between CYCLE_COLLECTING and CYCLE_COLLECTION
15:09bkellyprobably not worth a rototill to make those the same
15:11bkellyalthough it would be nice just to make the spell it as CC
15:24bkellyfarre: smaug: is requestIdleCallback ready to ship yet?
15:25farrebkelly: this leak is what's blocking https://treeherder.mozilla.org/#/jobs?repo=try&revision=17933301e6cf31ccf48b8e71e752d82ccc845a0a&selectedJob=84243110
15:26bkellyfarre: why do we think that is caused by requestIdleCallback?
15:26farrebkelly: and I just now managed to reproduce it:
15:26farrehttps://irccloud.mozilla.com/pastebin/X8Cp4ay1/
15:26bkellycool!
15:26farrebkelly: it appears due to the patch
15:28farrebkelly: smaug: and it has something to do with a chained rIC when we at the same time remove the frame. just as smaug predicted
15:28farrebut now that I have a local test case there's at least something to work with
15:29bkellynice
15:30* bz looks for a victim^Howner for bug 1347737
15:30firebothttps://bugzil.la/1347737 NEW, nobody@mozilla.org nsHtml5HtmlAttributes::clear spends a bunch of time deallocating
15:32farrewith the risk of sounding like a total beginner, how do I get symbols in the result of tools/rb/make-tree.pl?
15:35* bkelly got his new desktop.
15:36padenotbkelly, benchmark time !
15:37bkellynot sure when I'll have time to set it up
15:37* bz has that problem
15:37bzfarre: which OS?
15:37farrebz: linux
15:38farrebkelly: \o/
15:38bzfarre: pipe the output through tools/rb/fix_linux_stack.py
15:39bzfarre: (there's a fix_macosx_stack.py for Mac)
15:41farrebz: awesome! thanks
15:42* mystor just got a slow script dialog for tabbrowser.xml
15:42mystorI wasn't aware we allowed slow script dialogs for chrome
15:46bkellybz: changing stuff in Bindings.conf requires a Clobber?
15:46bzbkelly: shouldn't
15:47bkellybz: I'm changing the native class for an interface to a new Class/header... seems to still be trying to import the old one
15:47bkellyhmm
15:49bkellyI did a clobber... and its still failing...
15:49bkelly /srv/mozilla-central/obj-x86_64-pc-linux-gnu-optdebug/dom/bindings/ServiceWorkerGlobalScopeBinding.cpp:30:68: error: no matching function for call to 'StrongOrRawPtr(mozilla::dom::workers::ServiceWorkerClients*)
15:50bkellyI replaced ServiceWorkerClients type with Clients in Bindings.conf
15:50bzhuh
15:50bzWhat does it mean "no matching function"?
15:51bzAh
15:51bzum
15:51bzno...
15:52bkellyah
15:52bzSo we're talking about [SameObject] readonly attribute Clients clients;
15:52bzin ServiceWorkerGlobalScope.webidl, right?
15:52bkellybz: its coming from here: https://dxr.mozilla.org/mozilla-central/source/dom/workers/WorkerScope.h#282
15:52bkellyI have to fix that too
15:52bzok, that makes sense. ;)
15:52bzGood.
15:53bkellysorry for my confusion
15:57smaugbz: victim for that parses issue? why not hsivonen?
16:00* bkelly had to use [BinaryName=].
16:01bkellybz: still feels like I needed a clobber to make that work as well as fixing that header I linked
16:05bkellybz: hmm... now the webidl code generator is sticking an include to ServiceWorkerGlobalScope.h into my ClientsBinding.cpp... that header doesn't exist
16:07bkellyoh, I see
16:08bkellybz: probably coming from this: https://dxr.mozilla.org/mozilla-central/source/dom/webidl/Clients.webidl#19
16:08bkellybut I don't understand why that doesn't cause the include in m-c today
16:10bzsmaug: I might try to convince him to do it, yes....
16:10bzsmaug: see above for UpdateState thing
16:11bzsmaug: ?
16:11bzbkelly: so....
16:11bzbkelly: The bindings code tries to be smart for Func
16:11bzbkelly: and include the right header
16:11bzbkelly: by looking at the namespaces etc
16:11bzbkelly: _but_ it only does this if it has reason to believe that your namespaces and headers match up in sane ways
16:12bzbkelly: And as a proxy for that it uses whether you're using a custom haderFile
16:12bzer, headerFile
16:12bzbkelly: If you are, it's clear that your namespaces and headers are uncorrelated and it just stops trying to make things work for you
16:13bkellybz: ok... I'll move that func to live on the Clients object instead of the global
16:13bzbkelly: and doesn't generate any include for Func at all, and assumes that you will handle it somehow, because there's no way _it_ can handle it.
16:13bzbkelly: It's not great.
16:13bzbkelly: I mean, in an ideal world we'd nix all the headerFile crud, always include the right headers automatically, people would put stuff in headers that match its namespace
16:13bzbkelly: And so forth.
16:13bzbkelly: We're not quite there, for various reasons. Esp. around workers. :(
16:14bkellyI wonder if we even need this Func= any more
16:14bzThat sounds like a policy question, not mechanism. ;)
16:15* bz can answer mechanism questions about bindings.
16:15bkellyyea
16:25annevkbz: could you take a look at https://github.com/whatwg/html/issues/2440?
16:25annevkbz: folks suggesting poking holes in same-origin policy protections :/
16:25bzLooking
16:26annevkbz: I'm a little out-of-my-depth though since not everything about exceptions is defined and I haven't researched what implementations do myself
16:26jugglinmikeI have a question about how Event subclasses are initialized. Specifically: how is the provided "init dict" interpreted?
16:26jugglinmikeI've gotten as far as https://dom.spec.whatwg.org/#constructing-events , which reads For each dictionary member present in eventInitDict, find the attribute on event whose identifier matches the key of the dictionary member and then set the attribute to the value of that dictionary member.
16:26bzannevk: So...
16:27bkellyjugglinmike: are you planning to land a test for the ExtendableMessageEvent.source thing?
16:27annevkjugglinmike: each subclass defines a more specific dict but calls the argument eventInitDict
16:27jugglinmikeBut I'm not sure how that maps to JavaScript. I'm wondering, for instance, how `{ attr: null }` is interpreted
16:27annevkjugglinmike: object to dictionary conversion is defined in IDL
16:27jugglinmikebkelly: in part. I am porting some tests from Chromium, and I want to make sure they are correct
16:28bkellyjugglinmike: if you are intending to land a test that covers that crash I won't bother writing one in our tree
16:29bzannnevk: I think reporter is correct that outside of initial script execution you're SOL
16:29jugglinmikebkelly: Yup; been meaning to respond to your needsinfo request
16:29bkellyok, thanks
16:29bzannevk: in that the page can try/catch to get the actual exception anyway
16:29jugglinmikeI just want to have the test in hand first
16:29bkellynp
16:29jugglinmike(taking a little longer than I expected is all)
16:30jugglinmike( ...see above :P )
16:30annevkbz: incl URL line/col etc. then I guess?
16:30bzannevk: yes
16:30bzannevk: well
16:30bzannevk: "no"
16:30bzannevk: In that there is no spec for exceptions having those things at all
16:31bzannevk: They do in practice in browsers, but per spec the only thing exceptions have is a name and message
16:31annevkonerror does expose them per standard, but yeah, not really based on any primitive from JavaScript...
16:32bzright, exactly
16:32bzonerror has more information than exceptions are required to
16:32bzBut not more information than they actually have.
16:32bzI think
16:32annevkSo the only defense the foreign script has is that it's hard to overwrite the prototype of everything
16:32bz(this last bit is worth testing; since it's not covered by standards it's entirely likely it's not interoperable)
16:33bzFor example, I would not be surprised if all UAs have a .stack (in different formats) but not all have explicit url/line/column getters
16:33bzAnd of course the messages are not interoperable at all....
16:33annevkDoes stack include url/line/column?
16:33bz"yes"
16:33annevkright
16:33bzThe stack format is not specified.
16:34bzBut in practice it does.
16:34bz(I mean, the stack existing is not specified either....)
16:34annevkYeah I know, I saw some attempt at fixing that, but...
16:35bzI mean, we could in fact fix JS-side bits to sanitize exceptions
16:35bzif we wanted to.
16:36bzBut fixing the "everything has been hooked under you" bit is of course harder.
16:38bzannevk: I'll comment in the issue.
16:38annevkthanks
16:43jugglinmikeI believe this assertion is invalid-- annevk would you mind confirming? https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/tests/serviceworker/ServiceWorkerGlobalScope/resources/extendable-message-event-constructor-worker.js?sq&l=134
16:44jugglinmikespec: https://w3c.github.io/ServiceWorker/#extendablemessageevent-interface
16:44jugglinmikeI believe that should throw a TypeError, according to https://heycam.github.io/webidl/#es-sequence
16:45annevkjugglinmike: yeah
16:45annevkjugglinmike: you can only set it to []
16:45annevkback in a bit
16:45jugglinmikethanks!
16:54gandalfehsan: ping
17:16bzannevk: commented; sorry it took so long.
17:20annevkbz: there was no rush and that's a great comment
17:29* bz tries
17:45ehsangandalf: hi
17:45ehsan(in a meeting)
17:45gandalfehsan: I have a few questions about prefs and bootstrap. Would you find time today to help me a bit?
17:46smaugmystor: ping
17:46ehsangandalf: yeah sure, please shoot
17:46mystorsmaug: pong
17:46smaugmystor: this E10SUtils.redirectLoad call
17:46mystoryes
17:47smaugwith you patch when do we actually end up executing it?
17:47smaugI mean with postdata
17:47bkellymccr8: I was thinking the same thing! (about hangouts)
17:47mystorWith the patch I have added, what happens is nsIWebBrowserChrome3.shouldLoadURI is called passing aHasPostData = true
17:48mystorthat calls E10SUtils.shouldLoadURI, which then returns true
17:48mystorthis causes redirect load to never be calld
17:48mystor(in the performing a post request from a large-allocation process case)
17:48* mystor hopes that makes sense
17:48smaugmystor: right, it shouldn't be ever called
17:49mystoryes - the XXX comment is for other cases
17:49mystorso, say `aHasPostData` is true, after the patch we still call shouldLoadURIInThisProcess
17:49smaugoh, the XXX isn't about LA
17:49mystorwhich might return false if, for example, you're trying to load about:preferences in content
17:50mystorIn that case we definitely need to change processes, but we would also discard post data (ignore that you can't POST to about:preferences for a second)
17:50smaugdoing a post load for about:preferences ?
17:50gandalfehsan: ok, so basically. I'm reworking the guts of how we select locale at the startup. At the moment, we're doing this pretty complex dance, where we first kick off XPCLocale which asks for AppLocale *before* the prefs are initialized, so we can only return the "default" value for general.useragent.locale, and then it applies the user modifications at which
17:50gandalfpoint we get it from the observer and have to update the appLocale and notify XPCLocale that the appLocale is now different.
17:50mystorOK - let's pretend something different, we have a file:// URI which has an iframe with google.com in it
17:50gandalfehsan: my question is. Is that by design (JS context has to launch before prefs are initialized)?
17:50smaugmystor: ok, that might be valid
17:50mystorthat google.com iframe makes a post request to google.com/search (I don't know why it's posting search - we're playing pretend), which targets _top
17:50smaugbetter to file a followup bug
17:51mystoryeah, I don't think that is worth fixing in this bug
17:51mystorespecially because we need to uplift this one all the way to beta I think
17:51mystorIt's also a much bigger issue
17:52smaugbobowen might have some feedback to the file:// issue
18:04mccr8bkelly: hah hopefully it didn't come across as too mean.
18:05bkellynah
18:09ehsangandalf: yeah, "support" running the profile chooser which is written in xul without having a profile available
18:10ehsangandalf: so anything that can run while that thing wants to run needs to be able to run without having access to a profile
18:11gandalfso the order of -> 1) XPCLocale init asks for appLocale, 2) LocaleService reads the default general.useragent.locale value, 3) Prefs update to user-selected value, 4) LocaleService reacts by updating appLocale and notifying all listeners
18:11ehsangandalf: wait...
18:11gandalfis the best we can get?
18:11ehsangandalf: you didn't ask about profiles :( this is what I get for reading question when being in a meeting!
18:11ehsangandalf: so you're just asking about prefs right?
18:11gandalfyes
18:12ehsanok
18:12ehsangandalf: let me look at this code...
18:12gandalfthank you!
18:12ehsangandalf: is this it? http://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCLocale.cpp#248
18:13gandalfyes!
18:13ehsanok
18:13ehsangandalf: do you know where step 4 happens?
18:14gandalfhttp://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#363
18:15ehsanok, so from http://searchfox.org/mozilla-central/source/chrome/nsChromeRegistryChrome.cpp#317
18:15gandalfehsan: of course, you'll need a non-default value in `general.useragent.locale` to test it
18:15ehsanI'm just following the code... :)
18:15gandalfehsan: well, yeah. I'm moving this piece exactly :)
18:15* ehsan reads further
18:15gandalfaway from ChromeRegistry and into LocaleService
18:16gandalfon master, both ChromeRegistry and LocaleService react to update to general.useragent.locale
18:16gandalfbut only XPCLocale requests appLocale before that happens
18:16ehsanI think you're coming from http://searchfox.org/mozilla-central/source/chrome/nsChromeRegistryChrome.cpp#143
18:16gandalfyeah, so I removed that call in my branch :)
18:16ehsanhmm
18:17ehsanbut that doesn't call the observer
18:17ehsanhrm
18:17gandalfbecause nothing requires mSelectedLocale from ChromeRegistry
18:17gandalfthat early
18:17ehsanok
18:17gandalfin my log it shows as:
18:17gandalf1) http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#164 shows "en-US" (default value for the pref)
18:18ehsanone question
18:18* ehsan listens
18:18gandalf2) http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#354 gets fired with "updated pref for general.useragent.locale)
18:18gandalf3) http://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#164 returns "pl"
18:18gandalfmy only problem is that (1) is triggered by XPCLocale before (2)
18:19gandalfand wanted to check with you if it's necessary
18:19ehsanis the general.useragent.locale pref set in user_prefs.js in your profile?
18:20gandalfyes, the "en-US" is the default value from the buildtime, "pl" is manually set by the user
18:22ehsanhmm
18:22ehsangandalf: no that seems like a bug to me
18:23ehsangandalf: it seems like we should be able to initialize the profile in time for XPCLocale to have the correct pref value
18:23gandalfsweet! Any idea where the order of initialization takes place? (like, where it is decided that XPCLocale::Init is fired before Prefs:ReadUser)?
18:23gandalfI'll file a bug and work on that, just need a lead
18:24ehsanhmm
18:24ehsanlooking through searchfox, this is coming from nsXPConnect::InitStatics()
18:24ehsanhttp://searchfox.org/mozilla-central/rev/571c1fd0ba0617f83175ccc06ed6f3eb0a1a8b92/js/xpconnect/src/XPCModule.cpp#13
18:24ehsanaka libxul's ctor basically
18:25ehsanmy brain is rusty on xpcom init order
18:25ehsanI suggest filing a bug
18:25ehsanputting this into on the bug
18:25ehsanand ni?ing froydnj
18:25bkellybz: it would be nice if we could do something like [WorkerFunc=FooEnabled]
18:25ehsanI hope it'd be possible to rejigger stuff to make the prefs available at this point
18:25ehsanbut I wouldn't bet the house on it
18:26bkellybz: and make the generated code do GetCurrentThreadWorkerPrivate()->FooEnabled()
18:26ehsan(this is very early at startup)
18:26bkellybz: because our worker pref macros expose stuff as functions on the worker private
18:26gandalfehsan: ok! thanks
18:27bzbkelly: ah
18:27bzbkelly: hmmm
18:27ehsangandalf: good luck
18:27gandalfthank you :)
18:27bzbkelly: As written above that would only work for things that are exposed on workers
18:27bkellybz: or some other way to tie it into this: https://dxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrefs.h
18:28bkellybz: we could in theory expose a GetPref(string) on WorkerPrivate that only returns for values in WorkerPrefs.h
18:28* bz ponders
18:28bkellybz: yea, the generated code would need to do a main thread check and only do worker stuff if OMT
18:28bzI mean, this is obviously a good idea
18:28bzin the sense of solving the longstanding pain point around Pref not working for workers
18:28bkellyI guess I'll add it to https://bugzilla.mozilla.org/show_bug.cgi?id=917182
18:28firebotBug 917182 NEW, nobody@mozilla.org Adding [Pref=prefname] to worker specific WebIDL interfaces causes "not thread-safe" assertions
18:31bkellybz: sorry, I was thinking we were closer than we are
18:31bzbkelly: well
18:35bzbkelly: So we could totally have a WorkerAwarePref="foo.bar"
18:35bzbkelly: which translates to some function....
18:35* bz ponders
18:35gandalfehsan: am I correct to read that the code I'd like to be launched before xpcModule ctor is http://searchfox.org/mozilla-central/source/modules/libpref/Preferences.cpp#652 ?
18:35bkellybz: probably similar amount of work just to make Pref= DTRT for workers now
18:36bkellyI think
18:36bzbkelly: no
18:36ehsangandalf: yeah
18:36bzbkelly: We have certain dependencies on the IDL parser forbidding Pref on anythings exposed on workers
18:36bzhttp://searchfox.org/mozilla-central/source/dom/bindings/DOMJSClass.h#116-119
18:37bzfor example
18:37gandalfehsan: thanks!
18:37gandalfehsan: last question. In content process we don't read user prefs at all, correct?
18:37bzI mean, the right answer is to give up on handling Pref without a TLS lookup or something.
18:37ehsangandalf: correct
18:37ehsangandalf: we send them down from the parent
18:37bkellybz: we could avoid the TLS lookup... the global owning the DOM object should know if its main thread or worker scope
18:38bkellyor amortize it greatly I guess
18:38gandalfehsan: so, if we wanted to really fix localization, we'd need a way to communicate general.useragent.locale from parent to child with an observer, not just constructor.
18:38* bkelly wonders if pulling the display cable out of this machine will cause it crash...
18:38bzbkelly: Anyway, I'm happy to review patches here.
18:39ehsangandalf: what do you mean?
18:39bzbkelly: In terms of working on it myself, I likely won't have time at least until next week....
18:39ehsangandalf: why is this pref special?
18:39bkellybz: yea, ENOTIME for me as well
18:39gandalfehsan: it's not. Did i misunderstand you?
18:39gandalfehsan: can we read user prefs from content process?
18:39bkellybz: intern project over the summer?
18:40ehsangandalf: no, we can't
18:40bzbkelly: It's not big enough for an intern project, probably. But could be a nice side project.
18:40ehsangandalf: as in, we cannot read them from the disk
18:40ehsangandalf: we _can_ read them from the preferences APIs
18:40gandalfok
18:40bzbkelly: Unless the project were to make this play nicer with quantum DOM bits with DOM running on multiple threads even for windows
18:40gandalfso when LocaleService in content process asks Prefs for `general.useragent.locale` it'll just read it from parent process?
18:40bzin which case it might be a bit more involved. ;)
18:40bzBut we don't really know how or whether that will work yet
18:41bkellybz: my understanding is that quantum DOM on other threads should still just work
18:41bkellybz: since billm wants the other threads to still pretend to be main thread
18:41bzhrm
18:41bzI see
18:41bkellybz: and use a BGL
18:42gandalfehsan: because atm I'm getting hit with http://searchfox.org/mozilla-central/source/modules/libpref/prefapi.cpp#789 but that may be just the ordering thing where XPCLocale asks for locale before prefs are ready.
18:42ehsangandalf: no, the way that the prefs setup works in the content process is the parent tells the content process about the prefs when the content process launches, and it keeps the content process updated when the prefs change, and when the content process tries to read a pref it'll use the cached copy
18:42ehsangandalf: oh
18:42ehsangandalf: that means you have to add this pref to this list:
18:42* ehsan digs up the link
18:42ehsanhttp://searchfox.org/mozilla-central/source/dom/ipc/ContentPrefs.cpp#13
18:42gandalfsweet, thank you!
18:42ehsan(there's a comment about this right above the code you linked to)
18:43ehsanblassey: this content prefs setup is going to be a pain the behind to maintain :( ^
18:43ehsanblassey: (not to say I have a solution... just a ping about the general pain)
18:43gandalfehsan: I somehow read the comment as "consider moving it later or ask DOM peer" - don't ask me how my brain managed to read it selectively :)
18:44ehsangandalf: heh, it's OK :)
18:44ehsanmany people have been confused by it
18:44blasseyehsan: my hope is that we move away from prefs for most of these things
18:44ehsanblassey: problem is, people keep running into it with new things :(
18:44billmehsan: the problem is that people are reading prefs before they really need to in many cases. XPConnect is one such case.
18:44blasseyehsan: also... if you use a cached var, you don't need to be on the whitelist
18:45ehsanbillm: I think the case at hand here is legit, fwiw
18:45billmI think a more general fix for a few key places would help a lot of things
18:45billmehsan: why?
18:45ehsanblassey: really? a cached var would fix it?
18:45ehsan(is there support for string cached vars?)
18:45blasseyehsan: it'll use the default to start and call your handler if the actual value is non-default
18:45ehsanbillm: we need to remember the user's choice of UI locale?
18:46ehsanbillm: (maybe I'm misunderstanding what you mean?)
18:46billmehsan: I haven't seen the code. why do we need that at startup, before any content is loaded?
18:46ehsanthat I don't know...
18:46ehsan1 sec
18:46ehsanbillm: http://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCLocale.cpp#250
18:46blasseyehsan: no, not in the same way
18:46ehsanbillm: I have no idea what the implications of not calling JS_SetDefaultLocale is... sounds bad though
18:47blasseybut you can roll your own with RegisterCallback
18:47ehsanblassey: :(
18:47blasseyits done in other parts of the code
18:47ehsanblassey: maybe we should add support for it? seems like a common enough use case
18:47bkellybz: are you still working stylo stuff?
18:47ehsanwe !== me ;)
18:47blasseyif you want to to add the boiler plate to Preferences I don't think anyone would object
18:48billmehsan: I don't see any reason that couldn't be done later
18:48ehsanblassey: I honestly don't have time to read my mail these days :(
18:48ehsanlet alone anything else
18:48ehsanbillm: ok
18:48ehsangandalf: did you file a bug?
18:48ehsangandalf: billm may have an alternative solution
18:48gandalfehsan: yes
18:48ehsan#?
18:48* bkelly is happy his machine survived unplugging display/mouse/keyboard
18:49billmehsan: the problem is that there's no one who has time to do this work
18:49ehsanheh, there is another pref access before it too http://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCLocale.cpp#250
18:49gandalfehsan: https://bugzilla.mozilla.org/show_bug.cgi?id=1344355#c7
18:49ehsanbillm: tell me about it :(
18:49firebotBug 1344355 ASSIGNED, gandalf@aviary.pl Improve the locale registration in ChromeRegistry
18:49bzbkelly: Yes, I am, re stylo
18:49* ehsan comments
18:50bkellybz: how does the perf look?
18:50gandalfbillm: we do need the user-selected locale pref to be read before XPConnect is initialized, so that it can be initialized in the right locale.
18:51bzbkelly: In the one place I have measured it (inline style) abysmal. Bugs are filed.
18:51billmgandalf: why?
18:51bzbkelly: For pageload, I don't know.
18:51billmgandalf: when do we use the locale?
18:51gandalfbillm: I guess on the first new Date or first new Intl.DateTimeFormat?
18:51bkellybz: new project name... "Quantum Abyss"
18:51bzbkelly: heh
18:51billmgandalf: we shouldn't be doing that at such an early stage in the content process
18:52gandalfit's in chrome process
18:52billmgandalf: the assertion only fires in the content process.
18:52billmgandalf: perhaps you could make it go away by conditioning your code on XRE_IsParentProcess()
18:52ehsanhe was talking to me about two different issues afaict
18:52billmer, make the assertion go away
18:52ehsanthe first one in chrome
18:52gandalfbillm: oh, ok, yeah, so XPCLocale does it in both, chrome and content
18:53ehsansecond one in content
18:53ehsanyeah
18:53gandalfin chrome it's just too early, in content it hits the assertion
18:53ehsanif we make this happen later, we'll fix both issues
18:53gandalfcool!
18:53ehsanthe question is, when is a good time for later
18:53* ehsan isn't sure about that
18:53gandalfyeah, so the question is if it's better to init XPCLocale later or just the SetDefaultLocale
18:54billmgandalf: maybe you should just add your pref to the list. I don't think it's worth spending time dealing with this right now.
18:54gandalfbillm: it doesn't fix the chrome process problem for me :(
18:54ehsanbillm: that still fixes half of the issue for him
18:54billmI must have missed part of this discussion...
18:56ehsangandalf: you need to find a place in application startup before we start calling into the first piece of chrome js
18:56ehsangandalf: I unfortunately don't remember it off the top of my head :(
18:56ehsansorry
18:56* ehsan doesn't have more time to think about this at the moment
18:56gandalfyeah, I NI'd froydn
18:56ehsanok
18:56gandalfehsan: np.! thanks for your time, I should be able to go from here.
18:56ehsanI'll post on the bug if I remmeber something better...
18:57ehsanI hope so :)
19:15ehsantonight's pwn2own
19:17ehsanbkelly: can you file a bug on this stylesheet memory caching thing with some more details?
19:17ehsanbkelly: I don't know enough about the style system to understand your irc comments from the other day enough to file a bug with it :(
19:17bkellyehsan: https://bugzilla.mozilla.org/show_bug.cgi?id=219276
19:17firebotBug 219276 NEW, nobody@mozilla.org Cache parsed stylesheets
19:19ehsanbkelly: ty
19:43tbsaundeehsan: guess you failed at your take pto plan?
19:44ehsantbsaunde: it ain't tonight yet :P
19:44ehsantbsaunde: the first pwn2own slot is 4pm western time
19:44* ehsan goes on vacation at 6:59pm tonight ;)
19:45ehsanjdm: ping
19:45qDotI wasn't even aware we were back in pwn2own
19:45jdmehsan: pong
19:45ehsanqDot: we are
19:45ehsanjdm: hey
19:45ehsanjdm: you're a cookie peer, right?
19:46jdmyes
19:46ehsanjdm: good!
19:46ehsanjdm: do you think you can provide feedback on bug 1331680?
19:46firebothttps://bugzil.la/1331680 NEW, amchung@mozilla.com Consider not doing sync IPC for document.cookie getter/setter
19:46ehsanjdm: I could do that, but since I proposed the plan that feels like cheating
19:47jdmok
19:47ehsanjdm: (I want critical eyes on this... especially on the high level design)
19:47ehsanjdm: (and please do NOT trust _my_ sanity on the plan either)
19:49ehsanjdm: thanks so much
19:50ehsanbaku: ping?
20:43bobowensmaug, mystor: sounds like bug 1344465, for which I have potential patches up for review
20:43firebothttps://bugzil.la/1344465 ASSIGNED, bobowencode@gmail.com Can't submit form using post method form WebExtensions or file:// page
20:44smaugindeed
20:45froydnjmystor: whoa, your new patch is more complex
20:45mystorbobowen: bug 1348018 is what I filed, should I dup it?
20:45firebothttps://bugzil.la/1348018 NEW, nobody@mozilla.org Toplevel POST document loads may lose data if E10SUtils.shouldLoadURIInThisProcess(aURI) returns fal
20:45mystorfroydnj: Yeah - sorry about that
20:46* mystor had to rework some of the logic to hack around the lack of specialization
20:47bobowenmystor: sounds like it, if you're happy that we do post the data
20:47mystorbobowen: What do you mean by that?
20:48bobowenmystor: which didn't seem to happen from your description of a solution above
20:48bobowenmystor: so I wondered if you didn't want to post it
20:51bobowenmystor: does my solution fix bug 1347983 as well?
20:51firebothttps://bugzil.la/1347983 NEW, michael@thelayzells.com Forms submitted from a large-allocation page discard post data
20:52bobowenmystor: ah, I see your patch has already landed
20:52* bobowen feels a rebase coming on
20:53mystorbobowen: Once you land your code you can probably revert part 1 of tthe large-allocation fix
20:53mystorbobowen: It was a workaround because I didn't want to implement the stuff you were working on
20:54mystorbobowen: (basically I decide in that code to just not leave the process, instead of redirecting the post to the correct one)
20:54bobowenmystor: yes and I see yours needed uplifting
20:54mystoryeah, so I think I should uplift mine
20:54mystorand you should replace mine with yours when you land it
20:55* bkelly decides to set up his new computer...
20:55* bkelly goes offline for a bit.
20:55bobowenmystor: OK thanks, that's an easy rebase then :-)
20:55mystorbobowen: np :)
20:56mystorbobowen: make sure my test in part 2 passes once you apply the patch, if it does we should be good :)
20:57bobowenmystor: OK, I'll definitely check that
20:59bobowenmystor: from a brief scan I think it should pass, but I will check tomorrow
21:00mystorbobowen: awesome, thanks
21:00bobowennp, gtg
21:07bzfroydnj: ping
21:21* bkelly is now running on new machine...
21:24smaugbobowen: oh, the patches were even in my review queue :p
21:29gandalfehsan: got a response! I think I'm on the track to test the solution. Thanks a lot!
21:29bzjgraham: ping
21:29jgrahambz: Hi
21:29bzjgraham: So https://github.com/w3c/web-platform-tests/pull/5163#issuecomment-287196133
21:29gandalfehsan: for the content process - is, for example, datetimepicker stuff in content process or parent process? Do you know of any browser UI pieces that should be localized and are running in content process?
21:29bzjgraham: It has test results, but when I follow those links I get 404s
21:30bzjgraham: Is that because domenic pushed an update?
21:30* jgraham loks
21:30jgraham*looks
21:30smaugmystor: http://searchfox.org/mozilla-central/source/browser/base/content/browser.js#1010-1018
21:31smaugI'm a bit worried how that works when posting lots of data
21:31jgrahambz: Appears to be a bug with the sync script