Contributing Code

This page describes how to contribute changes to the WebKit source control repository. The WebKit project maintains severalscriptsto assist you. This page assumes you already know how tocheck outandbuildthe code.

Overview

Below are the recommended steps. Later sections of this page explain each step in more detail.

  1. Choose or create abug reportto work on.
  2. Developyour changes.
  3. Make sure your changes meet thecode style guidelines.Thecheck-webkit-stylescript may be of help.
  4. Run the layout tests using therun-webkit-testsscript and make sure they all pass. See thetesting pagefor more information, as well as what you need to do if you’ve modified JavaScriptCore.
  5. Add anynew filesto your working directory.
  6. Configure your WebKit checkout for upload pull-requests withTools/Scripts/git-webkit setup.
  7. Commit your local change withgit commit -a,the previously rungit-webkit setupcommand will have configured acommit message template
  8. RunTools/Scripts/git-webkit pull-requestto upload your commit for review onGitHub
  9. Make any changes recommended by the reviewer.
  10. Once reviewed, ask someone to land your change viaMerge-Queue.
  11. Please watch for any regressions it may have caused (hopefully none)!
    Flow chart on how to submit a patch
Flow chart on how to submit a patch

More detail about these steps is below.

Choose a Bug Report

Thebugs.webkit.orgdatabase is the central point of communication for contributions to WebKit. Nearly every contribution corresponds to a bug report there. Note that WebKit uses bug reports to track all types of code changes and not just bug fixes. Choose a bug report to work on. You can also create a new report. Be sure to search the database before creating new reports to avoid duplication. After choosing a bug report, follow the WebKitbug life cycleguidelines for the report. For example, it is often good practice to comment in a report if you are working on that issue. If your change may be controversial, you may want to check in advance with thewebkit-devmailing list.

Develop Your Changes

If you make substantive changes to a file, you may wish to add a copyright line for yourself or for the company on whose behalf you work. Below are sample copyright lines for an individual contributor and a company:Copyright (C) 2011 John Smith ([email protected])Copyright (C) 2011 Company Inc. All rights reserved.In addition, make sure that any new source code and script files you introduce contain license text at the beginning of the file. If you are the author of a new file, preferred license text to include can be found here:WebCore/LICENSE-APPLE.(The “Original Format” link at the bottom of the page contains text that can be cut and pasted more easily.) Simply replace the copyright line with your own information, for example as suggested above.

Code Style Guidelines

Patches must comply with thecode style guidelines.Some older parts of the codebase do not follow these guidelines. If you are modifying such code, it is generally best to clean it up to comply with the current guidelines. An exception is legacy components, which should not be cleaned up. Your patch will be automatically checked for style compliance before uploading if you usewebkit-patch upload.You can check style manually by running theTools/Scripts/check-webkit-stylescript. Style will also be checked on each patch after it is uploaded by theWebKit Early Warning System.

Regression Tests

Once you have made your changes, you need to run the regression tests, which is done via therun-webkit-testsscript. All tests must pass. Patches will not be landed in the tree if they break existing layout tests. For any feature that affects the layout engine, a new regression test must be constructed. If you provide a patch that fixes a bug, that patch should also include the addition of a regression test that would fail without the patch and succeed with the patch. If no regression test is provided, the reviewer will ask you to revise the patch, so you can save time by constructing the test up front and making sure it’s attached to the bug. If no layout test can be (or needs to be) constructed for the fix, you must explain why a new test isn’t necessary to the reviewer.

Tests for JavaScriptCore

If you’ve made changes to JavaScriptCore, execute therun-javascriptcore-testsscript. The script will run all the tests and summarize how the results differ from what is currently expected.

Add New Files to Your Working Directory

If your changes include adding new files (like new layout tests), use thegit addcommand to mark these files for addition to the repository. If you do not do this, the new files will be missing from the patch file you generate below. You can learn more about git commands likegit addfromgit’s online documentationor by using thegit --helpcommand.

Setting Up a Checkout

WebKit has a number of configurations we recommend you apply to your WebKit checkout. Ourwiki documents in detailwhat these are, we have automated this setup viaTools/Scripts/git-webkit setupand recommend contributors run this script before uploading a pull request.

Commit Messages

The WebKit project expects more detailed commit messages than many projects.Tools/Scripts/git-webkit setupwill configure a hook at.git/hooks/prepare-commit-msgin your WebKit checkout which will prepare a commit message template when you locally commit. Use this template to write up a brief summary of the changes you’ve made. Don’t worry about the “Reviewed by NOBODY (OOPS!)” line, the person reviewing your change will fill this in. A typical commit message entry before being landed looks like this:

Font::glyphDataAndPageForCharacter doesn't account for text orientation when using systemFallback on a cold cache.
https://bugs.webkit.org/show_bug.cgi?id=98452.

Reviewed by NOBODY (OOPS!).

The text orientation was considered only when there is a cache hit.
This change moves the logic to handle text orientation to a separate
inline function that is called also when the glyph is added to the cache.

Test: fast/text/vertical-rl-rtl-linebreak.html

* platform/graphics/FontFastPath.cpp:
(WebCore::applyTextOrientationForCharacter): Added.
(WebCore::Font::glyphDataAndPageForCharacter): Modified to use the new function in
both cases of cold and warm cache.

The “No new tests. (OOPS!)” line appears if tooling did not detect the addition of test cases. If your patch does not require test cases (or test cases are not possible), remove this line and explain why you didn’t write tests. Otherwise all changes require test cases which should be mentioned in the commit message. If you keep this line in your commit message entry, your patch will be rejected byMerge-Queue.

Respond to Reviewers

A WebKit reviewer must approve your patch before WebKit can accept it into the source control repository. A reviewer will typically either approve your patch (by responding with anr=mein the bug report and marking the pull request asApprovedin GitHub) or request revisions to your patch (andRequest changeson the pull request). In rare cases a patch may be permanently rejected, meaning that the reviewer believes the feature should never be committed to the tree. The review process can consist of multiple iterations between you and the reviewer as you submit revised patches.

Landing in the Tree

Once a change is approved, you should aska committer or reviewerto land your patch viaMerge-Queue.

Keeping the Tree Green

In either case, your responsibility for the patch does not end with the patch landing in the tree. There may be regressions from your change or additional feedback from reviewers after the patch has landed. You can watch the tree atbuild.webkit.orgto make sure your patch builds and passes tests on all platforms. It is your responsibility to be available should regressions arise and to respond to additional feedback that happens after a check-in. Changes should succeed on all platforms, but it can be difficult to test on every platform WebKit supports. Be certain that your change does not introduce new test failures on the high-traffic Mac or Windows ports by comparing the list of failing tests before and after your change. Your change must at least compile on all platforms.

Merge Queue

WebKit lands all changes through Merge-Queue or Unsafe-Merge-Queue.Committerscan add themerge-queueorunsafe-merge-queuelabels to a pull request to have that pull request merged into a production branch. Merge-Queue will block landing a change which breaks the build, and is thus the strongly recommended method of landing, unless a change has compelling reasons to land faster with less verification. See ourGitHub wikifor more details.

Obtaining Commit and Review Privileges

OurCommitter and Reviewer policyprovides details on obtaining commit and review privileges.