Archive for April, 2012

OSD 1.0

Posted in open source, Uncategorized on April 22, 2012 by scottdowne

This release is primarily a fix for my test on 677122. It has been pretty rough trying to get this test to be stable. It has tendencies to intermittently go into what looks like it’s streaming.

My newest patch, attempts to solve this by waiting for the current event to be finished, before firing the next test by calling SimpleTest.executeSoon(next); instead of just next(); This puts a new event on the stack, that fires the intended function. The next event on the stack will not fire until the current one returns. This way, we can be sure things are done in a sane, debuggable manner. Even this doesn’t fix the problem, which it didn’t, it is still imo good to do.

With the way my test is written, the frequency of this streaming mode is about one out of ten. After much trial and error I was able to reduce the test to failing every time, and filed that 747584.

The reason streaming mode is such a problem is this. I am testing that a video document triggers a time change on a media fragment hash change. I have covered this pretty extensively up to now, and my fix is solid. So to test it, I setup an iframe, a load event on the iframe, get the video resource on there, and listen for a seek, pause, end etc. when the video loads, it seeks to the media fragment time, right at load. But, if we’re streaming, at load, the duration is pretty much at the beginning, and streams only have a duration up to where they have played, so a seek beyond the current duration is not possible.

This only happens inside a mochitest, and only if the iframe src is remote. Something to do with the http server mochitests use, or so I thought. I to notice streaming also comes from js errors. Again, mochitests only. Also, because it would sometimes stream without a js error, and always with a js error. I thought maybe there was a connection, so I looked over my code, trying to clean it. While I did this, I got some advice to also move the iframe into the html, and attach the load function to the iframe’s onload event. I would never of thought of this… I am so used to writing JavaScript with addEventListener, I overlooked adding it to the iframe.

Adding an onload event to the iframe, living in the html, seems to be the trick to get this in the zone. I have running it through the try server, trying to get it to fail. At this point, I am pretty skeptic that it’s fixed. I have done 3+ try pushes, so far so good, but I don’t want to rush. This is such a flaky, intermittent fail I want to be sure.

I also had another fail in my patch’s test. This one only happened once, but I believe I also reduced it to a test case that can happen all the time. I filed it as well.

This bug has taken me through all sorts of interesting code. Starting from the docshell, to the video/mediaDocument, through the mediaResource and iframe loading, and finally ending in the mochitest and related http server. I feel quite comfortable in the video/mediaDocument code. The rest is so so. I am also over bugzilla anxiety. Filing bugs there is quite easy now, and I don’t care so much if I make a mistake and look foolish, as I know people don’t judge as much as people think. It is always worse in your own head. Everyone has been here, and everyone is just trying to help. It can be overwhelming at first, and just takes time to get comfortable with it. It’s not just a learning curve, but also an psychological hurdle.

My experiences in this class have been much different than in the previous OSD. Both classes I was jumping into something new, but I felt the pjs parser clicked with me instantly. My sort of thing, this I have found to be a real challenge. Sometimes I was wishing I took and easy bug, or decided to stick with more pjs bugs, but then I would of learned nearly as much. I might not be an expert, but I have definitely opened up the Firefox code. I have the ability to change it, test it, run it, and make a patch. This is a valuable skill for anyone, in any field. Browsers are so powerful and important now a days, that if you can say you can make it do something else, I don’t know anyone that wouldn’t have at least one idea of what to add or change to it. That’s power. The automobile of my generation.


Reflecting on Firefox bug 677122

Posted in open source on April 10, 2012 by scottdowne

My patch for 677122 is in what I suspect is the final stage, about to land.

This is a bit of a summary of this bug’s history. Some personal reflecting for myself, to step back and look at it in a new way.

For starters I was shocked to see that I had 9 patches, 8 of which are obsolete. By looking at the patches you can see the evolution of the bug. I actually feel really good about it, looking at it like this.

First patch:

I attempted a short circuit load only if the media fragment hash tag does not exist, otherwise we reload the whole page. From my experience, a short circuit load is an attempt to be smart and only load some of the page, that is needed.

