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?
|