|
| 1 | +# Contributing Code to WebKit |
| 2 | + |
| 3 | +WebKit has a rigorous code contribution process and policy in place to maintain the quality of code. |
| 4 | + |
| 5 | +### Coding style |
| 6 | + |
| 7 | +Code you write must follow WebKit’s [coding style guideline](https://webkit.org/contributing-code/#code-style-guidelines). |
| 8 | +You can run `Tools/Scripts/check-webkit-style` to check whether your code follows the coding guidelines or not |
| 9 | +(it can report false positives or false negatives). |
| 10 | +If you use `Tools/Scripts/webkit-patch upload` to upload your patch, |
| 11 | +it automatically runs the style checker against the code you changed so there is no need to run `check-webkit-style` separately. |
| 12 | + |
| 13 | +Some older parts of the codebase do not follow these guidelines. |
| 14 | +If you are modifying such code, it is generally best to clean it up to comply with the current guidelines. |
| 15 | + |
| 16 | +### Convenience Tools |
| 17 | + |
| 18 | +`Tools/Scripts/webkit-patch` provides a lot of utility functions like applying the latest patch on [bugs.webkit.org](https://bugs.webkit.org/) (`apply-from-bug`) |
| 19 | +and uploading a patch (`upload --git-commit=<commit hash>`) to a [bugs.webkit.org](https://bugs.webkit.org/) bug. |
| 20 | +Use `--all-commands` to the list of all commands this tool supports. |
| 21 | + |
| 22 | +### Regression Tests |
| 23 | + |
| 24 | +Once you have made a code change, you need to run the aforementioned tests (layout tests, API tests, etc...) |
| 25 | +to make sure your code change doesn’t break existing functionality. |
| 26 | +These days, uploading a patch on [bugs.webkit.org](https://bugs.webkit.org/) triggers the Early Warning System (a.k.a. EWS) |
| 27 | + |
| 28 | +For any bug fix or a feature addition, there should be a new test demonstrating the behavior change caused by the code change. |
| 29 | +If no such test can be written in a reasonable manner (e.g. the fix for a hard-to-reproduce race condition), |
| 30 | +then the reason writing a tests is impractical should be explained in the accompanying commit message. |
| 31 | + |
| 32 | +Any patch which introduces new test failures or performance regressions may be reverted. |
| 33 | +It’s in your interest to wait for the Early Warning System to fully build and test your patch on all relevant platforms. |
| 34 | + |
| 35 | +### Commit messages |
| 36 | + |
| 37 | +Commit messages serve as change logs, providing historical documentation for all changes to the WebKit project. |
| 38 | +Running `git-webkit setup` configures your git hooks to properly generate commit messages. |
| 39 | + |
| 40 | +The first line shall contain a short description of the commit message (this should be the same as the Summary field in Bugzilla). |
| 41 | +On the next line, enter the Bugzilla URL. |
| 42 | +Below the "Reviewed by" line, enter a detailed description of your changes. |
| 43 | +There will be a list of files and functions modified at the bottom of the commit message. |
| 44 | +You are encouraged to add comments here as well. (See the commit below for reference). |
| 45 | +Do not worry about the “Reviewed by NOBODY (OOPS!)” line, GitHub will update this field upon merging. |
| 46 | + |
| 47 | +``` |
| 48 | +Allow downsampling when invoking Remove Background or Copy Subject |
| 49 | +https://bugs.webkit.org/show_bug.cgi?id=242048 |
| 50 | +
|
| 51 | +Reviewed by NOBODY (OOPS!). |
| 52 | +
|
| 53 | +Soft-link `vk_cgImageRemoveBackgroundWithDownsizing` from VisionKitCore, and call into it to perform |
| 54 | +background removal when performing Remove Background or Copy Subject, if available. On recent builds |
| 55 | +of Ventura and iOS 16, VisionKit will automatically reject hi-res (> 12MP) images from running |
| 56 | +through subject analysis; for clients such as WebKit, this new SPI allows us to opt into |
| 57 | +downsampling these large images, instead of failing outright. |
| 58 | +
|
| 59 | +* Source/WebCore/PAL/pal/cocoa/VisionKitCoreSoftLink.h: |
| 60 | +* Source/WebCore/PAL/pal/cocoa/VisionKitCoreSoftLink.mm: |
| 61 | +* Source/WebCore/PAL/pal/spi/cocoa/VisionKitCoreSPI.h: |
| 62 | +* Source/WebKit/Platform/cocoa/ImageAnalysisUtilities.h: |
| 63 | +* Source/WebKit/Platform/cocoa/ImageAnalysisUtilities.mm: |
| 64 | +(WebKit::requestBackgroundRemoval): |
| 65 | +
|
| 66 | +Refactor the code so that we call `vk_cgImageRemoveBackgroundWithDownsizing` if it's available, and |
| 67 | +otherwise fall back to `vk_cgImageRemoveBackground`. |
| 68 | +
|
| 69 | +* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm: |
| 70 | +(-[WKContentView doAfterComputingImageAnalysisResultsForBackgroundRemoval:]): |
| 71 | +(-[WKContentView _completeImageAnalysisRequestForContextMenu:requestIdentifier:hasTextResults:]): |
| 72 | +(-[WKContentView imageAnalysisGestureDidTimeOut:]): |
| 73 | +* Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm: |
| 74 | +(WebKit::WebContextMenuProxyMac::appendMarkupItemToControlledImageMenuIfNeeded): |
| 75 | +(WebKit::WebContextMenuProxyMac::getContextMenuFromItems): |
| 76 | +
|
| 77 | +Additionally, remove the `cropRect` completion handler argument, since the new SPI function no |
| 78 | +longer provides this information. The `cropRect` argument was also unused after removing support for |
| 79 | +revealing the subject, in `249582@main`. |
| 80 | +``` |
| 81 | + |
| 82 | +The “No new tests. (OOPS!)” line will appear if `git webkit commit` did not detect the addition of new tests. |
| 83 | +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. |
| 84 | +Otherwise all changes require test cases which should be mentioned in the commit message. |
0 commit comments