mozilla :: #content

20 Apr 2017
08:35smauggabor: ping
10:39smaugbevistseng|away: ping
10:39bevistseng|awaysmaug: pong
10:40smaugbevistseng|away: hey, do we do something special with AbstractThread for labeling
10:40smauglike, do we set the current AbstractThread to some magical nsIEventTarget of a current TabGroup?
10:40bevistsengYes, We wrap Dispatch::EventTargetFor as an AbstractThread
10:41smaugis this documented anywhere?
10:42smaug(like I have no idea whether https://bug1353629.bmoattachments.org/attachment.cgi?id=8859921 makes sense or no. Based on AbstractThread.h it doesn't)
10:42firebothttps://bugzil.la/1353629 NEW, amarchesini@mozilla.com [meta] PBlob refactoring
10:42smaug(but maybe it does)
10:44bevistsengThe TLS only set here http://searchfox.org/mozilla-central/source/xpcom/threads/AbstractThread.cpp#162
10:44bevistsengWhen TabGroup specific AbstractThread is running
10:44smaugbevistseng: is that set whenever a runnable is running
10:44smaugeven if other code isn't using AbstractThreads?
10:45bevistsengwhen a runnable dispatched to AbstractThread is runnable
10:45bevistsenghttp://searchfox.org/mozilla-central/source/xpcom/threads/AbstractThread.cpp#72-74
10:45bevistsengwhen a runnable dispatched to AbstractThread is running
10:45smaugbevistseng: and when dispatching to event target from Dispatch::EventTargetFor, it is AbstractThread?
10:46bevistsengno,
10:47bevistsengonly if the AbstractThread was acquired from here
10:47bevistsenghttp://searchfox.org/mozilla-central/source/xpcom/threads/SchedulerGroup.cpp#160-173
10:47smaugok, so https://bug1353629.bmoattachments.org/attachment.cgi?id=8859921 doesn't do proper labeling then
10:48bevistsengyes, we should prevent AbstractThread::GetCurrent or AbstractThread::MainThread on main thread of content processes.
10:49bevistsengbut use AbstractMainThreadFor when possible
11:14annevkshawnjohnjr: you around? On Nightly in about:preferences -> Privacy & Security -> Site Data -> Settings, it claims nothing is stored locally which seems extremely dubious and related to work you've been doing I think?
11:15shawnjohnjrannevk:let me check with Fischer
11:18fischerAnnevk
11:18fischerannevk:ping
11:18annevkfischer: hey, I'm here
11:19fischerDo you mean we saved some data into the indexedDB but Site Data Settings doesn't list site?
11:20annevkfischer: there's no sites listed when I go there, which seems really weird
11:21annevkfischer: maybe nothing I visit is using IDB though
11:21fischerok,would you try to ask persistent-storage permission, open console to ask for one site
11:22fischerthe thing is that originally the list comes from Permission Manager, then the plan changed to go for Quota Manager
11:23annevkfischer: there's nothing still
11:23annevkfischer: aw it just took a long time
11:23fischerok i will try
11:23annevkfischer: now there's a site there
11:23annevkfischer: but only the one I invoked persist() on
11:24annevkfischer: and when I remove the permission it's gone again
11:24annevkfischer: okay, so I guess the problem is that it's not yet based on Quota Manager?
11:24annevkfischer: hopefully we won't let that go to release?
11:25fischeryes that is the current behavior, the reason we are still unable to switch to the list from Quota Manager is because the resources and schedule (photon)
11:26fischerwe want to fix that
11:26fischeralong with all other storage-related section in about:preferences
11:26fischerfor a better UX
11:27fischerthat's target for the next improvement
11:28annevkfischer: my understanding was that we'd at least get this fixed, since now we basically regressed functionality :/
11:40fischerannevk: ok would you please open a bug for this then NI: EPM: Francis
11:41annevkfischer: sounds good, will do after lunch
15:01farresmaug: hey, talk to me about wrappers on top of idleDispatch
15:03farresmaug: because I'm not so certain either anymore
15:23smaugfarre: yeah, I was just curious why you think we need it
15:24smaugbkelly's queue is to prevent bad web sites to run too many timeouts
15:24smaugwhy we'd need that for chrome code?
15:24farresmaug: so I'm a bit jealous of the idle slices, so I wan't some safe guard for fairness
15:25smaugfarre: but why would chrome JS be any different to C++ ?
15:25farresmaug: add-ons
15:26smaugchrome JS != addons
15:26smaugI don't know what APIs well expose to web extensions
15:26farresmaug: then we're good
15:26smaugI sure hope web extensions can't access nsIThread
15:26smaugand other addons should be going away
15:27smaugfarre: in other words, just hoping we aren't spending too much time to implement something which is rather trivial on top of the current idleDispatch :)
15:27farresmaug: then it's just a matter of adding idleDispatch(runnable, timeout) which wraps the runnable in a new runnable that also manages the timeout
15:27farresmaug: as you say, pretty trivial
15:28farresmaug: we should just do it
15:28smaugyes
15:28smaugand then try it first with the background parsing thing for example
15:29smaugI didn't add any telemetry probes when I added the current background parsing timer, but at least in manual testing it did help quite a big
15:29smaugbut it is still just a timer, so may run at any point
15:30smaugand in case foreground tab is not animating, the timer is actually too slow
15:30smaugand with multiple child processes
15:30smaug...hmm, yes, I want the idle thing now :)
15:39farreI have some stuff pending atm, but I also want it now :)
16:14smaugwhaat, what is CycleCollectedJSRuntime
16:16smaugmccr8: ping
16:16smaugsince billm isn't here
16:17smaugoh, perhaps merging isn't so bad
16:31bzsmaug: CycleCollectedJSRuntime is CycleCollectedJSContext, no?
16:31bzsmaug: Just renamed
16:31smaugno
16:32smaugbz: Context got just split to two
16:32bzI see
16:32* bz sighs
16:32smaugnot in searchfox yet
16:32smaugbug 1343396
16:32firebothttps://bugzil.la/1343396 FIXED, wmccloskey@mozilla.com Split CycledCollectedJSContext and XPCJSContext into separate context and runtime classes
16:32smaugit makes sense for QDOM
16:32smaugI just need to merge stuff
16:41ehsanno billm
16:41ehsanI need summoning powers
16:47bzehsan: swatting is the answer, clearly
17:07ehsanbz, swatting?
17:08ehsanbz, whoa
17:08ehsan(just looked it up)
17:08ehsancertainly not, that seems rather quick to escalate!
17:08bzehsan: In case it was unclear, I was joking. ;)
17:08ehsanGuest22574, nickserv is hating you too Peter? :(
17:08ehsanbz, certainly ;)
17:08bzehsan: The fact that this is a thing, btw, is beyond sad. :(
17:09ehsanbz, certainly!!! :D
17:09ehsan</brokenrecord>
17:09ehsanis it actually a thing though?
17:10bzehsan: https://en.wikipedia.org/wiki/Swatting#Notable_cases
17:10ehsanwowww
17:11ehsancrazy world we live in
17:11ehsanalso, ignorance is really bliss :)
17:12bzSorry for bursting your bubble... ;)
17:14ehsannow I have to live in a less rosy world, thanks to you :P
17:14ehsan;)
17:28gandalfqDot: when do you think you may have time to look into bug 1348042?
17:28firebothttps://bugzil.la/1348042 ASSIGNED, gandalf@aviary.pl Make LocaleService operate in client-server mode
18:01* smaug is trying a new setup for reviews. Keeping the queue open only when not working and when starting to work, close to queue and go through the existing requests. Then re-open queue when work day is done
18:05bzhmm
18:05bzintriguing
18:07smaugI&#39;m hoping that will help with the constant context switching
18:25gandalfI&#39;m nowhere near what you guys have, but my rule of thumb is that I start my day with reviews and have a 2h limit on them.
18:25gandalfbut I don&#39;t review in the middle of the day
18:26gandalfif I have a lot of reviews, then I prefer to spend a whole day on reducing the queue and not do my own hacking at all, than to try to context switch between reviewing and writing.
18:32qDotgandalf: I&#39;ll try to look at it today. Trying to get some of my own stuff into review too before some PTO, so it&#39;s hard to balance.
18:40gandalfqDot: when&#39;s your PTO? :)
18:40qDotTomorrow! :D
18:40qDot(Just taking a long weekend)
18:40gandalfah cool :)
18:42smauggandalf: if I did only 2h of reviewing, my reviewing queue would become super long. The question for me is that do I do only reviewing, or also tiny bit something else ;)
19:00ehsanbillm, ping
19:41qDotIs there a point at which you&#39;d ever need to use Move() on a nsTArray assignment? Assignment between nsTArray<> is basically an implicit move since it just calls swap elements, right?
19:43bzqDot: no
19:43bkellyqDot: I don&#39;t think assignment equals swap
19:44bzqdot: assignment copies, doesn&#39;t swap
19:44bzqdot: So if you&#39;re in a position to move, you should move!
19:45qDotOH wait yeah we don&#39;t have an rvalue reference at that point got it.
19:45qDotSo, similarly, if an rvalue reference is passed as an lvalue reference, an assignment while it&#39;s an lvalue reference is a copy then, right?
19:47bzqdot: yes
19:48bkellyFF53 feels slow after running on nightly for a while
19:48bkellyI guess thats a good thing
19:52bzindeed
20:05qDotgandalf: Ok, review done, and I also got my every-so-often refresher on move semantics. *sigh*
20:06gandalf:D
20:08qDotgandalf: BTW don&#39;t worry about getting it done before tomorrow if you&#39;re busy with other stuff. I&#39;ll be on PTO Friday/Monday but I can still do a quick review on your update patch in that time.
20:09gandalfqDot: I was thinking about doing the split. Is it ok if I do this in bug 1346877 instead of this one? I have a couple more functionality catchups to do and then I plan to (within a couple weeks) just sort out the structure of LocaleService and add more tests without any functional changes in the &quot;finalize&quot; phase.
20:09firebothttps://bugzil.la/1346877 ASSIGNED, gandalf@aviary.pl Finalize LanguageService
20:09qDotgandalf: Sure! That&#39;s why I put it as a comment but not a requirement.
20:09gandalfthank you :)
20:09gandalfyeah, I should be able to apply your feedback in the next hour then :)
20:11gandalfthe only difference I see is that that would remove the ability to switch modes mid-flight. Which I&#39;m not sure if we will ever need anyway, but worth noting
20:13qDotYeah, that&#39;s why I was tentative about the suggestion. Since I&#39;m not super familiar with l-big-number-n, didn&#39;t want to blindly hand out advice with an air of confidence. :)
21:10gandalfqDot: one question
21:10gandalfhow do I mark `LocaleService::GetInstance()->SetAppLocales(aXPCOMInit.appLocales());`
21:10gandalfto make it pass the &&nsTarray ?
21:11gandalfinstead of &nsTarray ?
21:11froydnjgandalf: SetAppLocales is defined in xpidl?
21:12gandalfno
21:12froydnjoh good
21:13froydnjgandalf: what does appLocales() return?
21:14gandalfit worked with &nsTArray
21:14gandalfsorry, nsTArray<nsCString>&
21:14gandalfbut qDot asked me to switch it to nsTArray<nsCString>&&
21:15gandalfwhich errors with https://pastebin.mozilla.org/9019562
21:16qDotOk. Yeah. That was one of those things I might&#39;ve been wrong about.
21:16gandalfI thought that just &quot;&(aXPCOMInit.appLocales())&quot; would do
21:16gandalfbut it errors
21:17gandalfbecause then it&#39;s &quot;const nsTArray<nsCString>*&quot;
21:17qDotOH. Yeah. Ok. Nevermind. Ignore my review comment.
21:17qDotI missed that you were calling that from two places.
21:18qDotI was only looking at the IPC message, not the aXPCOMInit object access.
21:20qDotgandalf: Ok. Updated review. Sorry about that. :)
21:21gandalfdid you?
21:21qDotOh. Yeah. Publish button.
21:22* qDot never gonna get used to mozreview flow.
21:22gandalfok!
21:23gandalfthanks :)
21:27kanrubaku|away or smaug: how do I know which page is using large Blobs when I see more than 1G memory-file-data in about:memory?
21:29smaugno idea
21:29smaugCC log doesn&#39;t tell the size either
21:30smaugand about:memory apparently doesn&#39;t tell the url
21:30smaugkanru: hmm, doesn&#39;t about:memory tell some url
21:30smaugat least it tries
21:32kanrudoes it? I only see sha1s
21:32smaugkanru: I see something like blob:null/b0b0d74e-3822-403b-837c-f717169a21ec
21:32smaugthat is in parent process
21:33smaugor maybe you&#39;re looking at something else
21:34kanruok, I see that too
21:34smaugbut I&#39;m not too familiar with about:memory
21:34smaugnjn is
21:34kanruowner unknown/blob:null/12d3d0ed-c8bd-4e52-9f9a-91b8083f812c
21:34smaugmccr8 might be too
21:34mccr8I don&#39;t know how the blob reporter stuff works.
21:35smaugI wonder if it reports file-blobs in parent process using null principal
21:35kanruI&#39;m talking about 528.62 MB (19.49%) file(length=5469, sha1=52b8ee960c135b328c564a21d2fa86182c6f84cd) [67663]
21:35smaugoh, that kind of thing
21:35smaugkanru: in which process is that?
21:35kanrusmaug: parent
21:36smaugkanru: perhaps worth to file a bug to improve reporting
21:36smaugCC njn and baku ?
21:36kanruok, will do
21:37smaugkanru: I think at some poing Google Suite was using quite large blobs
21:37smaugpoint
21:37kanrufor now I think I can gdb attach to it and see what&#39;s there
21:38smaugI&#39;m not sure whether parent process knows the domain from which the blob is coming from.
21:57bzehsan: ping
21:57bz!seen ehsan
21:57firebotehsan was last seen 2 hours and 57 minutes ago, saying &#39;billm, ping&#39; in #content.
21 Apr 2017
No messages
   
Last message: 93 days and 37 minutes ago