OSD700 – First release!

For the last two weeks I have been working on various Firefox bugs for my first release in OSD700.  My bugs have been centered around various parts of the media implementation, ranging from issues with the controls to problems with how preload states are handled.  As it stands, I’ve  managed to get a patch up for 3 of my tickets which is pretty cool if I do say so myself. The tickets that I’ve worked on so far are 702161, 708814, and 680321.  I’m going to touch briefly on each ticket and the changes that I made.

The first ticket I started on for the course was 702161 and was focused around removing anonymous functions that were set up for event listeners but were never removed because we had no reference to them.  Fixing these was a matter of storing a reference to each of these functions and removing them at the proper time.  You can read up on my earlier work for this ticket in my other blog posts. My first patch for this ticket was rejected because I attached each function to the video object instead of attaching an object to the video and attaching the event listeners to it. I forget my reasoning for not doing this in the first place, but thats beside point. I learned a lot about Firefox’s review process with this bug and how it differs from the review process in other open source projects I work on ( Popcorn.js and Popcorn Maker ).  In Popcorn projects we typically wait until both a fix and tests have been written before review is requested.  The review process in Firefox is a bit different.  It seems to the norm to submit patches more frequently in order to get early feedback on what you’ve been working on.  I like this because it allows you to find any glaring mistakes you are making early and ensures that you are on the right track for later patches.  It also allows reviewers to handle your code in smaller doses.  I remember one ticket that I submitted in Popcorn that touched ever file of every plugin ( ~100 files ).  I remember dumping that ticket on Chris De Cairos for review and seeing the look on his face.  I can just imagine how much more manageable my patch would have been if we did something similar to Firefox.  All in all this ticket taught me a lot about Firefox’s review process, how patches should look, and was a good way to get my foot in the door of Firefox development.

 

My second ticket was 708814 and had to do with fixing an issue where mousing over a full-screened video would not hide the controls.  It was cool that this ticket was centered around code that I just finished working with in 7021661, so I felt pretty confident about getting started.  The first thing I did was to make sure that I could reproduce the bug.  Chris Pearce posted a test case and some guidelines about how to reproduce the bug.  I fired up the test and followed his instructions and what do you know there was the bug.  The problem was that when using a hotkey to trigger a video to go full-screen that the controls would not disappear.  This was because prior to my patch, a timeOut for removing the controls was initially fired on mouse move when the video was fullscreen.  My fix consisted of checking to see if the mouse was over the video element when it was fullscreen.  If it was, we would start the timeOut then.  The interesting part about this ticket was that my fix spawned another ticket for me to work on.  The bug that I found ( that already existed in bugzilla )was 708553.

Bug 708553 is about the video element not properly handling hover states when in full-screen mode.  This relates to 708814 because my fix essentially says that if the video is full-screen and if the mouse is hovered over the video, then start the timeOut.  I picked up this bug because from what I can tell, it is blocking my fix for 708814, so I figured why not speed up the process and fix it myself.

My final bug for this release is 680321.  This bug is centered around the preload attribute on video elements and how it handles being changed in the resource selection algorithm.  The initial issue with this ticket was that the resource selection algorithm would ignore preload states that buffered less agressively if changed programatically.  This means that if we start video A with preload auto and set a timeOut for one second later to switch preload to auto and change the videos source, that the new preload state is now ignored.  This means that video B will begin loading assuming preload auto when it should use preload none.  This ticket was interesting because it was my first ticket involving C++.  Now I’m not gonna lie my C++ skills are pretty lacking, and I had to do a bit of refreshing to ever begin to understand what was going on in this code.  The file that I was working in, nsHTMLMediaElement.cpp, was also pretty huge ( ~4k lines ).  I spent pretty much the entirety of a whole night just reading this file and trying to understand how all of the pieces fit together. After finding an area of the code that I thought the bug was in, I began hacking.  After a few incremental builds and I found that I was pretty lost without a debugger.  I remember Chris and Jon talking about a good C++ debugger for the command line so I went to irc to ask for some help. I was given a quick rundown about GDB and how to use it, tho I found a way to screw it up so I opted out and chose to wait until David Humphrey explained it in class.  I was going to have to go with the ole trial and error approach if I wanted to make progress before Thursdays class.  The odd thing that I noticed was the no matter how many changes I made to the file I was not seeing and changes actually happen to the browser ( I even commented out huge blocks of seemingly important code, and nothing happened ) so I figured my build process was screwed up somewhere. My guess was that I was attempting to build too far into the tree and my changes were not actually being seen further up the tree where they needed to.  I said screw it and waited the few minutes and did a fresh new build and I finally saw my changes!  I was lucky in the fact that I was pretty close to a solution on my first try.  It took a few more small tweaks to make the change elegant and following the rest of the codes style and I was ready to submit for some feedback.  The fix involved removing a chunk of code that prevented the functionality that we now wanted and added in another conditional check for our preload state.

My experience so far hacking on Firefox has been awesome. I’ve had a chance to talk with some cool dev’s on irc, make changes to Firefox that will help Popcorn.js, and learn a ton along the way.  As it stands two of my tickets still need tests to be written for them before they get officially reviewed, but I’m optimistic about finishing them and can hopefully get that done by mid next week.  I managed to get 3 tickets somewhat finished so far and hope that I can continue this pace until the end of the semester.  I know the tickets are only going to get harder the deeper I go into Firefox but that also means the satisfaction I get out of finishing them will be even greater. I have no idea what other crazy bugs I will get to work on this semester but I can’t wait.