Archive for the 'Mozilla' Category

Firefox Modularity and WebExtensions

Saturday, September 3rd, 2016

Last week, Andy McKay posted skeptical thoughts about whether Firefox system addons (also known as go-faster addons) should be required to use the WebExtensions API surface. I believe that all Firefox gofaster addons, as well as our Test Pilot experiments and SHIELD studies, should use WebExtensions. The explanation comes in three main areas:

  1. The core limiting factor in our ability to go faster while still shipping high-quality software isn’t the packaging and shipping mechanisms for our code. It’s the modularity and module boundaries of our codebase.
  2. The module structure and API surfaces of the Firefox/gecko codebase is hurting our product quality, hurting our teams and our ability to attract volunteer contributors, and hurting our ability to go faster. But there are some shining counter-examples!
  3. WebExtensions should be the future for module boundaries in the Firefox frontend. It’s worth planning carefully and going slow enough to get good module boundaries first, so that we can go blazingly fast later.
  4. Coda: by sticking to a single model for Firefox addons and builtin system addons, we can focus supporting work more effectively and make life better for both addon authors and Firefox engineers.

A diagram showing the "clustered cost" of Mozilla's module interdependencies from 1998-2000.

There is a long history of academic research about software modularity. I remember the OSCON talk in 2006 when I first heard about Exploring the Structure of Complex Software Designs: An Empirical Study of Open Source and Proprietary Code: this talk and paper has changed the way I think about software structure and open source and how to organize engineering teams including volunteers. (Incidentally, this seems like another way of saying “everything that’s important to me at work”.)

Quoting one of the followup papers that studies the impact of software modularity on change over time:

Specifically, we show that i) tightly-coupled components are “harder to kill,” in that they have a greater likelihood of survival in subsequent versions of a design; ii) tightly-coupled components are “harder to maintain,” in that they experience more surprise changes to their dependency relationships that are not associated with new functionality; and iii) tightly-coupled components are “harder to augment,” in that the mix of new components added in each version is significantly more modular than the legacy design.

—Alan MacCormack, John Rusnak, and Carliss Y. Baldwin. The Impact of Component Modularity on Design Evolution: Evidence from the Software Industry.

The structure and connectedness of modules in a codebase determines what you can change easily (and therefore quickly) and what things have to change slowly. There is not a single design for a software project: you have to design the structure of your modules and interfaces based on what may need to change in the future. Even more fundamentally, the shape and structure of APIs and modules determines where teams can be innovative most successfully. Relatedly, module size and structure determines where contributors can be most effective and feel most welcome.

There are some shining success stories at Mozilla where good modular design has allowed teams to work independently and quickly. The devtools debugging API is a great example of carefully designing an API surface which then allowed the team to move quickly and innovate. The team spent a lot of time refactoring guts within the JS engine to expose a debugging API which wasn’t tightly tied to the underlying platform. And since devtools are isolated from the rest of the browser in an iframe, there is a natural separation which allows them to be developed as a unit and independently. The advantages of this model became quickly apparent when the team started reusing the same basic debugging structure for a desktop Firefox to debug content in Firefox for Android, or debug the Firefox chrome itself, or even debug content in Chrome!

A lot of the pain and slowness in Firefox development relates to our code not having well-defined, documented, or enforced module boundaries or interfaces. Within the Firefox frontend itself, we have many different kinds of functionality that runs in the single technical context of the main browser window. We sometimes pretend that there are relatively clean boundaries between the Firefox frontend and the underlying platform, but this is a myth. There are pervasive implementation dependencies between things like the tab browser in the frontend and the network/docshell/PSM validation in the platform which often have to be modified together.

Lack of clean modules and interfaces manifest themselves in many ways. It is notoriously difficult to write tests for Firefox. This is pretty natural, since good tests often focus on the interface boundaries between modules. It also makes it extremely difficult to “mock” unrelated modules cleanly for a test. The pervasiveness of “random oranges” is not because tests are written poorly: it is most related to tests being very difficult to write well.

Historically problems maintaining and building Firefox addons and the addon ecosystem is directly related to not having an API surface for addons. Because writing an addon usually involved hacking into the structure of the browser.xul DOM and JS, it was natural that addon authors were frequently affected by browser changes. And it was very difficult for anyone, Firefox hacker or addon hacker alike, to know whether or how any particular change would affect addons. The most common complaint from addon authors was that we didn’t have good API documentation; but this is not a problem with the documentation. It’s a problem of not having good APIs. Similarly, a major user complaint was that addons break too often. This is not a problem of addon authors being bad coders; it is more fundamentally a lack of stable API surfaces that would allow anyone to write code that continues working over time.

In order to make addon development fun and productive again, we’ve made big investments in WebExtensions. WebExtensions is a clearly specified, tested, documented, and maintained API surface targeted at addons. The WebExtensions system will allow addon authors to innovate and be creative within a structure that lets Firefox support them over time. Over the next year, we expect basically all Firefox addons to move to WebExtensions. We’ve already seen this energize the addon community and especially promote addon development by authors who gave up developing old-style Firefox addons.