This would not of worked because it may break anchor tags, and would be a potential disaster because it potentially touches other cases unrelated to media fragments, like anchors. Also, it reloaded the whole page, which is not ideal.

Second patch:

Understand more of what I was doing, and was able to describe it with “Don’t do a short-circuited load on synthetic documents”.

This is an attempt to make the previous patch better. I would solve the anchor issue, and only touch code that is doing a short circuited load, and did a general cleanup, but it still reloaded the whole page.

Third patch:

Scrapped my old attempt, and went with inserting a script tag in the mediaDocument head, that would listen to hash changes.

The problem with this was I went about it wrong. When I listened for hashchange, I would check the start and end values in the hash, and update the video accordingly, which worked, but was not taking advantage of the original media fragment parser. I did it myself in JavaScirpt.

Fourth patch

I fixed the above by simply updating the src attribute to the new hash, on a hash change. This would take advantage of the current mediaFragment parser, and any additions or changes to it, and was super modular, or so I thought.

Cept for some review nits, it was almost perfect good.

Fifth patch

This one was just cleaning up the nits. I got burned on NS_ENSURE_TRUE returning true, when attempting to change the return type of the function to void, which was interesting. made sense once it was brought to my attention.

This was also the first to get a review plus. I wanted to get a test in though, to get the full knowledge. I am here to learn, after all.

Sixth patch

This one was adding a mochitest.

Seventh patch

Ha, this is just fixing up my patch header information. Adding an email and updating the commit message.

Eighth patch

This is probably the one where I learned the most about the process. I started a fire, got try server access, and put out the fire.

The fire was caused because I was doing this on mediaDocument.cpp, which is misleading, and not just audio and video, but also images and add-ons. videoDocument.cpp is actually video and audio, which is what I wanted, with video just being an extension to audio.

I moved my script injection off mediaDocument and onto videoDocument. Solved the fire.

Ninth patch

I updated my test, because I was getting random orange failures from try server from my own test. Now passes or fails reliably.

OSD 700 0.9

Posted in open source on April 9, 2012 by scottdowne

Almost finished!

This release is another patch for bug 677122, and a patch for pdf.js that uses fontready instead of firing a load event inside an iframe.

I thought I was finished 677122, but it turns out I was failing tests, so I had to go back.

I was inserting my hashchange script onto the head of the mediaDocument, but it turns out inside the c++ code a mediaDocument is images, addons, audio and video documents, and not just audio and video. Image documents do things in a slightly different order, so at the time I insert into the head, image documents don’t yet have a head. Crash. There were two ways to fix this. Either check for head, to be safe, or move this code off mediaDocument and on to videoDocument. It turns out videoDocument is also for audio, so that ended up being perfect!

I also noticed my own test would fail at times. It would only run one test, and fail it. Looked like ended was being fired with non of the other tests being hit. I added the ended test for this reason, as to get a fail, and not a timeout. I ran the test again on my machine, pass. So I decided to run it again, refresh, fail. It would fail consistently on every consecutive refresh. It would go into what looked like streaming mode, and not fire any of my events I listen for, until it ends, then it would fail. Looks like a bug to me, still not sure what yet. I cannot reproduce it outside of mochitests, so if it is a bug, it is not so simple. I thought maybe the refresh wasn’t doing a complete refresh, so I decided to make the iframe myself, instead of leaving it in the markup. So, I create my iframe with createElement and insert it. It passes, refresh, passes. Weird. Push it back to try, no more failures on my test.

The results of my final try have 6 orange failures. I am getting the impression that the ones left are OK, but we’ll see. Boris has been really fast with reviews up to this point, so I hope it won’t be an issue, as I want it landing for my 1.0.

My results from the pdf.js hacking are basically rewiring a load event on an iframe, sending a message to the parent via postmessage, that the fonts are ready.

Apparently @font-face triggers a load event when it is done, if it was defined statically, if it is defined later after the load event, obviously, won’t receive another load event. A way around this is to setup an iframe with the dynamic @font-face statically injected into a new iframe, which passes the load out into the parent.

So I just cut out all the iframe business, and replaced it with my fontready event. This will be fired for each and every consecutive @font-face load.

There is still the need to have to create an element that uses that @font-face before it’ll start the loading, so I had to leave the code to do that for now.

