mozilla :: #content

18 Apr 2017
00:00gandalfusually when I did it's called "leaking" and is frowned upon
00:12gandalfsmaug: so this is what I have now
00:12gandalfand I'm getting:
00:12gandalf 1:22.69 ../../build/unix/gold/ld: error: /home/zbraniecki/projects/mozilla/mozilla-unified/obj-x86_64-pc-linux-gnu/toolkit/library/../../js/xpconnect/src/Unified_cpp_js_xpconnect_src0.o: requires dynamic R_X86_64_PC32 reloc against '_ZTV17XPCLocaleObserver' which may overflow at runtime; recompile with -fPIC
00:12gandalf 1:22.69 ../../build/unix/gold/ld: error: read-only segment has dynamic relocations
00:12gandalf 1:22.69 /home/zbraniecki/projects/mozilla/mozilla-unified/js/xpconnect/src/XPCLocale.cpp:27: error: undefined reference to 'vtable for XPCLocaleObserver'
00:12gandalf 1:22.69 ../../build/unix/gold/ld: the vtable symbol may be undefined because the class is missing its key function
00:12gandalfnot sure what key function am I missing :(
00:14smaugthe pastebin has only parts of the patch, I assume
00:15gandalfI bet it's the NS_IMPL_ISUPPORTS
00:15smaugstrcmp(aTopic, "intl:app-locales-changed") is wrong. Should be !strcmp(aTopic, "intl:app-locales-changed")
00:15gandalfdoesn't it return 0 when matches exactly?
00:16smauggandalf: and you expect that it returns non-0
00:17smaugif (strcmp(aTopic, "intl:app-locales-changed")) {
00:17smaugthat should never be true in the current patch
00:17smaugsince you observe only intl:app-locales-changed
00:17smaugmeaning that strcmp returns 0
00:17smaug== false
00:17gandalfagh, correct
00:18gandalfthanks, didn't get to the point where I could test if it works yet :)
00:44smauggandalf: you should ask also someone from js team to review. it is unclear to me what happens in JS engine when JS_SetDefaultLocale is called after initialization
00:45gandalfnot much -
00:45gandalfjust that the next call to "new Intl.DateTimeFormat(undefined);" will use the correct locale :)
00:46gandalfsmaug: but sure, if you can review the xpconnect bits I'll get someone from the SM team to confirm that it's ok to update locale :)
01:02froydnjsmaug: it sounds like your problem is the blame tool, not the commit message...
01:45bzfroydnj: even later reping
06:01gandalfjessica: I think I'll have the patches for you landed this week
06:01gandalf(for you == for the datetime picker content side locale )
06:02jessicagandalf: that's great! I'll test it again once it is landed.
06:02jessicagandalf: thanks for fixing it.
06:03gandalfn/p. I hope it didn't delay your work too much. The locale selection in Gecko has been an... interesting thing to clean up.
13:29froydnjbz: now that both of us might actually be at the computer simultaneously...
14:25bkellyweird... the time in irccloud is off
14:25bkellyoh, its not... I'm just stupid
14:26* bkelly has that problem a lot...
14:31mconleyehsan: spilled coffee - there in 2 min
14:32ehsanmconley: kk
14:45bkellyasuth: this is what I am doing for the child process selection-or-creation:
15:36bz_sleepfroydnj: ping
15:41froydnjbz: pong
15:41froydnjbz_sleep: pong, even
15:41bzfroydnj: so are you trying to rebase the timer data structure patches, or should I?
15:41bzfroydnj: Just don't want us to duplicate work.
15:42froydnjbz: I am OK with you doing it
15:43bzfroydnj: OK, I can do it.
15:45froydnjbz: thanks
15:46bkellybz: ping
15:48bzbkelly: in a meeting
15:48bzbkelly: gimme 15 mins?
15:51bzbkelly: ok, with you now
15:51bzbkelly: What's up?
15:51bkellyfroydnj: do you know if we have any helpers like the ones in this SO answer?
15:51bkellybz: well, I find myself in a difficult position with ErrorResult and trying to find a solution
15:52* bz listens
15:52bkellyI've written my new dom Clients code using c++ lambdas
15:52bkellyI want to pass error values through the lambdas
15:52bkellybut c++11 lambdas do not support capturing by move
15:52bkellywhich makes it difficult to use ErrorResult
15:53bkellyI just found this SO answer, though... maybe its the right solution
15:53bkellya thing that fakes a Move capture for c++11 lambdas
15:54froydnjbkelly: what is "this SO answer"? did you mean to link it?
15:55* bz thinks
15:55bkellyfroydnj: ^^^
15:55bkellythe first answer has two workarounds
15:56bkellyhmm, although, I think I have places where the lambda is evaluated twice
15:56bkellybz: if I had some kind of serialized type I could "freeze" the ErrorResult into, that would be ok
15:56froydnjbkelly: I don't think we have any kind of those helpers
15:57froydnjbkelly: I know gsquelart and...xidorn, maybe?...have complained about not being able to use C++14 lambdas for this exact feature
15:57bkellybz: I'm using nsresult today, but I really want to pass informative TypeError messages
15:57bzbkelly: so ignoring for the moment ErrorResult and lambdas and whatnot
15:57bkellyfroydnj: do you know how close we are to c++14 lambdas?
15:57bzbkelly: What is the conceptual information flow here?
15:58bzbkelly: Sorry for the vague question; just trying to understand things before I start trying to come up with solutions....
15:58bkellybz: I have a cross process operation I am performing... code in process X creates a series of actors that do something in process Y... that action in process Y can throw a TypeError that I need to report in process X
15:58froydnjbkelly: we need to get some android things sorted out before we can use c++14 everywhere
15:59froydnjbkelly: I have a Q2 goal for most of the android stuff
15:59bzbkelly: OK.
15:59bkellybz: I am using lambdas with MozPromise to make working with all the actors easier
16:00bzbkelly: So the Y-side actor puts an ErrorResult on the stack, say, and calls some function that has an ErrorResult outparam
16:00bzbkelly: Is the MozPromise X-side or Y-side?
16:00bkellybz: I have MozPromises on both X and Y side... its a long chain of plumbing... it also goes through the parent process between X and Y
16:01xidornfroydnj: that's one of features I complained for... another one around lambda is generic lambda. I think I wanted to use that somewhere...
16:02jibI'm just curious: What's the reason to disallow passing ErrorResult by copy?
16:02jibby value
16:02bzjib: So people can't fuck up
16:02jibsuch as?
16:02bzjib: ErrorResult foo; DoBar(foo);
16:02jibah :)
16:02bzjib: Where DoBar is defined as void DoBar(ErrorResult arg)
16:03jibsecond arg syndrome
16:03bzjib: not second arg syndrome
16:03bzjib: "oops, I made a copy instead of taking a reference"
16:03bzSo that's why no explicit copies
16:03jibmakes sense
16:04bzer, implicit copies
16:04bzWe could have some sort of explicit cloning thing
16:04bzBut keep in mind that ErrorResult is conceptually representing an exception
16:04bzWhat does it mean to clone it?
16:04bkellybz: we do have an explicit clone... but not sure how to use it for this use case
16:04bzAh, we did add the explicit clone, yes
16:04bzbkelly: So next question.
16:05bkellybz: in this case all of my errors are set natively... so I don't have any js stacks
16:05bzbkelly: this ErrorResult that the lambda wants to capture...
16:05bzbkelly: Does it plan to _read_ it, or _write_ it?
16:05bkellybz: I believe read it
16:06bz(good, because the other was not making sense to me at all ;) )
16:06jibI&#39;ve also been waiting for move in lambdas in C++14. You could use terrible tricks like Refcountable<UniquePtr<ErrorResult>> - I&#39;ve used that in places to fake passing by value -
16:06jibnot pretty
16:06bkellybz: oh, I am remembering another problem here
16:06bkellybz: I do use an ErrorResult in Cache API here:
16:07bzbkelly: I mean, so far I&#39;m thinking in terms of a holder struct that you can capture by value and whose copy ctor clones the ErrorResult
16:07bkellybz: but you can only use an ErrorResult in an argument directly... if you embed it in an ipdl struct or union it fails to compile because those types require a copy constructor :-(
16:08bzSeems like a similar problem
16:08bzwe want a thing that has a copy ctor
16:08bzAnd can store an ErrorResult
16:08bzI agree this is super-annoying....
16:09bzOne additional issue is that once you allow copy construction you can have temporaries lying around
16:09bkellybz: maybe a ReadOnlyErrorResult type
16:09padenotannevk, ping
16:09bzAnd ErrorResult temporaries would tend to assert on destruction
16:09bzbecause you did not handle the error
16:09bkellybz: ReadOnlyIgnoredErrorResult
16:10bkellybz: ok, I&#39;ll think about writing that
16:10bzThank you
16:11bkellybz: maybe ReadOnlyErrorResult could imply the &quot;ignored&quot; behavior... and forbid js stacks, etc
16:11bzFwiw, there is precedent for this sort of thing; c.f. OOMReporter
16:11bzbkelly: yeah
16:11bz(I mean, OOMReporter is quite different in the details, but conceptually similar somehow in my head)
16:13jibbz: Speaking of js stacks in errors, I have an issue I wanted to run by you:
16:14jibactually two issues
16:16* jib needs to take a call. whoops
16:19firebotBug 1357463 ASSIGNED, create a copyable read-only ErrorResult to make reporting easier across IPC actors
16:24bzjib: Ok, lemme know when you&#39;re back
16:25jibok thx
16:32gandalfsmaug, bz: sorry to bother you guys, but my intl reviewer said that bug 1348042 is outside of the scope of his expertese and I need to find someone with dom/ipc background to review it :(
16:32firebot ASSIGNED, Make LocaleService operate in client-server mode
16:32gandalfwould you be willing to review it or can you suggest the right person?
16:33annevkpadenot: somewhat pong, I can probably write back later today if you leave me something concrete
16:33padenotannevk, I got hold of lth in #developers in the meantime, thanks, clarifying things about [[CanWait]]
16:34smauggandalf: how is baku|away or qDot&#39;s review queues?
16:34smaug(we really need better tools for looking at who has been reviewing a lot recently)
16:34smaug(review queue length is rather useless measurement)
16:35gandalfbaku is on 9/30 and qdot on 3/1 (review/needinfo)
16:35gandalfwhich may either mean that baku is more busy or qdot is better at maintaining a clean queue
16:35gandalfok, I&#39;ll ping them and see if either is available. Thank you!
16:36gandalfqDot: ping
16:37annevkpadenot: cool
16:53jibbz: ping
16:54jibIs it bad for internal file and line numbers to show up in expanded error objects in web console? -
17:04bzjib: so....
17:04bzjib: That sounds much like
17:04firebotBug 1356643 NEW, Stack traces shown in console include self-hosted frames
17:05bzjib: it seems like a regression to me....
17:05bzjib: In that I thought we had explicitly made that not be the case
17:05jibI can look for a regression range if you like, but I actually had a similar but unrelated question also,
17:05bzjib: It&#39;s certainly highly unexpected from a web developer point of view
17:05bzjib: fwiw, I tried something like Firefox 45 on and it had the same issue, iirc
17:06bzjib: so if I&#39;m right about this being the same issue, chances are it&#39;s not very recent. :(
17:06bzjib: Listening
17:06jibWe&#39;re also having a bit of trouble with errors in PeerConnection.js lately. It&#39;s a pain to work there because whenever we mess up, we don&#39;t get any good error messages anymore, even in browser console.
17:07bzjib: Might have regressed with bug 1038238 ...
17:07firebot FIXED, Make Error instances use SavedFrame for stacks
17:07bzjib: hmm
17:08bzjib: do you have a testcase that has a messup without corresponding error message?
17:08bzjib: Happy to debug if that would be useful....
17:09jibWell here&#39;s a problem description at least in pastebin
17:09jibBasically, if we mess up anything in PeerConnection.js we get this crypting error message:
17:09jibPromise rejection value is a non-unwrappable cross-compartment wrapper.
17:12bzSo this got broken by the switch to SpiderMonkey promises
17:12bzbecause DOM promises handled this better. ;)
17:13jibShould I file a bug?
17:13bzone sec
17:13bz> web console: Promise rejection value is a non-unwrappable cross-compartment wrapper.
17:13bzThat&#39;s ... moderately broken. :(
17:13bzBut not clear what else it could do
17:14firebotBug 1306200 NEW, Promise code should log original exception when it sanitizes exceptions
17:14bzI reported it for you already.
17:14bzBack in September....
17:15bzPoke till? :(
17:15bzTo either fix or delegate?
17:15bzI could probably write a fix for this, depending on the API shape the SpiderMonkey folks are OK with....
17:15jibAny workarounds?
17:16bzFor your local tree, or for your .js file?
17:18bzjib: ^
17:19jibfor getting better internal errors when working on PeerConnection.js
17:20bzYes, but what I meant is are you looking for a workaround in terms of a patch applied to spidermonkey or in terms of how you write your .js or something else?
17:22jibIn how we write our js
17:22bzhave explicit try/catch with logging around anything that you pass a then() callback to a content promise...
17:23bzhuge PITA
17:23jibI tried writing that with no success
17:23bzlet me see if I can just write a fix here. One sec.
17:24bz(or rather copy/paste some code)
17:31qDotgandalf: pong
17:31qDotgandalf: Yeah, you can hand the review to me, I&#39;ll kick it over to baku if I&#39;m not qualified.
17:32gandalfqDot: you&#39;re right next to my desk! I can guide you through on a whiteboard :)
17:32qDotgandalf: WHY ARE YOU IN MY HOUSE oh you mean at the office I never go to
17:32gandalfqDot: I&#39;m going to wait with applying :jfkthame&#39;s feedback until I get yours if that&#39;s ok with you
17:33qDotgandalf: Cool, no prob.
17:33gandalfqDot: come to the office! It&#39;s fun and all. And they have food
17:33qDotgandalf: Yes, it has food, but it also has people, and those are scary.
17:36bzjib: try ?
17:36jibbz: thanks! will try!
17:37bzjib: If that has sane behavior I will clean it up and get it reviewed and all that
17:37* bz did more copy/paste than he&#39;s ok landing
17:37jibwill let you know asap!
17:40sheppyWe have a doc bug asking if this sentence is correct. Anyone know if it is? &quot;Stylesheets are considered active content, so content normally considered passive referenced from a stylesheet is considered mixed passive/display content.&quot;
17:40sheppyThe bug says that the last bit should say &quot;... is considered active content.&quot;
17:40bzSounds like a #security question....
17:41sheppybz: Whoops... I thought I&#39;d clicked over to there. :)
17:41bzsheppy: ;)
17:42gandalfqDot: oh, that...
17:49jibbz: \o/
17:52gandalfqDot: anyhoo, let me know if you have questions. I know l10n/intl is scary and complicated and complex. I believe we have a consensus within the intl group on that approach, so now it&#39;s mostly about making it clean and sane from the C++/IPC perspective
17:58bzjib: great. did the browser console thing have a useful stack or at least file/line too?
17:59jibbz: Yes, sorry I cut&#39;n&#39;pasted wrong: TypeError: &quot;&quot; is not a function PeerConnection.js:1230:7
17:59bzOK, let me clean this up, etc
18:03bzThank you for poking this
18:03bzThe fact that it&#39;s sat this long is silly
18:05jibGlad to have a fix coming. We&#39;ll be working a bit in this area now, so this will be quite useful
18:06qDotgandalf: Yeah, I&#39;ll take a look! And if it does get hairy, I can stop by the office and talk about it too. Need to get used to talking to actual people again before the all-hands.
18:07gandalf:) sounds good!
18:18bkellyannevk: from this github issue, what does &quot;They can of course continued to be GCd as long as that is not observable, not even from the network.&quot;
18:19bkellywhat does that mean? ^^^
18:19bkellyit seems to me that worker GC is somewhat observable and that can&#39;t be changed
18:20annevkbkelly: how is it observable?
18:20bkellyannevk: I can think of a number of ways
18:20annevkbkelly: teach me
18:20bkellyannevk: probably most precise is: 1. get a worker that is the last controlled client of a service worker 2. register a new SW and set a state change handler on it 3. when worker is GC&#39;d old SW is removed and new SW is promoted 4. event on GC
18:21bkellymodulo async timings, etc
18:21annevkbkelly: can you do it without service workers?
18:21annevkbkelly: I&#39;ve raised the GC concern a couple times about service workers, but nobody really seemed to care
18:21bkellyannevk: it depends what you mean by &quot;observable&quot;... you could totally have a worker that sends postMessages to window and the window can infer when the GC happens because the messages stop
18:22bkellyannevk: or broadcast channel if you prefer
18:22annevkbkelly: how would GC happen if the messages keep coming?
18:22bkellyannevk: what about using an API like broadcast channel would keep the worker ref&#39;d?
18:23annevkbkelly: I guess BroadcastChannel doesn&#39;t at the moment, maybe that&#39;s a bug
18:23annevkMessagePort definitely does
18:23bkellyannevk: through the source attribute on the events?
18:23annevkI&#39;ll file an issue on BroadcastChannel
18:24annevkbkelly: the global just keeps track of created MessagePort objects and whether they have listeners attached
18:24annevk(although the listener part isn&#39;t defined in detail and that should probably be fixed
18:25bkellyannevk: so your intent is that dedicated and shared workers can only be destroyed via GC if they are idle? they can busy work themselves to stay alive?
18:26annevkbkelly: you might want to discuss the service worker scenario with smaug
18:26annevkbkelly: perhaps he has ideas on how to address that or maybe he&#39;s okay with embracing GC
18:26bkellyannevk: Clients API also lets you poll to determine when a Worker is GC&#39;d
18:26annevkbkelly: sigh
18:26bkellyI&#39;m all for reducing observability of GC... but I think there are practical limits
18:27annevkbkelly: I&#39;m a little bit annoyed that the service worker folks don&#39;t listen to &quot;don&#39;t make GC observable&quot; and Alex Russell just dismissing that completely as if it&#39;s nothing
18:27annevkAnyway, too late to get too worked up about this
18:28smaugI don&#39;t know ServiceWorkers well enough, but isn&#39;t that all exposing UA specific behavior.
18:28smauglike all the stuff when the worker dies
18:28* smaug is missing the context
18:29bkellyannevk: well, we used to have a close event when the worker died... so not sure we ever felt too strongly about it
18:29annevksmaug: 1) does BroadcastChannel expose GC for dedicated/shared workers and should it 2) how much should we care about service workers destroying the GC invariants we&#39;ve tried to keep intact for so long
18:30annevkbkelly: there&#39;s a reason we removed it and didn&#39;t bring it back (and a reason it was never standardized...)
18:30annevkbkelly: there&#39;s definitely folks that care
18:31bkellyannevk: I care too... but I think an 80% solution is probably good enough and getting the last 20% would be quite hard and constraining
18:31smaugbaku: you might recall whether our implementation of broadcastChannel keeps dedicated/shared workers alive
18:31smaugIMO that should be happening
18:31bkellyannevk: what about a worker holding an IDB transaction so that you can tell when the worker is GC&#39;d when the window&#39;s transaction unblocks?
18:33smaugannevk: since I don&#39;t really understand ServiceWorkers lifetime management, hard to say much to 2). But exposing GC behavior is most probably going to cause issues, web sites relying on some behavior.
18:33annevksmaug: :/
18:34annevkbkelly: you mean that writing to disk should keep the worker alive if we wanted to avoid that?
18:35bkellyannevk: I&#39;m saying that if you write a worker that creates an IDB transaction and just holds it open... and then a window also tries to open a transaction on the same DB... you can tell when the worker&#39;s transaction is closed via the worker GC
18:35annevkbkelly: could you tell whether it was just terminated (which I guess is also allowed for whatever reason) or GC&#39;d though?
18:35bkellyannevk: no... all of these things are a bit contrived which is why I think they are ok
18:36bkellyannevk: I don&#39;t think any of these things can tell the difference between terminated and GC... they just know the worker went away, not why it went away
18:36annevkThere&#39;s a similar thing for a network request
18:37annevkbkelly: anyway, I&#39;ll make the commit message once it&#39;s rebased and all less dramatic
18:37bkellyok, I commented in the PR
18:37annevkbkelly: it does seem like something we should investigate a bit and close any easy gaps
18:38annevkoh, it doesn&#39;t even need to be rebased
19:00bakusmaug: yes it does.
19:11bkellybz: do we track that a user has made a recent interaction anywhere in window, etc?
19:11bzbkelly: I don&#39;t know
19:12bkellybz: seems EventStateManager::IsHandlingUserInput() may be what I want
19:14mystorbkelly: That checks that a user input event is literally on the stack
19:14bkellymystor: yea... probably what I want
19:14mystorbkelly: What are you wanting to use it for?
19:14bkellymystor: to know if its ok for an API to open a window
19:15mystorbkelly: Yeah, that&#39;s probably what you want then
19:15bkellystep 4 here:
19:15bkellywritten for just service workers there... but trying to make this usable from main thread in future
19:15bzBest to use the popup blocker state, no?
19:16bzThat&#39;s what it&#39;s there for....
19:16bkellybz: where is that?
19:16* bz looks it up
19:18bzIs youre code associated with a window
19:18bzer, your
19:18bzSo the way does this is like so....
19:18bz PopupControlState abuseLevel = gPopupControlState;
19:18bz abuseLevel = RevisePopupAbuseLevel(abuseLevel);
19:19bz(where RevisePopupAbuseLevel is an nsGlobalWindow method)
19:19bzThen if abuseLevel >= openAbused we bail out
19:20bzSee and following
19:20bzNote also that if abuseLevel < openAbused but is openControlled we do some bookkeeping
19:20bzAnd that this bookkeeping actually needs to be passed to the window watcher bits....
19:20bz(the isPopupSpamWindow stuff at and following)
19:21bzAll of which is to say, can you reuse this code at all? ;)
19:21bkellybz: not sure without some refactoring
19:21bkellybz: I might just leave this a TODO for now... since this API isn&#39;t actually exposed on main thread yet
19:22bzAnyway, we want it to integrate with the general popup blocker machinery
19:22bznot add a back door for it
19:22bkellybz: well, today its hooked from notificationclick event in service worker only
19:22bkellyonly allowed from notificationclick in SW
19:33bkellyI have to say I really hate this api
19:50bkellyasuth: do we have a relatively simple async storage helper wrapper for IDB for use in mochitests?
19:50asuthbkelly: landed recently?
19:51asuthbkelly: devtools also has mebbe
19:51asuthoh, no, that has require in it. no go
19:52bkellyasuth: I&#39;m tempted to just abuse Cache API
19:53asuthbkelly: you could use jugglinmike&#39;s SW paranoia IDB helper:
19:54asuthbkelly: but yes, you should abuse the Cache API.
19:54asuthbkelly: also, localStorage can be fun to abuse.
19:56bkellyasuth: localstorage isn&#39;t available in SW, right?
19:57bkellyasuth: I think the best solution here would actually be to just echo stuff to the main test window and let it maintain the global state
19:58asuthbkelly: yeah, localStorage/sessionStorage are only hung off of nsGlobalWindow.
19:58asuthfor now! Jonas can&#39;t stop this innovation train! moohahahaha
19 Apr 2017
No messages
Last message: 11 days and 11 hours ago