Within Firefox development itself, we need this same combination of structure and innovation. Starting with the new system addons, we need to spend the time up front to build API surfaces that allow teams within Mozilla to experiment and build new systems safely, with the confidence that they aren’t going to break Firefox by accident. We need to have enough foresight to say where we want to experiment, so that we can make sure that we have the right API and module boundaries in place so that experiments can be successful.

If our system addons are still coded in a way that depends on the internal structure of browser.xul, or XPCOM, or the network stack, then they have inherently high testing cost. We need to re-validate them against each release, and be very cautious about pushing changes because we don’t know whether any change could break the browser.

If, on the other hand, we use WebExtensions and have technical guarantees that addons are isolated from other code, we have the ability to push new changes aggressively and actually independently. QA and release teams don’t need to test every change extensively and in all combinations: instead we can use the technical isolation guarantees of the WebExtensions API to allow teams to be more completely independent, the same way we want addon authors to be essentially independent.

As we start developing system addons using WebExtensions, there are going to be APIs which we don’t want to commit to, or that we’re not sure about. I think we will have to come up with APIs that are not exposed to all addons, but only to particular system addons where we understand the risks and experimental nature. We are going to need a way for many teams to be responsible for their own API surfaces, and not route every change through the addons engineering team. We should embrace these challenges as the cost of success.

Having more code use WebExtensions is a way to focus engineering efforts. We know that both addons and system code needs better support for profiling and performance monitoring, telemetry, unit testing, localization, and other engineering support functions. By having our system addons use the same basic API layer as external addons, we can build tools that improve life for everyone. The cost of building each of these things well is large, but through shared effort we can make our engineering investment return much greater dividends.

At the end of the day, I believe we should end up in a world where most of the Firefox frontend is made available via system addons and glued together using well-defined API surfaces. What would it be like to have our tabbed browser, location bar, search interface, bookmarks system, session restore, etc be independently hackable? It would be awesome! It would mean a higher quality product, with more opportunities for volunteer contributors to improve specific areas. It would likely mean reduced edit/build/run/test cycles. It might even give addon authors the ability to completely replace certain components of the browser if they so choose.

I shared a draft of this post with Andy and other tech leads and got some great feedback. In order for discussion to go to one place, please post any replies or thoughts to the firefox-dev mailing list.

Yak Shaving

Wednesday, May 27th, 2015

Yak shaving tends to be looked down on. I don’t necessarily see it that way. It can be a way to pay down technical debt, or learn a new skill. In many ways I consider it a sign of broad engineering skill if somebody is capable of solving a multi-part problem.

It started so innocently. My team has been working on unifying the Firefox Health Report and Telemetry data collection systems, and there was a bug that I thought I could knock off pretty easily: “FHR data migration: org.mozilla.crashes”. Below are the roadblocks, mishaps, and sideshows that resulted, and I’m not even done yet:

Tryserver failure: crashes

Constant crashes only on Linux opt builds. It turned out this was entirely my fault. The following is not a safe access pattern because of c++ temporary lifetimes:

nsCSubstringTuple str = str1 + str2;
Fn(str);
Backout #1: talos xperf failure

After landing, the code was backed out because the xperf Talos test detected main-thread I/O. On desktop, this was a simple ordering problem: we always do that I/O during startup to initialize the crypto system; I just moved it slightly earlier in the startup sequence. Why are we initializing the crypto system? To generate a random number. Fixed this by whitelisting the I/O. This involved landing code to the separate Talos repo and then telling the main Firefox tree to use the new revision. Much thanks to Aaron Klotz for helping me figure out the right steps.

Backout #2: test timeouts

Test timeouts if the first test of a test run uses the PopupNotifications API. This wasn’t caught during initial try runs because it appeared to be a well-known random orange. I was apparently changing the startup sequence just enough to tickle a focus bug in the test harness. It so happened that the particular test which runs first depends on the e10s or non-e10s configuration, leading to some confusion about what was going on. Fortunately, I was able to reproduce this locally. Gavin Sharp and Neil Deakin helped get the test harness in order in bug 1138079.

Local test failures on Linux

I discovered that several xpcshell tests were failing locally on Linux which were working fine on tryserver. After some debugging, I discovered that the tests thought I wasn’t using Linux, because the cargo-culted test for Linux was let isLinux = ("@mozilla.org/gnome-gconf-service;1" in Cc). This means that if gconf is disabled at build time or not present at runtime, the tests will fail. I installed GConf2-devel and rebuilt my tree and things were much better.

Incorrect failure case in the extension manager

While debugging the test failures, I discovered an incorrect codepath in GMPProvider.jsm for clients which are not Windows, Mac, or Linux (Android and the non-Linux Unixes).

Android performance regression