This is 2/3 things that I intended for this release. The other being passing an object into the fontready event, which, was more of a task than I guessed. Not something that can be done in a few days. I have a start, but that must wait for now.

Next release I’ll land 677122, remove the need for a @font-face to be used before it starts the load, and hopefully an update on a fontready object. I also intend to do a post on what I have learned about the quirks of @font-face, what should change, and why, and a post on everything I did this semester.

fontready event and pdf.js experiment

Posted in open source on April 5, 2012 by scottdowne

This is another blog post that I intend to write along side writing code, as opposed to doing it afterwards. This is my second time doing this, and I feel it has more of a narrative or story, than pure information. You get to read thoughts as they unfold, and something before I know what to do with them. I don’t feel this is better or worse than other blog posts, just different and I enjoy it from time to time.

The task at hand is to modify pdf.js to use my fontready patch. I am excited to see if I can decrease load time, but we’ll see. Another goal would be to reduce pdf.js’ code base. Also, this might be a good way for people to learn how to jump into a new project.

First thing: get their source. Easy enough. I went to their github pages page and found a simple git clone command, which I ran.

I explored their pdf.jd directory, and found it is quite similar to our own popcorn-maker directory. I opened a started reading, which basically told me put it on a webserver and run make to compile all the source into one pdf.js file. So, I moved it to my webserver directory, and opened the src directory.

For testing my changes, I am going to go here: http://localhost/pdf.js/web/viewer.html

It was at this point I decided to start this blog. So I next did a bit of time traveling.

I am going to be looking for anything to do with fonts. The fact that all the parts of the source are split might be helpful, if there is something clearly marked font.js.

Instead of finding a font.js, I found a fonts.js 😛 Perfect.

Now I think, it is probably best to do this in a branch.

>git branch fontready master
>git checkout fontready

Good to go.

Now to explore fonts.js. very first line of code is “var kMaxWaitForFontFace = 1000;” which looks extremely interesting to me. But, after searching for all instances of “kMaxWaitForFontFace” I was surprised to see it was not in this file. Odd. I scrolled down some more, and once I got past all the declarations, I came to a FontLoader object, that has an attribute called listeningForFontLoad initially set to false, and two functions, bind(fonts, callback) and prepareFontLoadEvent(rules, names, fonts). First thing I see in prepareFontLoadEvent is /** Hack begin */ and another comment explaining the nature of the hack, how no fontready event exists, and how “This code will be obsoleted by Mozilla bug 471915“. Happens to be why I am here. Safe to say I am in the right spot. This code is reading quite well.

prepareFontLoadEvent is primarily doing two things. It is creating an invisible div to use the new @font-face, this will trigger the download of the @font-face. The second piece is an iframe with all the css @font-faces attached to it, with an onload event, that uses post message to inform the parent the font is ready. Interesting, I was not aware onload waited until a statically defined @font-face is loaded. This is both good and bad. It can solve the need to wait for @font-face, but i can see it holding back a lot of other good stuff in the event that doesn’t need to be held back. It does not allow for dynamically inserted @font-face’s, and still requires the font to be used before the @font-face triggers loading. This may potentially stall a onload forever, if the page is only using the font inside canvas, and the developer is not aware of this quirk. It is quite valuable for me to know how others load this, I am very happy about this find. Anyway, the path of least resistance here is to slap in my fontready listener and just make that work. Go from there.

I first tried to simply replace the onload event in the iframe to my fontready, and refreshed http://localhost/pdf.js/web/viewer.html. As expected, nothing happened. I am not in my fontready build.

Turns out I had to re build, from clean, so it took a bit. Anyway, it ended up working, sort of. I also removed all the iframe stuff, and just rewired the post message listener to be a fontready instead, and removed some code. I have a patch for anyone that is curious. Remember, this is just an experimental demo, not something that can become real.

I most definitely removed code. Hard to say if it is faster, as I did my testing in a debug build.

I had to leave in the invisible div to initiate the loading of the @font-face. I think this is the next thing I am going to try my hand at removing. That, or pass in useful information through the fontready event, but not sure what that should look like yet.