My Resolved Mozilla Bugs
Before starting my masters, I worked on a couple of Mozilla/Gecko items on Bugzilla. Here is a list of all tasks I tackled.
Implementing canvas putImageData’s optional arguments
Bug 498826 is about the HTML canvas putImageData method. It did not implement the optional arguments specified in the WHATWG spec. These optional arguments specify the dirty rectangle for the image data transfer (specifically, these arguments are the coordinates of the top left corner and the dimensions of the dirty rectangle, any of which are allowed to be negative). A quick glance at the description of the algorithm for handling of the optional arguments may not reveal the overall intent of the algorithm. Some of its key aspects are:
- Adjusting the dimensions of the rectangle to be positive by shifting the top left corner if necessary (step 2).
- Ensuring the top left corner of the dirty rectangle is in the first quadrant (step 3) which effectively eliminates all negative arguments.
- Clipping the dirty rectangle to ensure its lower right corner does not extend beyond the bounds of the incoming imagedata’s dimensions (step 4).
- Verifying that the newly adjusted dirty rectangle has positive dimensions (step 5), and if so, using the region bounded by the dirty rectangle on the incoming imagedata object as the source for the operation.
The patch is rather straightforward (although admittedly, it was not as straightforward to create on my part). If there are enough arguments to specify a dirty rectangle, then the JS_ValueToECMAInt32 function is used to convert the JavaScript values into integers. The CheckedInt32 and the gfxRect classes do most of the heavy lifting in the patch, and then only the dirty rectangle is redrawn.
Remove unused nsCookieService::SetCookieString argument
Bug 520914 is the only Bugzilla item I got to look at this month – since it required the least amount of time :). All that needed to be done was to remove the aPrompt argument currently in nsICookieService’s SetCookieString method. The locations in need of change were easily located with a simple MXR search.
Practical Example of Artificial Intelligence Principles
While digging around in Bugzilla (as is now my usual daily custom), I came across Bug 580468 – JM: Tune Trace JIT Heuristics. It was interesting following the discussion since it was a perfect illustration of the principles being taught in CS470. Therefore, I wrote up this brief summary of the bug discussion to reinforce to myself how practical these issues are: Practical AI.
Make location.host and location.hostname return “” for host-less URIs
In the rather straightforward bug 562433, Firefox’s location.host and location.hostname need to return the empty string for host-less URIs instead of throwing an exception. What ends up wasting my time with such 1-minute fixes is figuring out the right test location and the right command to run the test. At least documenting this should save a few minutes next time:
TEST_PATH=dom/tests/mochitest/bugs/test_bug562433.html make -C /c/mozilla/obj mochitest-plain
See the Mochitest automated testing framework documentation for details.
Questioning Programming Assumptions
One of the more interesting facets of programming is probably the close relationship between a program’s correctness and the set of assumptions about its execution environment and/or data. In the web world, the significance of these assumptions takes on a new height since they determine not just the correctness of the application/browser, but also how secure it is (“hackability?”). I decided to do a little test and what faster way to get em browsers running code than by bombarding em with WM_* messages. Here’s a little code snippet that I wrote up for this test (warning: will blow your computer to smithereens, compile and run at your own risk):
// SendMsg.cpp : Defines the entry point for the console application.
#include "stdafx.h"
#include "windows.h"
#define START_MESSAGE WM_CLOSE
#define END_MESSAGE (START_MESSAGE + 0x0001)
int _tmain(int argc, _TCHAR* argv[])
{
WPARAM wParam = 0;
LPARAM lParam = 0;
HWND hWnd = FindWindow(NULL, TEXT("Mozilla Firefox"));
if (!hWnd) {
MessageBox(NULL, TEXT("Could not find firefox window"), TEXT("Error"), MB_OK);
return -1;
}
int errors = 0;
for (DWORD i = 0; i < 10; i++) {
if (!PostMessage(hWnd, START_MESSAGE, wParam, lParam)) {
wchar_t buffer[4096];
DWORD lasterror = GetLastError();
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, lasterror, 0, buffer, 4096, NULL);
MessageBox(NULL, buffer, TEXT("Error"), MB_OK);
errors++;
}
}
if (errors)
return -1;
return 0;
}
The objective of this code is to send the WM_CLOSE message to a browser a given number of times (10 in this case). It starts out with a call to FindWindow to get a handle to the Firefox window. Since the window title is hard coded, a new blank tab must be active in Firefox. Once we have the handle, all that’s left is to call PostMessage the desired number of times. We use GetLastError and FormatMessage for feedback if things do not proceed as planned. I settled on WM_CLOSE since its effect should be the most noticeable ;). Below are some of the results I found:
Interestingly enough, the concept of getting a window handle based the window title worked well for Firefox, Opera, and IE8. It didn’t work in Chrome, but I didn’t want to invest the time to figure out how to get the handle there. Opera 10.53 suffers from the same problem as Firefox as pictured below (the only difference being that the browser ends up closing when one of the timeouts hits 0 so you’d better be sure you have saved all your work).
So the concept of a “Modal” dialog box seems to depend on having exactly one way to launch the box, which is definitely a questionable assumption as shown. IE8 displayed only 1 close prompt box. Kudos to them. I wonder if this type of bug effectively disappears with multiprocess browsing?
In closing, I should note that I also tried the WM_QUIT message (only 1 of which is necessary). Firefox just died as did just about everything else on my system, including explorer.exe and all the other EXEs unfortunate enough to have been running at the time (I was doing some mass message posting initially and it was only after losing a bunch of data that I decided it was in my best interest to post these messages to a specific window).
For anyone interested in running this program, note that Opera 10.53 will CRASH if you send it WM_QUIT! Crash report explains that “Opera.exe 3374 caused exception C0000005 at address 677DD1F3 (Base: 1250000).” I’ve sent in a crash report so that should hopefully get fixed soon. This shouldn’t affect web content since it can’t pull these PostMessage stunts.
Note that this program is easily modified to automate the sending of a range of messages by changing to loop limits to [START_MESSAGE, END_MESSAGE]. I will hopefully have enough time to build a full blown GUI tool for this.
Not using underscores to start #defines
Bug 492625 was a rather easy fix, but one from which I tried to gleam as much as I could from nonetheless. Some /storage/* header files used defines with leading underscores. I used the following regular expression in the search and replace:
Find: (\s+)(_+)(moz\w.+_h)(_+)
Replace With: \1\3
What was interesting was that MXR suggested that there were 72 changes to be made, but the above regex only caught 63. It was only after doing all the manual labor of combing through the list that I realized that I could have simply done a regex search for lines starting with #define (duh) to determine the source of the discrepancy. It turns out that there were some #defines without the leading underscore but having a trailing one.
It was also interesting trying to find the document defining the C++ standard. I ended up at the JTC1/SC22/WG21 – The C++ Standards Committee page which had the relevant text in the “17.6.3.3.2 Global names” section. Stack overflow was the key pointer as to exactly where the relevant info was:
Certain sets of names and function signatures are always reserved to the implementation:
- Each name that contains a double underscore _ _ or begins with an underscore followed by an uppercase letter (2.12) is reserved to the implementation for any use.
- Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.
Building Firefox with Debug Symbols
Building firefox with debug symbols has been a rather tricky endeavor for me for quite some time. The documentation on how to do this seems to indicate that adding these lines to your mozconfig file should be sufficient:
export MOZ_DEBUG_SYMBOLS=1 ac_add_options --enable-debugger-info-modules=yes
should do the trick. However, I have (for months now) been running into this make error: no rule to make nspr4.pdb needed by export whenever I try to build with debug symbols. Bug 338224 has a pending patch for this issue. In the mean time, the following trick seems to solve the problem for me: force the -Zi compiler option into the compiler flags variable. For the nspr files, the PDB file will be generated as desired. For the other files, the -Zi option will be redundant but this fix gets you up and running ASAP. Here’s the associated patch file:
diff --git a/nsprpub/configure.in b/nsprpub/configure.in
--- a/nsprpub/configure.in
+++ b/nsprpub/configure.in
@@ -2827,18 +2827,18 @@ if test -n "$_SAVE_DEBUG_FLAGS"; then
fi
if test -n "$MOZ_OPTIMIZE"; then
CFLAGS="$CFLAGS $_OPTIMIZE_FLAGS"
CXXFLAGS="$CXXFLAGS $_OPTIMIZE_FLAGS"
fi
if test -n "$MOZ_DEBUG_SYMBOLS"; then
- CFLAGS="$CFLAGS $_DEBUG_FLAGS"
- CXXFLAGS="$CXXFLAGS $_DEBUG_FLAGS"
+ CFLAGS="$CFLAGS $_DEBUG_FLAGS -Zi"
+ CXXFLAGS="$CXXFLAGS $_DEBUG_FLAGS -Zi"
fi
if test -n "$MOZ_OPTIMIZE"; then
OBJDIR_TAG=_OPT
else
OBJDIR_TAG=_DBG
fi
Here is the mozconfig file I used with this patch:
. $topsrcdir/browser/config/mozconfig mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj ac_add_options --disable-xpconnect-idispatch ac_add_options --disable-activex ac_add_options --disable-activex-scripting ac_add_options --disable-accessibility ac_add_options --enable-optimize=no ac_add_options --enable-debug-symbols=yes
Building Firefox With Visual C++ Express 2010
I just installed Visual C++ Express 2010 (the most recent version online) in order to build Firefox. However, MozillaBuild has not yet been officially updated to handle Visual C++ 2010. Only 2008 is currently supported as per the build prerequisites. Thankfully, they mention that the trunk version of MozillaBuild should support VC++ Express 2010. The only thing that wasn’t so obvious here (for someone dashing around trying to get building ASAP) was where on earth to look for this. After some totally unnecessary and mostly unhelpful Google-ing around, I ended up at http://hg.mozilla.org/mozilla-build/rev/ad3bd5686474. The necessary start-msvc10.bat and guess-msvc.bat files did the trick.
Bookmark Service getItemGUID Issues
Bug 484026 from last November was an interesting peek into the Firefox bookmarks code. I looked into Dan Mill’s claim that getItemGUID was creating new GUID annotations for deleted items. I started off by creating an xpcshell test for the bug. Interestingly, the test passed! The problem had apparently been fixed at some point.
My first thought on how to proceed was that perhaps the error was being thrown from the wrong method, hence my erroneous attempt to patch nsNavBookmarks.cpp (probably an instance of “if it isn’t broken…”). That would explain why Marco Bonardo didn’t think the change was such a great idea – i.e. you might as well go changing every other possible caller if you aren’t fixing the source.
Nonetheless, it was interesting to note the use of scopers and how declaring this mozStorageStatementScoper eliminated the need for if (statement) statement->reset() calls at every function exit point! I’m sure the idea goes way back, but whoever figured out that a scoping object’s destructor could handle this is a genius in my humble (C++ wise) opinion.
So, even though I didn’t get some C++ action as I had anticipated, I still picked up a thing or two – such as the coding standard used for the bookmarks service xpcshell tests, and how to use the do_throw and do_check_eq functions.