The landing caused an Android startup performance regression, bug 1163049. On Android, we don’t initialize NSS during startup, and the earlier initialization of the addon manager caused us to generate random Sync IDs for addons. I first fixed this by using Math.random() instead of good crypto, but Richard Newman suggested that I just make Sync generation lazy. This appears to work and will land when there is an open tree.

mach bootstrap on Fedora doesn’t work for Android

As part of debugging the performance regression, I built Firefox for Android for the first time in several years. I discovered that mach bootstrap for Android isn’t implemented on Fedora Core. I manually installed packages until it built properly. I have a list of the packages I installed and I’ll file a bug to fix mach bootstrap when I get a chance.

android build-tools not found

A configure check for the android build-tools package failed. I still don’t understand exactly why this happened; it has something to do with a version that’s too new and unexpected. Nick Alexander pointed me at a patch on bug 1162000 which fixed this for me locally, but it’s not the “right” fix and so it’s not checked into the tree.

Debugging on Android (jimdb)

Binary debugging on Android turned out to be difficult. There are some great docs on the wiki, but those docs failed to mention that you have to pass the configure flag –enable-debug-symbols. After that, I discovered that pending breakpoints don’t work by default with Android debugging, and since I was debugging a startup issue that was a critical failure. I wrote an ask.mozilla.org question and got a custom patch which finally made debugging work. I also had to patch the implementation of DumpJSStack() so that it would print to logcat on Android; this is another bug that I’ll file later when I clean up my tree.

Crash reporting broken on Mac

I broke crash report submission on mac for some users. Crash report annotations were being truncated from unicode instead of converted from UTF8. Because JSON.stringify doesn’t produce ASCII, this was breaking crash reporting when we tried to parse the resulting data. This was an API bug that existed prior to the patch, but I should have caught it earlier. Shoutout to Ted Mielczarek for fixing this and adding automated tests!

Semi-related weirdness: improving the startup performance of Pocket

The Firefox Pocket integration caused a significant startup performance issue on some trees. The details are especially gnarly, but it seems that by reordering the initialization of the addon manager, I was able to turn a performance regression into a win by accident. Probably something to do with I/O wait, but it still feels like spooky action at a distance. Kudos to Joel Maher, Jared Wein and Gijs Kruitbosch for diving into this under time pressure.

Experiences like this are frustrating, but as long as it’s possible to keep the final goal in sight, fixing unrelated bugs along the way might be the best thing for everyone involved. It will certainly save context-switches from other experts to help out. And doing the Android build and debugging was a useful learning experience.

Perhaps, though, I’ll go back to my primary job of being a manager.

Hiring at Mozilla: Beyond Resumés and Interview Panels

Monday, May 11th, 2015

The standard tech hiring process is not good at selecting the best candidates, and introduces unconscious bias into the hiring process. The traditional resume screen, phone screen, and interview process is almost a dice-roll for a hiring manager. This year, my team has several open positions and we’re trying something different, both in the pre-interview screening process and in the interview process itself.

Hiring Firefox Platform Engineers now!

Earlier this year I attended a workshop for Mozilla managers by the Clayman Institute at Stanford. One of the key lessons is that when we (humans) don’t have clear criteria for making a choice, we tend alter our criteria to match subconscious preferences (see this article for some examples and more information). Another key lesson is that when humans lack information about a situation, our brain uses its subconscious associations to fill in the gaps.

Candidate Screening

I believe job descriptions are very important: not only do they help candidates decide whether they want a particular job, but they also serve as a guide to the kinds of things that will be required or important during the interview process. Please read the job description carefully before applying to any job!

In order to hire more fairly, I have changed the way I write job descriptions. Previously I mixed up job responsibilities and applicant requirements in one big bulleted list. Certainly every engineer on my team is going to eventually use C++ and JavaScript, and probably Python, and in the future Rust. But it isn’t a requirement that you know all of these coming into a job, especially as a junior engineer. It’s part of the job to work on a high-profile open-source project in a public setting. But that doesn’t mean you must have prior open-source experience. By separating out the job expectations and the applicant requirements, I was able to create a much clearer set of screening rules for incoming applications, and also clearer expectations for candidates.

Resumés are a poor tool for ranking candidates and deciding which candidates are worth the investment in a phone screen or interview. Resumés give facts about education or prior experience, but rarely make it clear whether somebody is an excellent engineer. To combat this, my team won’t be using only resumés as a screening tool. If a candidate matches basic criteria, such as living in a reasonable time zone and having demonstrated expertise in C++, JavaScript, or Python on their resumé or code samples, we will ask each candidate to submit a short written essay (like a blog post) describing their favorite debugging or profiling tool.

Why did I pick an essay about a debugging or profiling tool? In my experience, every good coder has a toolbox, and as coders gain experience they are naturally better toolsmiths. I hope that this essay requirement will be good way to screen for programmer competence and to gauge expertise.

