IRC Log Viewer » #firebreath » 2010-11-06

IRC Nick Time (GMT-7) Message
cygmatic 08:11 hi
taxilian 10:11 cygmatic: hey, good to see you
cygmatic: I think I have a fix for the unsigned long issue, but the best I can come up with it to make it a double if it doesn't fit inside a int32_T
cygmatic 10:11 that's what i'm working on :)
taxilian 10:11 lol. okay, I'll leave it and let you fix it then
cygmatic 10:11 though it seems i'm ending up with something that should go in 1.4, not 1.3 fix
taxilian 10:11 this is the sort of thing that you'll probably do cleaner than I would
how big of a change is it?
cygmatic 10:11 only a local change but i would want it at least tested & looked over in dev before moving it
i refactored a bit from the if-else-cascade to a (type_info -> conversionFunction) map
taxilian 10:11 this is what I was considering: http://pastebin.com/NvfJkXbK
ahh... that's probably not a bad idea
cygmatic 10:11 hm, my thought was not trying to cast it but to handle the types depending on wether they fit or not
i.e. less runtime overhead
taxilian 10:11 makes sense; the problem is that this is used for passing data into the browser, so it seems like the best way to do it is "if it can be sent, send it. if not, fail"
I guess an alternative to failing would always be to convert to string
which Javascript can basically treat like a number
cygmatic 10:11 i think i can push it in a few minutes, so just revert it if you think differently :)
taxilian 10:11 lol. okay. yeah, put it in 1.4
the really fun one to track down will be the API object not getting destructed; that implies that we are holding a reference to it somewhere that isn't getting cleared
cygmatic 10:11 yep, no idea on this one so far
and not the time to hunt it
taxilian 10:11 yeah; I'll have to look for it next week
I'm not looking forward to it
cygmatic 10:11 i definitely wish you good luck
taxilian 10:11 I'll probably need it :-/ the other issue brought up, the one with DetachedEvent not getting called, is fixed with the fix he suggested, which was the correct one
cygmatic 10:11 nice, i like it that people start to suggest more fixes
taxilian 10:11 yeah
I am much more motivated to help someone who seems to be taking some initiative on their own
the other fun issue, the one they thought was dereferencing a NULL pointer, is nearly as fun; in IE if a callback throws an exception it crashes the plugin, it seems
but I'm hoping that will be simple
cygmatic 10:11 yep, plus we don't have time to track everything down in a reasonable timeframe
if a c++ method throws a c++ exception? or js callback & js exception?
taxilian 10:11 js callback and js exception
it might be something as simple as needing to provide an EXEPINFO structure
cygmatic 11:11 hopefully :)
and you've got your midterms behind you?
taxilian 11:11 most of them
I have another to take either today or monday
that would be why I'm not tracking this down today
cygmatic 11:11 to be honest, i'm glad we don't have midterms
i've got enough to do anyway
taxilian 11:11 lol
taxilian 11:11 hey rob
physicsrob 11:11 hi richard
taxilian 11:11 the really weird thing about that bug is that it doesn't look like it's in our plugin
physicsrob 11:11 yeah I know
it's making me want to pull my hair out
taxilian 11:11 I believe you
physicsrob 11:11 did you manage to reproduce it ?
taxilian 11:11 no; I don't have a 10.5 machine to try
physicsrob 11:11 ah ok
taxilian 11:11 cygmatic does, but only at work, and he has limited time
physicsrob 11:11 I have a 10.6 box that I'm going to play around with some more
because I felt like I saw a random crash on 10.6
that was very similar
but it was before I learned how to reproduce this so regularly
taxilian 11:11 there is known to be a weird crash on Safari that we can't reliably reproduce; they could be related
physicsrob 11:11 so far I've been going through all the different commit's, doing a sort of binary search to try to find which commit introduced the bug
taxilian 11:11 not a bad approach
physicsrob 11:11 1.2.2 definitely does not have the problem
taxilian 11:11 hmm. that's still a lot of ground to cover
physicsrob 11:11 commit 639 definitely *does* have the bug
in between it's kind of hard to tell
because onload fails for a lot of those builds
and when onload fails I can't reproduce the bug
taxilian 11:11 oooh
I might know what it is, then
at least I can suggest something to look at
physicsrob 11:11 what's that
taxilian 11:11 at some (recent) point they introduced timer support in NPAPI on mac
physicsrob 11:11 right
taxilian 11:11 similarly-ish, there are cases in Safari where we can't use NPN_PluginThreadAsyncCall
so I'm doing a workaround using the timer code
if the bug seems at all related to the onload event, that could likely be related
because that's the first bit of code that uses the async callback
physicsrob 11:11 interesting
hmm
taxilian 11:11 src/NpapiPlugin/NpapiPluginModule_NPP.cpp line 128 (at least in the dev repo)
if the timers don't work right (either 'cause we're not using them right, or more likely in this case because Safari is buggy) then that would explain the crash with that dump
since the crash is on the main thread in timer code
you know a lot more about mac programming than I do, so you could probably figure out how to set a direct carbon or cocoa timer/callback
physicsrob 11:11 grr.. sorry.. only bad thing about this web client is it's easy to press the back button :)
taxilian 11:11 lol. using the web-based irc client?
hehe. yep
physicsrob 11:11 actually went back with a trackpad swipe... whoops
heh
taxilian 11:11 lol
physicsrob 11:11 uhm.. okay
I'll take a look at that
taxilian 11:11 to be honest, the PluginThreadAsyncCall stuff is so buggy on Safari that if we can find a good alternative way to do it I'd seriously consider always using it
physicsrob 11:11 hmm
so I haven't really looked at the implementation. It pushes the functor on to a stack and sets of a timer that gets executed by the main thread?
taxilian 11:11 basically, yes
it's a Queue, not a stack, for all the difference that makes
and it is threadsafe
physicsrob 11:11 ok
taxilian 11:11 ok; looks like we are *always* using that method, which may not actually be a good thing; that might be what is going on
http://code.google.com/p/firebreath/source/browse/src/NpapiPlugin/NpapiTypes.cpp#109
it should only use it if srcFuncs->scheduletimer is set
then again, obviously in this case it is set and isn't working
the problem is that sometimes Safari acts like it has PluginThreadAsyncCall and doesn't
here is a discussion on the topic: http://stackoverflow.com/questions/1931883/generating-async-javascript-events-from-browser-plugin-npapi
cygmatic 11:11 if we knew what version have that problem we could fix it for those specifically ...
taxilian 11:11 the suggestion there is to use performSelectorOnMainThread
cygmatic: I know for sure that the latest version of safari on snow leopard does, but not which others do
cygmatic 11:11 -performSelectorOnMainThread: works nicely btw
my npapi plugins use that on mac
taxilian 11:11 I just don't know how to use it; might be a good idea to use that in place fo the ScheduleTimer stuff
that may well fix the issue
cygmatic 11:11 the question is though, are current FF versions always cocoa-based?
taxilian 11:11 doesn't matter, according to the guy who suggested it
cygmatic 11:11 my plugins only target webkit atm, so i haven't extensively tested with FF
taxilian 11:11 even if it is cocoa-based, the carbon function works
and vise versa, as long as cocoa is available
I don't know for certain if that is true or not
but that's what they said
physicsrob 11:11 but you dont want to drop 64bit support
cygmatic 11:11 right, thus cocoa-based would be ideal
taxilian 11:11 you two are the resident mac dev experts =]
physicsrob 11:11 thanks for clues... I'm going to dig into this a little more
taxilian 11:11 if you have questions, just ask
we need it fixed as well =]
physicsrob 11:11 unfortunately I really need it fixed ASAP. I promised a really important customer 64-bit safari support by monday (because I had it running). I didn't realize I was losing 10.5 support
hopefully it isn't too bad
taxilian 11:11 like I said; if you need help that we can provide, let us know
where are you located, BTW?
physicsrob 11:11 I'm in california
you?
taxilian 11:11 UT
cygmatic 11:11 hm, if it really is only the async problem it shouldn't take too much time
... to test an alternative performSelectorOnMainThread based version
physicsrob 11:11 cygmatic, I would be greatly appreciative for any progress you could make on that -- I'm going to have to do a lot of research before I could start
taxilian 11:11 that's my hope; when it comes down to it, all you have to do is find a way -- any way -- to call asyncCallbackFunction on the main thread
physicsrob: I just sent you my work cell # in case you get stuck somewhere and I'm not around. Like I said; I'm going to need this functionality working within a few weeks, so it's in my best interests to get it working as well
physicsrob 11:11 thanks
I appreciate that community effort with firebreath -- a lot
cygmatic 11:11 physicsrob, i'll see that i get the alternative getNpvariant() commited soon then i'll see wether i can at least add a quick hack
taxilian 12:11 physicsrob: open source only works when everyone works together; when they do, it works really well
we appreciate you being willing to do some of the legwork and not expecting us to do it all =]
cygmatic 12:11 euh, why does the current dev fbtestplugin fail to load in safari?
taxilian 12:11 it does? hmm. news to me. I haven't tested on Safari recently, though;
cygmatic 12:11 FF is fine
fails with safari 32bit & 64bit
taxilian 12:11 that's not good
what does it do?
physicsrob 12:11 hmm... I just made some progress: commenting out:
if (isWebKit) srcFuncs->pluginthreadasynccall = NULL;
in NpapiTypes.cpp
and no crash
and onload works
and everythin
taxilian 12:11 it will now crash randomly sometimes on Safari :-/
in 64 bit
physicsrob 12:11 gotcha
at least it confirms what we suspected
taxilian 12:11 yes
physicsrob 12:11 your fix works in safari 64-bit though
is there anyway we can test the browser version?
and just use your workaround for 64-bit safari?
taxilian 12:11 oh, right; it's 32 bit safari on 10.6 that sometimes randomly crashes and other times doesn't
if you look at the #ifdef it already does that
physicsrob 12:11 hmm
I'm still at 639, let me go back to the latest
taxilian 12:11 this is actually a problem that Facebook told me about and I didn't believe 'em 'cause I hadn't seen it =] then I ran into it myself
cygmatic 12:11 taxilian: onload event fails
and it crashes :/
taxilian 12:11 cygmatic: huh; well, that'll be related to this same issue
cygmatic 12:11 with that timer stuff that rob reported
taxilian 12:11 weird; it was working
at least it's consistent
cygmatic 12:11 that is strange
taxilian 12:11 this is what happens when you let me write mac code :-P
is there a way to detect if you can use performSelectorOnMainThread or not?
cygmatic 12:11 i'm going to commit the npvariant stuff for now as it works fine in FF
taxilian 12:11 if you can't and it's carbon, you could use the NULL events in a similar way
ok
physicsrob 12:11 I've actually been thinking about moving all of my timers to javascript to avoid some of these headaches (i.e. pluginElement.idle())
because at least on safari -- timers are buggy
taxilian 12:11 yeah; well, if we can come up with a reliable way to do this maybe we can make a reliable method for doing timers
I guess using javascript would be an interesting fallback, though; you could actually call window.setInterval as pass it a c++ function to call :-P
cygmatic 12:11 ok, how about i open a clone and try always using -performSelectorOnMainThread: and we test the hell out of it=
?
taxilian 12:11 sounds like a good plan to me
physicsrob 12:11 I think that would be awesome
cygmatic 12:11 ok, hopefully i'll have it within the hour so you can look at it
taxilian, when you have time please look over the getNpvariant stuff - if you think it's solid we could move it to stable after testing it on windows
taxilian 12:11 ok
cygmatic 12:11 not entirely sure about this: http://code.google.com/p/firebreath/source/browse/src/NpapiPlugin/NPVariantUtil.h?spec=svn.dev.9ce70e0b1ed06b0e3ee68d1eb48bc57d77d7575c&repo=dev&r=9ce70e0b1ed06b0e3ee68d1eb48bc57d77d7575c#236
taxilian 12:11 hmm. one comment; I have removed support for FB::Npapi::NpapiNull in favor of a variant-specific type in dev
cygmatic 12:11 but when going by boost::integer_traits<T>::const_max i ran into some annoying signed/unsigned conversion warnings which would take more time to adress
hm, i just copied the implementation over, it was still there :)
taxilian 12:11 weird...
actually, I thought I'd fixed it in stable as well
ahh
cygmatic 12:11 in stable it was still there - haven't checked after moving that to dev to be honest
taxilian 12:11 only fixed half of it
right; the half that mattered :-P
line 161
and the reason I forgot about it is because I wasn't figuring that anyone would likely be using that to return
I haven't decided how that ought to work, really
mainly if you want to return NULL you probably are returning an object
so I had it just support returning NULL through empty objects
not a problem, then
we should probably throw an exception on an unhandled type
cygmatic 12:11 agreed
taxilian 12:11 this feels like overkill to me, to be honest, but since it's written and it should be pretty much ultimately flexible, let's use it if it works =
=]
we'll eventually likely need something similar for ActiveX, though :-/
cygmatic 12:11 shouldn't be too difficult to replicate
taxilian 12:11 cool
well, focus on Rob's problem first, and then we can look at ax stuff later
cygmatic 12:11 yeah, if the "hg clone" will finish soon ...
taxilian 12:11 soon is such a relative term... =]
kylehuff 12:11 taxilian: yes, it was I who was discussing the SVR record with you..
or more
taxilian 12:11 kylehuff: I was going to have you test it, but I found a way to do so myself
it isn't working
I don't know why
I have the record added with namecheap (my DNS provider), but it doesn't work
kylehuff 12:11 it appears to work when I test it
taxilian 12:11 it really annoys me, to be honest
really? well, maybe something I changed while I was trying to make it work actually did work
kylehuff 12:11 firebreath.org. 1800 IN TXT "v=spf1 include:_spf.google.com ~all"
taxilian 12:11 oh, good
huh; all I did was move it around a bunch
weird
kylehuff 12:11 I tested using a utility called dig; `dig TXT firebreath.org`
taxilian 12:11 yeah, that's what I was using yesterday
among other thigns
kylehuff 12:11 huh..
well, it at least is returning a record now
taxilian 12:11 yeah
now I just have to not touch it :-P
kylehuff 12:11 I can't speak for the syntax, but it is there!
cygmatic 12:11 taxilian: can you remind me where the async implementation is?
ah, NpapiPluginModule::scheduleAsyncCallback()
taxilian 12:11 cygmatic: more specifically, it is in NpapiPluginModule_NPP
the one we're looking at right now
(sorry, I had to go put my son down for a nap)(
physicsrob 12:11 taxilian: NpapiTypes.cpp:120 -- seems to force all webkit browsers to use the async hack
right?
taxilian 12:11 but it could potentially be done in either place; it's just that the one in NpapiPluginModule_NPP.cpp is a direct replacement for the NPN_PluginThreadAsyncCall and provides a queue and everthing
right
because they aren't reliable otherwise
physicsrob 12:11 but it's reliable in 32-bit safari, isn't it?
taxilian 12:11 I wish
but no
it just crashes less frequently
physicsrob 12:11 ah
I see
taxilian 12:11 it is *always* needed on 64 bit; on 32 bit it usually works, but sometimes randomly doesn't
physicsrob 12:11 I'm having to quiet expletives directed at apple
haha
taxilian 12:11 join the club
physicsrob 12:11 okay.. so using safari's pluginthreadasynccall is out of the question
and using safari's scheduletimer is out of the question
what does that leave us with...
taxilian 12:11 aparently so
physicsrob 12:11 hmm
taxilian 12:11 I thought that scheduletimer would be reliable
that leaves us with hacks; there are options on all platforms, it's just a question of which ones we have to use
in cocoa we can use the one mentioned above; in carbon if need be we can use NULL events
physicsrob 12:11 right
is it possible that we're supposed to unschedule the timer used in scheduleAsyncCallback?
taxilian 13:11 https://wiki.mozilla.org/NPAPI:CocoaEventModel#Null_events
the second parameter is whether or not it should repeat (keeping in mind that the npp is supplied by the host)
physicsrob 13:11 woohoo -- I may have a hack
taxilian 13:11 oh, yeah?
physicsrob 13:11 I'm setting the timer for 100ms repeating, and then unscheduling the timer on the first fire
and it appears to be stable so far
taxilian 13:11 100ms is *way* too short
too long, I mean
just FYI =]
interesting result, though
physicsrob 13:11 hehe
taxilian 13:11 try setting it to 1 instead of 0
and/or use the unschedule trick
physicsrob 13:11 100ms single fire crashes
1ms repeating and unscheduling works
wooohooo
taxilian 13:11 huh; try 0ms repeating and unscheduling
cygmatic: you getting this?
physicsrob 13:11 a 0ms timer seems bad... infinite frequency. but it's probably fine since we're only letting it fire once.. I'll try
taxilian 13:11 that's the theory
physicsrob 13:11 safari has no problem with it
this actually makes sense
it's hard for me to believe that they have a completely broken scheduletimer implementation
taxilian 13:11 well, except when you consider that their API is obviously broken
just a relatively untested one
physicsrob 13:11 but it is believable that they didn't have test coverage for a single fire
taxilian 13:11 right
well cool
now we just need to update that check so instead of always disabling the default for webkit, it only does so if ScheduleTimer isn't set
isn't available
physicsrob 13:11 ?
I'm not following
why do we need to change the check?
can't we just use this modification of scheduleAsyncCallback
taxilian 13:11 because the check makes it so that anytime you're running webkit it tries to use the hacked one
and there are some versions of safari (older ones now) that don't support ScheduleTimer
those versions, incidently, seemed to do fine with the NPN_PluginThreadAsyncCall
physicsrob 13:11 gotcha
how old are we talking about?
taxilian 13:11 I think some versions of Safari 4, but I could be wrong
maybe 3
rather probably 3
physicsrob 13:11 would you like me to commit this two liner to stable, or dev?
taxilian 13:11 it's a bugfix; let's put it in stable
we're getting close to where a 1.3.1 release would make sense
physicsrob 13:11 okay... I'll do some more testing and check it in later today
taxilian 13:11 could you update the check for when to use it as well?
physicsrob 13:11 so
cygmatic 13:11 do you have something working?
taxilian 13:11 something like if (isWebKit && srcFuncs->version >= NPVERS_MACOSX_HAS_COCOA_EVENTS && srcFuncs->scheduletimer) srcFuncs->pluginthreadasynccall = NULL
physicsrob 13:11 okay
sounds good
cygmatic, yeah -- it turns out Safari doesn't like single fire timers. But if you schedule it repeating and then cancel it everything is fine
taxilian 13:11 at least so far
cygmatic 13:11 very... euh... nice
physicsrob 13:11 haha
taxilian 13:11 hehe. if you can come up with something cleaner, we're open to the idea =]
physicsrob 13:11 had to hold back the vommit for a second there, didn't ya? :)
taxilian 13:11 however, these are exactly the reasons that FireBreath is such an imporant project... can you imagine dealing with all of these issues by yourself?
physicsrob 13:11 seriously
cygmatic 13:11 ok, someone commited weird settings for FBTestPlugins PluginConfig.cmake
probably me :(
physicsrob, so your fix works across the relevant osx/safari versions?
physicsrob 13:11 well the bug was only seen on Safari in 10.5
not sure if the fix fixed more than that, or broke anything
I'm going to be testing more this afternoon
do you have any cases your specifically concerned with?
cygmatic 13:11 just curious wether that works on 10.6 too :)
physicsrob 13:11 me too
cygmatic 13:11 and if it works i'll rather be fixing the windows build and then stop working ... can't really concentrate anymore ^^
physicsrob 13:11 man I wish I didn't have to support so many platform/browser combos
cygmatic 13:11 i know, multiple os versions can be bad enough already :|
taxilian 14:11 cygmatic: there is no reason to expect that this wouldn't work on 10.6 if the other worked
I think the question would be whether or not this also fixes the 10.6 issues we've occasionally seen that seemed to be timer-related
cygmatic 15:11 well, i've gotten very distrustful of such assumptions ;)
taxilian 15:11 very wise =]
cygmatic 15:11 do you know of a broken test-page for ie?
i mean fbtestplugin?
taxilian 15:11 a broken test page?
cygmatic 15:11 yep, something breaks for me there
haven't yet investigated, just asking before i figure out something you already know
taxilian 15:11 let me try it real quick
it was working last I tried
is this on 64 bit xp?
cygmatic 15:11 yep
but everything worked there with previous dev revisions, so...
taxilian 15:11 RE: issue 100, you're right; if I change that back it breaks the whitespace thing again
cygmatic 15:11 i have to admit that batch confuses me greatly
taxilian 15:11 yeah, it's really strange
I'm not sure what to do about it, though
if (as it seems) it only affects XP64 Pro, I'm not all that worried about it
cygmatic 15:11 pfft
taxilian 15:11 sorry =]
I'm doing a build of the fbtestplugin on dev to check
it'll be a few minutes
cygmatic 15:11 argh, i was wondering why i can't reproduce the build error on hudson... then i realized i hadn't pushed the fix
time to stop working ^^
taxilian 15:11 lol
and the build is fixed!
that server is nice to have up
I'm glad I took the time
cygmatic 15:11 dito, definitely helpful
cygmatic 16:11 taxilian
oops
wanted to say, that IE problem... it's coming from an async call
taxilian 16:11 hmm. wonder if it's related to the message window
cygmatic 16:11 the invoke on the callback (i.e. call default method fails) with some script error HRESULT
taxilian 16:11 hmm
cygmatic 16:11 and then it throws an exception which isn't catched anywhere
the async functors don't have try/catch blocks
taxilian 16:11 hmm. they should
wonder if that's related to this other issue
cygmatic 16:11 which one?
taxilian 16:11 the one where javascript exceptions in a callback cause a plugin crash
cygmatic 16:11 additionally there seems to be some timing issue with it (probably the onload event)
taxilian 16:11 hmm
cygmatic 16:11 blocking the plugin load with a message box seems to make it work if i wait long enough
re other issue: sounds quite likely
taxilian 17:11 strange
cygmatic 17:11 shall i at least give the functors try/catch blocks?
or are you looking into it?
taxilian 17:11 I won't be able to until Monday
so feel free
cygmatic 17:11 also, shall i fix the conversion issue with minimal changes on stable and leave the refactoring for 1.4?