mozilla :: #devtools

7 Aug 2017
08:26nchevobbeochameau: ping
08:40ochameaunchevobbe: pong
08:40nchevobbeochameau: Hello Alex, I have a server-related question
08:41nchevobbeochameau: So I'd like to use https://dxr.mozilla.org/mozilla-central/source/js/src/doc/Debugger/Debugger.Object.md#186 in the Function preview (http://searchfox.org/mozilla-central/source/devtools/server/actors/object.js#1151-1192) of the objectActor
08:42nchevobbebut it always returns undefined, even when loggin an async function
08:42nchevobbeso I think there's something I don't understand here
08:45ochameauobj.isAsyncFunction should be what you would expect
08:45ochameaudo you have a link to your test?
08:46nchevobbeI'm just evaling `async function x(){};x;` in the console input. reps already handle the property i'm trying to include in the grip
08:47ochameaucan you try with "isArrowFunction" see if that works for checking arrow function, or if that's really only async function which are broken
08:51nchevobbeochameau: isArrowFunction returns undefined too, as well as isGeneratorFunction
08:52ochameauI'm starting to wonder if that's broken at c++ level
08:52ochameauhere is where it is being implemented
08:52ochameauhttp://searchfox.org/mozilla-central/source/js/src/vm/Debugger.cpp#10264
08:52nchevobbewhen I add a breakpoint, I do see those properties in obj
08:53nchevobbecould it be that MOZ_ASSERT(isDebuggeeFunction()); isn't satisfied ?
08:55ochameaumay be as that looks common to all these functions
08:56ochameauRemoveAsyncWrapper usage looks also common
08:58ochameauhave you ever touched firefox's c++?
08:58nchevobbeonce, and it was very simple :)
08:58ochameauif you don't have the bandwith to look into that, jimb may help you
08:59ochameauI would suggest either doing a quick gdb session or even simplier, putting a couple of printf()
08:59nchevobbeyeah, i'll look what I can do and ask jim if I can't do much :)
09:00ochameaulike: printf(&quot;isFunction(%d)\n&quot;, (int)referent()->is<JSFunction>());
09:01ochameauyou are a console contributor so you may be more into console logs rather than using a debugger ;)
09:01ochameautbh, using gdb isn&#39;t trivial, especially for printing values in a human readable way
09:02nchevobbeochameau: okay, thanks !
09:03ochameauI confirm that I get the same results. it seems to work fine while being in a breakpoint
11:52julienwnchevobbe, do you have plans in reps to show how many elements are ellipsified (like when an array has more than 3 elements, have [ element1, element2, element2, ... 3 more elements ] instead of just &quot;...&quot;
11:53julienwnchevobbe, same for objects (esp when they are array items): { ... 5 properties } instead of { ... } (and better yet: { prop1, prop2, prop3, ... 2 more properties })
11:56julienwI can file an issue but I wanted to know if you already had plans :)
12:32jdescottesjulienw: related -> https://github.com/devtools-html/devtools-core/issues/447
12:44julienwjdescottes, thanks
12:45julienwjdescottes, do you think I should file a bug ?
12:46julienwjdescottes, the goal is to be able to understand what the logged element contains without expanding it; with the current setup I have to expand it to know anything
12:51jdescottesjulienw: I think it&#39;s worth filing to iterate on this. For objects showing the number of properties was often wrong/miscalculated but for arrays I think it&#39;s worth discussing
13:03julienwjdescottes, I think I&#39;ve seen it somewhere already (maybe chrome ?)
13:16nchevobbejulienw: the plan was to get rid of those things because I see little value in it. Willing to change my view though
13:17nchevobbejulienw: note that this is only for TINY mode (nested element in console, scopes in the debugger)
13:17julienwnchevobbe, yep
13:17julienwnchevobbe, for example I have a very big array of elements (renders: Array [ {}, {}, {}, {}, {}, {}, {}, {}, {}, {}, ])
13:18nchevobbejulienw: yes, I think we don&#39;t have the data in the grip
13:18julienwnchevobbe, when I expand it, because it&#39;s very big, I have the it subdivisied as 0-99 etc (note: I like this !). And so I have to expand once more to have a feeling of what it&#39;s about
13:19nchevobbejulienw: previously, it was Array[Object, Object, ]
13:19nchevobbeso you&#39;d have to expand it too
13:19julienwyeah, I don&#39;t say it&#39;s something new :)
13:19nchevobbejulienw: so one thing that would help is to use console.dir
13:19nchevobbeso your object is already expanded
13:19julienwbut what I get in the console is useless
13:20julienwyeah well I don&#39;t want it compeltely expanded either, it&#39;s enormous :)
13:20nchevobbeit only expand the first level
13:20nchevobbeso it might be enough ?
13:20julienwmaybe, but I also don&#39;t control all the code here
13:21julienwso I think there are 2 things that I don&#39;t liek
13:21julienw1. I don&#39;t know if the array is somewhat big or very big, I need to expand it; I think this is easily fixable ?
13:22nchevobbejulienw: would you not expand it if it was not that big ?
13:22julienw2. I don&#39;t know what&#39;s in the array because the object content is hidden -- I can understand why and I don&#39;t have a good suggestion now. Maybe doing something on hover ?
13:23nchevobbejulienw: yeah, hover could show the number of elements
13:23julienwis there a downside showing the number of elements right away ?
13:23nchevobbebut not the content I think (without changing how we do things in the server, which are done this way for perf reason I think)
13:24nchevobbejulienw: it might clutter things
13:24julienwwell, I think it brings clarity on the contrary :)
13:24nchevobbejulienw: if it&#39;s not on hover I mean
13:24julienwsee the rendering above: this is clutters because that&#39;s only symbols I need to decipher
13:24nchevobbejulienw: so your example would translate to Array [980 items]
13:25nchevobbejulienw: Let&#39;s debate this on a GH issue so we can get the pros and cons and what we can do :)
13:25julienwno, more like Array [{...}, {...}, and 978 more]
13:25julienw:)
13:26julienwyep
13:26julienwI&#39;ll file a bug
13:27nchevobbejulienw: okay, thanks :)
15:08tromeyochameau: I feel like there was some discussion of dom promises and async stacks when promises landed
15:08tromeyochameau: I don&#39;t recall the details but I think you should NI whoever wrote that stuff
15:12ochameautromey: any idea who?
15:13tromeyI can&#39;t remember now
15:13tromeysorry about that
15:14tromeyok, I do see some stack-capturing code in there
15:14tromeysee PromiseObject::onSettled, e.g.
15:14tromeyor CreatePromiseObjectInternal
15:16ochameauyes but it seems to be only used for PromiseDebugging, not to augment JS stacks
15:18tromeyok, it is complicated, but from reading the code I think it should happen
15:19tromeythere&#39;s a way for the embedding to tell JS how to enqueue promise jobs
15:19tromeysee JSRuntime::enqueuePromiseJob
15:19tromeyand JS::SetEnqueuePromiseJobCallback
15:19tromeythis is called from xpcom
15:19tromeyCycleCollectedJSContext::InitializeCommon
15:20tromeyxpcom makes a PromiseJobRunnable
15:20tromeyPromiseJobRunnable makes a PromiseJobCallback
15:20tromeythis is defined in dom/webidl/Promise.webidl
15:20tromeyand inherits from CallbackFunction
15:20tromeywhich handles async stack parenting
15:22ochameauahah, so simple :)
15:22ochameaubut then, it looks broken, non?
15:22ochameaudo think think the behavior I&#39;m seeing is the expected one?
15:23tromeywhat I currently don&#39;t understand is why test_executeSoon expects to see waitForTick on the stack
15:23tromeydoes that make sense to you?
15:24ochameautbh, I don&#39;t see how any of this work at all
15:24ochameauI&#39;m puzzled to see the stack beyond the &quot;yield waiForTick&quot;
15:24ochameaubut it kind of make sense and is super useful as you really see the whole stack
15:26tromeyI guess my mental model is that stack is captured at &quot;Components.stack&quot; and that waitForTick isn&#39;t even on the stack at that point
15:26tromeythat&#39;s what I don&#39;t understand
15:26tromeythe test talks about server frames but I don&#39;t understand that either, since I don&#39;t see anything there calling the server?
15:27tromeylike, maybe if waitForTick was capturing the stack, then it would make sense to me somehow
15:28tromeyis the idea supposed to be that the yield introduces an async parent?
15:28tromeyI find that kind of hard to wrap my head around
15:28tromeylike I guess it is true under the hood but really non-obvious
15:30ochameauif you remove Task/yield, I think such stack make sense
15:31ochameauit just get much harder to follow with Tasks
15:31ochameauwe do use this magic Cu.callFunctionWithAsyncStack in a couple of places in devtools
15:31ochameauso I imagine we have some test to check that:
15:31ochameauhttp://searchfox.org/mozilla-central/search?q=symbol:%23callFunctionWithAsyncStack&redirect=false
15:34tromeyI suppose my view is that this test is actually testing behavior that is not under our control
15:34tromeyexactly what stack to present is a platform decision made by other people
15:35tromeyand in any case I don&#39;t think the new behavior is obviously bad or something
15:35tromeywell, maybe it is partly bad, because it doesn&#39;t mention test_executeSoon.js, which is truly surprising
15:36tromeyscratch that, it does, I just missed that line :(
15:36tromeyanyway to sum up, I would be inclined to change the test and move along
15:36tromeymaybe file a bug against spidermonkey promises if you think it&#39;s really incorrect
15:37ochameauyes, I&#39;ll at least uncouple defer from the rest as things are simplier for others
15:43nchevobbeHonza: I think you want bugs 1384965, 1384892 to be blocked by a new devtools-context-menu bundle (if there&#39;s one, I don&#39;t know it well)
15:46Honzanchevobbe: ah, this is only for reps
15:46nchevobbeHonza: okay, it seems like the netmonitor uses the package only for the launchpad I guess ? Then you&#39;d have a bugzilla bug &quot;bump devtools-contextmenu to new version&quot; that would block those 2 bugs
15:47Honzanchevobbe: yes, it&#39;s for the Launchpad
15:47Honzanchevobbe: And who&#39;s usually building new module and uploading to npm?
15:48nchevobbeHonza: https://github.com/devtools-html/devtools-core/commits/master/packages/devtools-contextmenu suggests Jason :)
15:48nchevobbeHonza: but we should all be able to publish devtools-html modules to npm if I got bryan emails right
15:49Honzayeah, I saw it too
16:07ochameaunchevobbe: did you had any luck with the printfs/gdb?
16:07nchevobbeochameau: sort of, I was able to see that they only return true when paused in the debugger
16:08nchevobbeochameau: Since there is this thing about the referent being a debugee function, I guess it&#39;s by design
16:24ochameaunchevobbe: so was that isDebuggeeFunction() which is always false?
17:26jimbochameau: What&#39;s the best way to catch a mochitest child process crash in a debugger?
17:26jimbI thought there was some sort of &#39;debugger&#39; option in the &#39;mach test&#39; harness, but it doesn&#39;t seem like there is one.
17:27jimbAh, &#39;mach test&#39; doesn&#39;t support debugging, but &#39;mach mochitest&#39; does!
17:29tromeyyeah, mach test only supports some subset of the options
17:29tromeyannoying from time to time
17:30ochameauI&#39;m still using &#39;mach mochitest&#39; and &#39;mach xpcshell-test&#39; everytime. I heard about &#39;mach test&#39; long time ago and forgot about this!
17:30jimbI knew I&#39;d seen it somewhere...
17:31jimbtromey: More confusing than annoying
17:32ochameaujimb: the good thing is that now, we have a team working on these confusing/annoyings pieces \o/ cc: bgrins ;)
17:32tromeyat least in earlier times I never knew which specific *test subcommand to use
17:32tromeyI think there used to be separate mochitest-chrome and others?
17:35ochameaubgrins: ./mach test --jsdebugger isn&#39;t supported whereas it works for both ./mach mochitest and ./mach xpcshell-test. if you are looking for easy bugs to improve workflows...
17:38ochameautromey: fyi, about the promise bugs, I&#39;m waiting for try results before r? these patches as I may find yet another major thing to address before proceeding...
17:38tromeythanks for the heads up ochameau
17:39tromeyhow do you add a patch to reviewboard with an r? but without triggering a review request?
17:39ochameaugit mozreview push, say no when it ask to publish the review requests
17:39ochameaugo to mozreview webpage, reset the reviewer input, then publish
17:39tromeyoh, is that what that means?
17:39tromeygeez
17:40ochameauif you say no, it will still push, but only you, on mozreview webpage, can see the new intermediate changeset
17:40ochameauyou have to remember going to the page to click on publish button!
17:41ochameauyou can push to try unpublished patches like this, but I prefer to show the patches ahead of time
17:52bgrinsochameau: yes, i&#39;ve been burned by this before as well (with other options, at least)
17:52bgrinsi&#39;ll see if there&#39;s a bug on file for this
17:53bgrinsi&#39;m guessing that the option isn&#39;t supported by at least one of the test harnesses which may be a reason why it isn&#39;t there.. but it&#39;s definitely inconvenient
18:03bgrinstromey: ochameau: jimb: filed https://bugzilla.mozilla.org/show_bug.cgi?id=1388117
18:03firebotBug 1388117 NEW, nobody@mozilla.org ./mach test doesn&#39;t support the jsdebugger option
18:03tromeythanks
18:03jimbthanks!
18:40tromeyQ
18:40tromeygrr
8 Aug 2017
No messages
   
Last message: 10 days and 3 hours ago