With resumés, essays, and code samples in hand, Vladan and I will go through the applications and filter the applications. Each passing candidate will proceed to phone screens, to check for technical skill but more importantly to sell the candidate on the team and match them up with the best position. My goal is to exclude applications that don’t meet the requirements, not to rank candidates against each other. If there are too many qualified applicants, we will select a random sample for interviews. In order to make this possible, we will be evaluating applications in weekly batches.

Interview Process

To the extent possible, the interview format should line up with the job requirements. The typical Mozilla technical interview is five or six 45-minute 1:1 interview sessions. This format heavily favors people who can think quickly on their feet and who are personable. Since neither of those attributes is a requirement for this job, that format is a poor match. Here are the requirements in the job description that we need to evaluate during the interview:

  • Experience writing code. A college degree is not necessary or sufficient.
  • Expertise in any of C++, JavaScript, or Python.
  • Ability to learn new skills and solve unfamiliar problems effectively.
  • Experience debugging or profiling.
  • Good written and verbal communication skills.
  • Candidates must be located in North or South America, Europe, Africa, or the Middle East.

This is the interview format that we came up with to assess the requirements:

  • A 15-minute prepared presentation on a topic related to the candidate’s prior experience and expertise. This will be done in front of a small group. A 30-minute question and answer session will follow. Assesses experience writing code and verbal communication skills.
  • A two-hour mentoring session with two engineers from the team. The candidate will be working in a language they already know (C++/JS/Python), but will be solving an unfamiliar problem. Assesses experience writing code, language expertise, and ability to solve unfamiliar problems.
  • A 45-minute 1:1 technical interview. This will assess some particular aspect of the candidate’s prior experience with technical questions, especially if that experience is related to optional/desired skills in the job description. Assesses specialist or general expertise and verbal communication.
  • A 45-minute 1:1 interview with the hiring manager. This covers a wide range of topics including work location and hours, expectations about seniority, and to answer questions that the candidate has about Mozilla, the team, or the specific role they are interviewing for. Assesses candidate location and desire to be part of the team.

During the debrief and decision process, I intend to focus as much as possible on the job requirements. Rather than asking a simple “should we hire this person” question, I will ask interviewers to rate the candidate on each job requirement and responsibility, as well as any desired skillset. By orienting the feedback to the job description I hope that we can reduce the effects of unconscious bias and improve the overall hiring process.

Conclusion

This hiring procedure is experimental. My team and I have concerns about whether candidates will be put off by the essay requirement or an unusual interview format, and whether plagiarism will make the essay an ineffective screening tool. We’re concerned about keeping the hiring process moving and not introducing too much delay. After the first interview rounds, I plan on evaluating the process, and ask candidates to provide feedback about their experience.

If you’re interested, check out my prior post, How I Hire At Mozilla.

Using crash-stats-api-magic

Monday, April 20th, 2015

A while back, I wrote the tool crash-stats-api-magic which allows custom processing of results from the crash-stats API. This tool is not user-friendly, but it can be used to answer some pretty complicated questions.

As an example and demonstration, see a bug that Matthew Gregan filed this morning asking for a custom report from crash-stats:

In trying to debug bug 1135562, it’s hard to guess the severity of the problem or look for any type of version/etc. correlation because there are many types of hangs caught under the same mozilla::MediaShutdownManager::Shutdown stack. I’d like a report that contains only those with mozilla::MediaShutdownManager::Shutdown in the hung (main thread) stack *and* has wasapi_stream_init on one of the other threads, please.

