| OLD | NEW |
| 1 How to land Skia changes that change Blink layout test results | 1 Blink layout tests |
| 2 ============================================================== | 2 ================== |
| 3 | 3 |
| 4 How to land Skia changes that change Blink layout test results. |
| 5 |
| 4 Changes that affect a small number of layout test results | 6 Changes that affect a small number of layout test results |
| 5 --------------------------------------------------------- | 7 --------------------------------------------------------- |
| 6 Changes affecting fewer than ~20 layout tests can be rebaselined without special | 8 Changes affecting fewer than ~20 layout tests can be rebaselined without |
| 7 coordination with the Blink gardener using these steps: | 9 special coordination with the Blink gardener using these steps: |
| 8 | 10 |
| 9 1. Prepare your Skia change, taking note of which layout tests will turn red | 11 1. Prepare your Skia change, taking note of which layout tests will turn red |
| 10 \(see http://www.chromium.org/developers/testing/webkit-layout-tests for more | 12 \(see http://www.chromium.org/developers/testing/webkit-layout-tests for more |
| 11 detail on running the Blink layout tests\). | 13 detail on running the Blink layout tests\). |
| 12 2. Check in your code to the Skia repo. | 14 2. Check in your code to the Skia repo. |
| 13 3. Ahead of the Skia auto roll including your change, manually push a change to
the | 15 3. Ahead of the Skia auto roll including your change, manually push a change to
the |
| 14 Blink LayoutTests/TestExpectations [file](http://src.chromium.org/viewvc/blin
k/trunk/LayoutTests/TestExpectations), | 16 Blink LayoutTests/TestExpectations [file](http://src.chromium.org/viewvc/blin
k/trunk/LayoutTests/TestExpectations), |
| 15 flagging tests expected to fail as a result of your change with \[ NeedsManua
lRebaseline \]. | 17 flagging tests expected to fail as a result of your change with \[ NeedsManua
lRebaseline \]. |
| 16 4. Wait for the Skia roll to land successfully. | 18 4. Wait for the Skia roll to land successfully. |
| 17 5. Check in another change to the Blink TestExpectations file changing the flags
to | 19 5. Check in another change to the Blink TestExpectations file changing the flags
to |
| 18 \[ NeedsRebaseline\], which will prompt the automatic rebaseline. | 20 \[ NeedsRebaseline\], which will prompt the automatic rebaseline. |
| 19 | 21 |
| 20 | 22 |
| 21 | 23 |
| 22 Changes that affect a large number of test results | 24 Changes that affect a large number of test results |
| 23 -------------------------------------------------- | 25 -------------------------------------------------- |
| 24 Where a 'large number' or 'many' means more than about 20. | 26 Where a 'large number' or 'many' means more than about 20. |
| 25 Follow the instructions below: | 27 Follow the instructions below: |
| 26 | 28 |
| 27 In the following the term 'code suppression' means a build flag \(a\.k\.a\. defi
ne\). | 29 In the following the term 'code suppression' means a build flag \(a\.k\.a\. defi
ne\). |
| 28 Such code suppressions should be given a name with the form SK\_IGNORE\_xxx\_FIX
. | 30 Such code suppressions should be given a name with the form SK\_IGNORE\_xxx\_FIX
. |
| 29 | 31 |
| 30 There are dependency revisions which must be updated in this | 32 There are dependency revisions which must be updated in this process. Updating |
| 31 process. Updating a dependency revision is called a 'roll'. | 33 a dependency revision is called a 'roll'. There are two different rolls which |
| 32 There are two different rolls which concern this process, each of which happens | 34 concern this process, each of which happens via an Auto Roll Bot multiple |
| 33 via an Auto Roll Bot multiple times per day, and can also be done manually: | 35 times per day, and can also be done manually: |
| 34 | 36 |
| 35 * Skia roll into Chromium. See | 37 * Skia roll into Chromium. See |
| 36 https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search for | 38 https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search for |
| 37 skia\-deps\-roller. | 39 skia\-deps\-roller. |
| 38 * Blink roll into Chromium. See | 40 * Blink roll into Chromium. See |
| 39 https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search for | 41 https://chromium.googlesource.com/chromium/src/+log/master/DEPS and search for |
| 40 blink\-deps\-roller. | 42 blink\-deps\-roller. |
| 41 | 43 |
| 42 ### Setup | 44 ### Setup |
| 43 #### Code suppression does not yet exist \- Direct method | 45 #### Code suppression does not yet exist \- Direct method |
| 44 1. Make a change in Skia which will change many Blink layout tests. | 46 1. Make a change in Skia which will change many Blink layout tests. |
| 45 2. Put the change behind a code suppression. | 47 2. Put the change behind a code suppression. |
| 46 3. Check in the change to the Skia repository. | 48 3. Check in the change to the Skia repository. |
| 47 4. Manually roll Skia or append the autoroll with the code suppression to | 49 4. Manually roll Skia or append the autoroll with the code suppression to |
| 48 Chromium's 'skia/skia\_common\.gypi' | 50 Chromium's 'skia/skia\_common\.gypi' |
| 49 5. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp'. | 51 5. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp'. |
| 50 6. Wait for Blink roll into Chromium. | 52 6. Wait for Blink roll into Chromium. |
| 51 7. Remove code suppression from Chromium's 'skia/skia\_common\.gypi'. | 53 7. Remove code suppression from Chromium's 'skia/skia\_common\.gypi'. |
| 52 | 54 |
| 53 #### Code suppression does not yet exist \- Alternate method | 55 #### Code suppression does not yet exist \- Alternate method |
| 54 1. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp' before maki
ng code | 56 1. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp' before maki
ng code |
| 55 changes in Skia. | 57 changes in Skia. |
| 56 2. Make a change in Skia which will change many Blink layout tests. | 58 2. Make a change in Skia which will change many Blink layout tests. |
| 57 3. Put the change behind a code suppression. | 59 3. Put the change behind a code suppression. |
| 58 4. Wait for Blink roll into Chromium. | 60 4. Wait for Blink roll into Chromium. |
| 59 5. Check in the change to the Skia repository. | 61 5. Check in the change to the Skia repository. |
| 60 6. Wait for Skia roll into Chromium. | 62 6. Wait for Skia roll into Chromium. |
| 61 | 63 |
| 62 #### Code suppression exists in header | 64 #### Code suppression exists in header |
| 63 1. Remove code suppression from header file in Chromium and add code suppression
to | 65 1. Remove code suppression from header file in Chromium and add code suppression
to |
| 64 Chromium's 'skia/skia\_common\.gypi'. | 66 Chromium's 'skia/skia\_common\.gypi'. |
| 65 The code suppression cannot be in a header file and a defined in a gyp file a
t the | 67 The code suppression cannot be in a header file and a defined in a gyp file a
t the |
| 66 same time or a multiple definition warning will be treated as an error and br
eak | 68 same time or a multiple definition warning will be treated as an error and br
eak |
| 67 the Chromium build. | 69 the Chromium build. |
| 68 2. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp'. | 70 2. Add code suppression to Blink's 'public/blink\_skia\_config\.gyp'. |
| 69 3. Wait for Blink roll into Chromium. | 71 3. Wait for Blink roll into Chromium. |
| 70 4. Remove code suppression from Chromium's 'skia/skia\_common\.gypi'. | 72 4. Remove code suppression from Chromium's 'skia/skia\_common\.gypi'. |
| 71 | 73 |
| 72 ### Rebaseline | 74 ### Rebaseline |
| 73 1. Choose a time when the Blink tree is likely to be quiet. Avoid PST afternoons
in | 75 1. Choose a time when the Blink tree is likely to be quiet. Avoid PST afternoons
in |
| 74 particular. The bigger the change, the more important this is. Regardless, | 76 particular. The bigger the change, the more important this is. Regardless, |
| 75 determine who the Blink gardener is and notify them. You will be making the | 77 determine who the Blink gardener is and notify them. You will be making the |
| 76 Chromium\.WebKit tree very red for an extended period, and the gardener needs
to | 78 Chromium\.WebKit tree very red for an extended period, and the gardener needs
to |
| 77 know that they are not expected to fix it. | 79 know that they are not expected to fix it. |
| 78 2. Create a CL removing the code suppression from Blink's | 80 2. Create a CL removing the code suppression from Blink's |
| 79 public/blink_skia_config.gyp while simultaneously adding [ NeedsRebaseline ] | 81 public/blink_skia_config.gyp while simultaneously adding [ NeedsRebaseline ] |
| 80 lines to Blink's LayoutTests/TestExpectations [file](http://src.chromium.org/
viewvc/blink/trunk/LayoutTests/TestExpectations). | 82 lines to Blink's LayoutTests/TestExpectations [file](http://src.chromium.org/
viewvc/blink/trunk/LayoutTests/TestExpectations). |
| 81 Then the auto rebaseline bot will take care of the work of actually checking
in the | 83 Then the auto rebaseline bot will take care of the work of actually checking
in the |
| 82 new images. This is generally acceptable for up to 600 or so rebaselined imag
es. | 84 new images. This is generally acceptable for up to 600 or so rebaselined imag
es. |
| 83 Above that you might still use [ NeedsRebaseline ], but it's best to coordina
te with | 85 Above that you might still use [ NeedsRebaseline ], but it's best to coordina
te with |
| 84 the gardener/sheriff. This should go through the CQ cleanly. | 86 the gardener/sheriff. This should go through the CQ cleanly. |
| 85 3. Be careful with tests that are already failing or flakey. These may or may no
t need | 87 3. Be careful with tests that are already failing or flakey. These may or may no
t need |
| 86 to be rebaselined and flakey tests should not be removed from TestExpectation
s | 88 to be rebaselined and flakey tests should not be removed from TestExpectation
s |
| 87 regardless. In such cases revert the TestExpectations changes before committi
ng. | 89 regardless. In such cases revert the TestExpectations changes before committi
ng. |
| 88 4. If you are not the one handling the cleanup step, please open a Skia Issue of
the | 90 4. If you are not the one handling the cleanup step, please open a Skia Issue of
the |
| 89 form | 91 form |
| 90 Title: "Remove code suppression SK\_IGNORE\_xxx\_FIX\." | 92 Title: "Remove code suppression SK\_IGNORE\_xxx\_FIX\." |
| 91 Comment: "Code suppression SK\_IGNORE\_xxx\_FIX rebaselined with Blink revisi
on | 93 Comment: "Code suppression SK\_IGNORE\_xxx\_FIX rebaselined with Blink revisi
on |
| 92 123456\." and assign it to the individual responsible for the cleanup step. | 94 123456\." and assign it to the individual responsible for the cleanup step. |
| 93 | 95 |
| 94 ### Cleanup | 96 ### Cleanup |
| 95 1. Wait for Blink roll into Chromium, so that Chromium is using the new Skia cod
e | 97 1. Wait for Blink roll into Chromium, so that Chromium is using the new Skia cod
e |
| 96 and new Blink baselines. | 98 and new Blink baselines. |
| 97 2. Remove the now unused old code from Skia and any defines which were introduce
d | 99 2. Remove the now unused old code from Skia and any defines which were introduce
d |
| 98 to suppress the new code. | 100 to suppress the new code. |
| 99 3. Check in the cleanup change to the Skia repository. | 101 3. Check in the cleanup change to the Skia repository. |
| 100 4. Wait for Skia roll into Chromium. | 102 4. Wait for Skia roll into Chromium. |
| 101 | 103 |
| OLD | NEW |