It Pays to Try

On Sunday afternoon and into the evening, I learned a very important lesson. I recently uploaded a patch that was supposed to fix the media type user preferences. These prefs allow users to toggle on and off different media types such as OGG, Wave Webm and Raw. I narrowed the issue down to each media decoders' Clone method. I patched it up with a little something like this:

This code sample was lost because wordpress

in each of the decoders. I built nightly and tested the prefs, everything looked great.

One thing I did not anticipate, neither did my reviewer, was the Raw media type. I'm not entirely sure as to what it is exactly, but it is disabled by default for Desktop Firefox builds. It is, however, enabled when building Mobile Firefox. When My patch was landed on Mozilla-Inbound, It killed the mobile builds.

The failing build was being caused by my call to nsHTMLMediaElement::IsRawEnabled() in the Raw Decoder. Unlike the other "is[Type]Enabled()" methods, the Raw version was not a static member of the class, and thus, would fail when building for that platform. I quickly made the method virtual, and threw up the fix. It was promptly merged into mozilla-inbound, and right off the bat I saw it failing once again on Android.

Turns out IsRawEnabled iscalled within the static IsRawType function. It was failing because it needed to know that IsRawEnabled was now a member of nsHTMLMediaElement. A silly thing to miss, and because of it we had to once again back out the patch. I fixed it up, and threw up the new patch.In a change of pace, Chris Pearce pushed it to the try servers for testing, and everything looked good!

Lesson learned? Even small patches should be tested, cause you never ever know what you could break!

And on Android, I'll leave you with the words of philor:

[19:16] <philor> nobody expects the Android bustage: its three main weapons are fear, surprise, unexpected bustage, and a fanatical devotion to the Pope