To build this report, start with a basic query and then refine it in the tool:

  1. Construct a supersearch query to select the crashes we’re interested in. The only criteria for this query was “signature contains ‘MediaShutdownManager::Shutdown`. When possible, filter on channel, OS, and version to reduce noise.
  2. After the supersearch query is constructed, choose “More Options” from the results page and copy the “Public API URL” link.
  3. Load crash-stats-api-magic and paste the query URL. Choose “Fetch” to fetch the results. Look through the raw data to get a sense for its structure. Link
  4. The meat of this function is to filter out the crashes that don’t have “wasapi_stream_init” on a thread. Choose “New Rule” and create a filter rule:
    function(d) {
      var ok = false;
      d.json_dump.threads.forEach(function(thread) {
        thread.frames.forEach(function(frame) {
          if (frame.function && frame.function.indexOf("wasapi_stream_init") != -1) {
            ok = true;
          }
        });
      });
      return ok;
    }

    Choose “Execute” to run the filter. Link

  5. To get the final report we output only the signature and the crash ID for each result. Choose “New Rule” again and create a mapping rule:
    function(d) {
      return [d.uuid, d.signature];
    }

    Link

One of the advantages of this tool is that it is possible to iterate quickly on the data without constantly re-querying, but at the end it should be possible to permalink to the results in bugzilla or email exchanges.

If you need to do complex crash-stats analysis, please try it out! email me if you have questions, and pull requests are welcome.

Gratitude Comes in Threes

Tuesday, February 17th, 2015

Today Johnathan Nightingale announced his departure from Mozilla. There are three special people at Mozilla who shaped me into the person I am today, and Johnathan Nightingale is one of them:

Mike Shaver taught me how to be an engineer. I was a full-time musician who happened to be pretty good at writing code and volunteering for Mozilla. There were many people at Mozilla who helped teach me the fine points of programming, and techniques for being a good programmer, but it was shaver who taught me the art of software engineering: to focus on simplicity, to keep the ultimate goal always in mind, when to compromise in order to ship, and when to spend the time to make something impossibly great. Shaver was never my manager, but I credit him with a lot of my engineering success. Shaver left Mozilla a while back to do great things at Facebook, and I still miss him.

Mike Beltzner taught me to care about users. Beltzner was never my manager either, but his single-minded and sometimes pugnacious focus on users and the user experience taught me how to care about users and how to engineer products that people might actually want to use. It’s easy for an engineer to get caught up in the most perfect technology and forget why we’re building any of this at all. Or to get caught up trying to change the world, and forget that you can’t change the world without a great product. Beltzner left Mozilla a while back and is now doing great things at Pinterest.

Perhaps it is just today talking, but I will miss Johnathan Nightingale most of all. He taught me many things, but mostly how to be a leader. I have had the privilege of reporting to Johnathan for several years now. He taught me the nuances of leadership and management; how to support and grow my team and still be comfortable applying my own expertise and leadership. He has been a great and empowering leader, both for me personally and for Firefox as a whole. He also taught me how to edit my own writing and others, and especially never to bury the lede. Now Johnathan will also be leaving Mozilla, and undoubtedly doing great things on his next adventure.

It doesn’t seem coincidental that triumverate were all Torontonians. Early Toronto Mozillians, including my three mentors, built a culture of teaching, leading, mentoring, and Mozilla is better because of it. My new boss isn’t in Toronto, so it’s likely that I will be traveling there less. But I still hold a special place in my heart for it and hope that Mozilla Toronto will continue to serve as a model of mentoring and leadership for Mozilla.

Now I’m a senior leader at Mozilla. Now it’s my job to mentor, teach, and empower Mozilla’s leaders. I hope that I can be nearly as good at it as these wonderful Mozillians have been for me.

An Invitation

Sunday, November 30th, 2014

I’d like to invite my blog readers and Mozilla coworkers to Jesus Christ.

For most Christians, today marks the beginning of Advent, the season of preparation before Christmas. Not only is this a time for personal preparation and prayer while remembering the first coming of Christ as a child, but also a time to prepare the entire world for Christ’s second coming. Christians invite their friends, coworkers, and neighbors to experience Christ’s love and saving power.

I began my journey to Christ through music and choirs. Through these I discovered beauty in the teachings of Christ. There is a unique beauty that comes from combining faith and reason: belief in Christ does not require superstition nor ignorance of history or science. Rather, belief in Christ’s teachings brought me to a wholeness of understanding the truth in all it’s forms, and our own place within it.

Although Jesus is known to Christians as priest, prophet, and king, I have a special and personal devotion to Jesus as king of heaven and earth. The feast of Christ the King at the end of the church year is my personal favorite, and it is a particular focus when I perform and composing music for the Church. I discovered this passion during college; every time I tried to plan my own life, I ended up in confusion or failure, while every time I handed my life over to Christ, I ended up being successful. My friends even got me a rubber stamp which said “How to make God laugh: tell him your plans!” This understanding of Jesus as ruler of my life has led to a profound trust in divine providence and personal guidance in my life. It even led to my becoming involved with Mozilla and eventually becoming a Mozilla employee: I was a church organist and switching careers to become a computer programmer was a leap of faith, given my lack of education.

Making a religious invitation to coworkers and friends at Mozilla is difficult. We spend our time and build our deepest relationships in a setting of on email, video, and online chat, where off-topic discussions are typically out of place. I want to share my experience of Christ with those who may be interested, but I don’t want to offend or upset those who aren’t.

This year, however, presents me with a unique opportunity. Most Mozilla employees will be together for a shared planning week. If you will be there, please feel free to find me during our down time and ask me about my experience of Christ. If you aren’t at the work week, but you still want to talk, I will try to make that work as well! Email me.

1. On Jordan’s bank, the Baptist’s cry
Announces that the Lord is nigh;
Awake, and hearken, for he brings
Glad tidings of the King of kings!

2. Then cleansed be every breast from sin;
Make straight the way for God within;
Prepare we in our hearts a home
Where such a mighty Guest may come.

3. For Thou art our Salvation, Lord,
Our Refuge, and our great Reward.
Without Thy grace we waste away,
Like flowers that wither and decay.

4. To heal the sick stretch out Thine hand,
And bid the fallen sinner stand;
Shine forth, and let Thy light restore
Earth’s own true lovliness once more.

5. Stretch forth thine hand, to heal our sore,
And make us rise to fall no more;
Once more upon thy people shine,
And fill the world with love divine.3

6. All praise, eternal Son, to Thee
Whose advent sets Thy people free,
Whom, with the Father, we adore,
And Holy Ghost, forevermore.

—Charles Coffin, Jordanis oras prævia (1736), Translated from Latin to English by John Chandler, 1837

How I Do Code Reviews at Mozilla

Wednesday, October 22nd, 2014

Since I received some good feedback about my prior post, How I Hire at Mozilla, I thought I’d try to continue this is a mini-series about how I do other things at Mozilla. Next up is code review.

Even though I have found new module owners for some of the code I own, I still end up doing 8-12 review/feedback cycles per week. Reviews are only as good as the time you spend on them: I approach reviews in a fairly systematic way.

When I load a patch for review, I don’t read it top-to-bottom. I also try to avoid reading the bug report: a code change should be able to explain itself either directly in the code or in the code commit message which is part of the patch. If bugzilla comments are required to understand a patch, those comments should probably be part of the commit message itself. Instead, I try to understand the patch by unwrapping it from the big picture into the small details:

The Commit Message

  • Does the commit message describe accurately what the patch does?
  • Am I the right person to make a decision about this change?
  • Is this the right change to be making?
  • Are there any external specifications for this change? This could include a UX design, or a DOM/HTML specification, or something else.
  • Should there be an external specification for this change?
  • Are there other experts who should be involved in this change? Does this change require a UX design/review, or a security, privacy, legal, localization, accessibility, or addon-compatibility review or notification?

Read the Specification

If there is an external specification that this change should conform to, I will read it or the appropriate sections of it. In the following steps of the review, I try to relate the changes to the specification.

Documentation

If there is in-tree documentation for a feature, it should be kept up to date by patches. Some changes, such as Firefox data collection, must be documented. I encourage anyone writing Mozilla-specific features and APIs to document them primarily with in-tree docs, and not on developer.mozilla.org. In-tree docs are much more likely to remain correct and be updated over time.

API Review

APIs define the interaction between units of Mozilla code. A well-designed API that strikes the right balance between simplicity and power is a key component of software engineering.

In Mozilla code, APIs can come in many forms: IDL, IPDL, .webidl, C++ headers, XBL bindings, and JS can all contain APIs. Sometimes even C++ files can contain an API; for example Mozilla has an mostly-unfortunate pattern of using the global observer service as an API surface between disconnected code.

In the first pass I try to avoid reviewing the implementation of an API. I’m focused on the API itself and its associated doccomments. The design of the system and the interaction between systems should be clear from the API docs. Error handling should be clear. If it’s not perfectly obvious, the threading, asynchronous behavior, or other state-machine aspects of an API should be carefully documented.

During this phase, it is often necessary to read the surrounding code to understand the system. None of our existing tools are very good at this, so I often have several MXR tabs open while reading a patch. Hopefully future review-board integration will make this better!

Brainstorm Design Issues

In my experience, the design review is the hardest phase of a review, the part which requires the most experience and creativity, and provides the most value.

  • How will this change interact with other code in the tree?
  • Are there edge cases or failure cases that need to be addressed in the design?
  • Is it likely that this change will affect performance or security in unexpected ways?

Testing Review

I try to review the tests before I review the implementation.

  • Are there automated unit tests?
  • Are there automated performance tests?
  • Is there appropriate telemetry/field measurement, especially for error conditions?
  • Do the tests cover the specification, if there is one?
  • Do the tests cover error conditions?
  • Do the tests strike the right balance between “unit” testing of small pieces of code versus “integration” tests that ensure a feature works properly end-to-end?

Code Review

The code review is the least interesting part of the review. At this point I’m going through the patch line by line.

  • Make sure that the code uses standard Mozilla coding style. I really desperately want somebody to automated lots of this as part of the patch-submission process. It’s tedious both for me and for the patch author if there are style issues that delay a patch and require another review cycle.
  • Make sure that the number of comments is appropriate for the code in question. New coders/contributors sometimes have a tendency to over-comment things that are obvious just by reading the code. Experienced contributors sometimes make assumptions that are correct based on experience but should be called out more explicitly in the code.
  • Look for appropriate assertions. Assertions are a great form of documentation and runtime checking.
  • Look for appropriate logging. In Mozilla we tend to under-log, and I’d like to push especially new code toward more aggressive logging.
  • Mostly this is scanning the implementation. If there is complexity (such as threading!), I’ll have to slow down a lot and make sure that each access and state change is properly synchronized.

Re-read the Specification

If there is a specification, I’ll briefly re-read it to make sure that it was covered by the code I just finished reading.

Mechanics

Currently, I primarily do reviews in the bugzilla “edit” interface, with the “edit attachment as comment” option. Splinter is confusing and useless to me, and review-board doesn’t seem to be ready for prime-time.

For long or complex reviews, I will sometimes copy and quote the patch in emacs and paste or attach it to bugzilla when I’m finished.

In some cases I will cut off a review after one of the earlier phases: if I have questions about the general approach, the design, or the API surface, I will often try to clarify those questions before proceeding with the rest of the review.

There’s an interesting thread in mozilla.dev.planning about whether it is discouraging to new contributors to mark “review-” on a patch, and whether there are less-painful ways of indicating that a patch needs work without making them feel discouraged. My current practice is to mark r- in all cases where a patch needs to be revised, but to thank contributors for their effort so that they are still appreciated and to be as specific as possible about required changes while avoiding any words that could be perceived as an insult.

If I haven’t worked with a coder (paid or volunteer) in the past, I will typically always ask them to submit an updated patch with any changes for re-review. This allows me to make sure that the changes were completed properly and didn’t introduce any new problems. After I gain some experience, I will often trust people to make necessary changes and simply mark “r+ with review comments fixed”.

How I Hire at Mozilla

Thursday, October 2nd, 2014

As a manager, one of my most important responsibilities is hiring. While I’m hiring, I spend a lot of time sifting through resumés, screening candidates, and eventually doing interviews. This is both fun and depressing at the same time, because you get to meet and learn a lot about some interesting people, but you also have to wade through a lot of terrible resumés and phone screens. I want to share some things that I look for during the hiring process, and some tips for potential job-seekers about how to deal with recruiting:

Read the Job Description

Please read the job description before applying for a job! I put a lot of effort into writing a job description, and I try very hard to describe both the job responsibilities and the necessary and desirable skills. Your cover letter should show some evidence that you’ve thought about applying for this particular job.

Write a Good Cover Letter

Periodically, I see articles advising job-seekers to ditch the cover letter. This is terrible advice. I read cover letters carefully. In fact, the cover letter sometimes gives me a better sense for your skill level and ability to do the job than your resumé.

Grammar and Spelling Matter

Every job I’ve ever posted has required good communication skills, and your cover letter is the first evidence of your communication skills. It doesn’t need to be more than a paragraph or two; I’d love to know why you think that this job is a good fit, and some evidence that you read the job description. Spelling and grammar are important.

I’m Picky

The last time I posted a job position, I screened more than 800 resumés, did almost 100 phone screens, and did five interviews before I found the right person. It took six months. It’s better for Mozilla if I’m too picky and reject a qualified candidate than if I’m not picky enough and accept a bad candidate. Please don’t take rejection as a comment on your personal worth. I’m going to reject many people during this process.

Smart and Gets Things Done

Joel Spolsky is right: I’m primarily looking for somebody who is smart and gets things done. When I’m scanning through resumés, I’m looking for evidence that you’ve gotten things done in the past. If you’re just coming out of school, internship and open-source project experience helps. If you’ve been working, I want to see some description of the things you’ve gotten done. Be specific: “Led multiple projects to completion” is meaningless. You can expect phone-screen questions about your prior projects. Be prepared to talk about what worked, what didn’t, and how you solved problems.

No Assholes

It may seem obvious, but assholes need not apply. I will reject a technically-qualified candidate for even a whiff of assholery. I use reference checks to try and guard against assholes.

Passion Isn’t Sufficient

At Mozilla we get a lot of applicants who are passionate, either about the open web in general or about Firefox. Passion is great, perhaps even necessary, but it’s not sufficient.

Interview Questions Are Based on Your Resumé

In phone screens and interview panels, I try very hard to base my questions on the things that you already know. If your resumé says that you are a master of C++, you should expect that there is at least one person on the interview panel who will grill you on C++ in excruciating detail. If you say that you know statistics, you had better be able to answer detailed questions about statistics.

Don’t overstate your skills. If you have written a couple scripts in Python, you are familiar with Python, not proficient. More than once I’ve rejected people because they claimed to be a master of C++ debugging but didn’t know how a vtable is structured in memory. Knowing vtable layout isn’t usually a job prerequisite, but if you are a master of C++ debugging you’d better know how that works in detail.

If you claim to be a master of both Python and JavaScript, expect me to ask you detailed questions about how Python and JS closures and methods work, how they are different in JS and python, how JS this-binding is different from Python bound methods, and other details of the language. I will be impressed if you can discuss these questions intelligently, and reject your application if you can’t.

I Value Code Reading

I value people who can learn new code quickly. You’re going to need to be comfortable learning new systems, libraries, and languages. My team in particular often touches large swaths of the Mozilla codebase; you will be expected to be able to read and understand new code quickly. You can expect an interview session entirely dedicated to reading and explaining a piece of code that you’ve never seen before. Perhaps it will be reviewing a patch, or trying to find a bug in a piece of code. I’ll try to find a problem in a language that you are already comfortable with, so see above about keeping your resumé honest!

Do You Love Your Tools?

Every good programmer has a toolbox. I don’t care whether you use vim or emacs or Visual Studio or Eclipse or XCode, but I do care that you have an editor and use it productively. I also care that you are proficient using a debugger (or two, or three) and hopefully a profiler (or two, or three). If you can’t tell me why you love your editor or your debugger, it’s likely that you won’t be a successful software engineer.

You need to be proficient in a scripting language. I expect you to be able to use at least one scripting language to process text data, read CSV files, write JSON, and that kind of thing. Mozilla has gravitated toward Python for scripting, and you’ll probably be expected to learn Python if you don’t know it already.

Also, can you type? I’m constantly surprised by applicants who are unable to type quickly and accurately. A significant portion of your job is going to be writing code, and not having mastered the act of typing is probably not a good sign.

Phone Screens

When I conduct a phone-screen, it is usually over Skype video. Please make sure that you have a decent headset and that I will be able to hear you. During a phone screen, I try to include at least one coding question which is typically conducted via etherpad. You should be at a computer with access to the web in order to do this successfully.

By the way, did I mention I’m hiring?

Using Software Copyright To Benefit the Public

Friday, March 21st, 2014

Imagine a world where copyright on a piece of software benefits the world even after it expires. A world where eventually all software becomes Free Software.

The purpose of copyright is “To promote the Progress of Science and useful Arts”. The law gives a person the right to profit from their creation for a while, after which everyone gets to profit from it freely. In general, this works for books, music, and other creative works. The current term of copyright is far too long, but at least once the term is up, the whole world gets to read and love Shakespeare or Walter de la Mare equally.

The same is not true of software. In order to be useful, software has to run. Imagine the great commercial software of the past decade: Excel, Photoshop, Pagemaker. Even after copyright expires on Microsoft Excel 95 (in 2090!), nobody will be able to run it! Hardware that can run Windows 95 will not be available, and our only hope of running the software is to emulate the machines and operating systems of a century ago. There will be no opportunity to fix or improve the software.

What should we reasonably require from commercial software producers in exchange for giving them copyright protection?

The code.

In order to get any copyright protection at all, publishers should be required to make the source code available. This can either happen immediately at release, or by putting the code into escrow until copyright expires. This needs to include everything required to build the program and make it run, but since the same copyright rules would apply to operating systems and compilers, it ought to all just work.

The copyright term for software also needs to be rethought. The goal when setting a copyright term should be to balance the competing desires of giving a software author time to make money by selling software, with the natural rights of people to share ideas and use and modify their own tools.

With a term of 14 years, the following software would be leaving copyright protection around now:

  • Windows 95
  • Excel 95
  • Photoshop 6.0
  • Adobe InDesign 1.0

A short copyright term is an incentive to software developers to constantly improve their software, and make the new versions of their software more valuable than older versions which are entering the public domain. It also opens the possibility for other companies to support old software even after the original author has decided that it isn’t worthwhile.

The European Union is currently holding a public consultation to review their copyright laws, and I’ve encouraged Mozilla to propose source availability and a shorter copyright term for software in our official contribution/proposal to that process. Maybe eventually the U.S. Congress could be persuaded to make such significant changes to copyright law, although recent history and powerful money and lobbyists make that difficult to imagine.

Commercial copyrighted software has done great things, and there will continue to be an important place in the world for it. Instead of treating the four freedoms as ethical absolutes and treating non-Free software as a “social problem”, let’s use copyright law to, after a period of time, make all software Free Software.

Use -debugexe to debug apps in Visual Studio

Monday, March 10th, 2014

Many people don’t know about how awesome the windows debuggers are. I recently got a question from a volunteer mentee: he was experiencing a startup crash in Firefox and he wanted to know how to get the debugger attached to Firefox before the crash.

On other systems, I’d say to use mach debug, but that currently doesn’t do useful things on Windows. But it’s still pretty simple. You have two options:

Debug Using Your IDE

Both Visual Studio and Visual C++ Express have a command-line option for launching the IDE ready for debugging.

devenv.exe -debugexe obj-ff-debug/dist/bin/firefox.exe -profile /c/builds/test-profile -no-remote

The -debugexe flag informs the IDE to load your Firefox build with the command lines you specify. Firefox will launch with the “Go” command (F5).

For Visual C++ express edition, run WDExpress.exe instead of devenv.exe.

Debug Using Windbg

windbg is a the Windows command-line debugger. As with any command-line debugger it has an arcane debugging syntax, but it is very powerful.

Launching Firefox with windbg doesn’t require any flags at all:

windbg.exe obj-ff-debug/dist/bin/firefox.exe -profile /c/builds/test-profile -no-remote

Debugging Firefox Release Builds

You can also debug Firefox release builds on Windows! Mozilla runs a symbol server that allows you to automatically download the debugging symbols for recent prerelease builds (I think we keep 30 days of nightly/aurora symbols) and all release builds. See the Mozilla Developer Network article for detailed instructions.

Debugging official builds can be a bit confusing due to inlining, reordering, and other compiler optimizations. I often find myself looking at the disassembly view of a function rather than the source view in order to understand what exactly is going on. Also note that if you are planning on debugging a release build, you probably want to disable automatic crash reporting by setting MOZ_CRASHREPORTER_DISABLE=1 in your environment.