Bug 99759 - Create skia_webkit.gyp to improve rebaselining.
Summary: Create skia_webkit.gyp to improve rebaselining.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: bungeman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-18 14:47 PDT by bungeman
Modified: 2012-10-24 12:31 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.36 KB, patch)
2012-10-18 14:50 PDT, bungeman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description bungeman 2012-10-18 14:47:35 PDT
Create skia_webkit.gyp to improve rebaselining.
Comment 1 bungeman 2012-10-18 14:50:48 PDT
Created attachment 169479 [details]
Patch
Comment 2 bungeman 2012-10-18 19:47:12 PDT
If this is committed, then https://codereview.chromium.org/11191074 would be checked in to Chromium after the next WebKit roll into Chromium.
Comment 3 Stephen Chenney 2012-10-22 08:34:07 PDT
Am I correct in understanding that there is a matching chromium patch that adds the reference to this file?

To rebaseline, we would:
1. Change a flag in this new skia_webkit.gyp file
2. Watch the build bots generate all the new images (with lots and lots of failures)
3. Rebaseline WebKit
4. Roll WebKit into Chromium, and pick up both the flag change and the new baselines

Sounds to me like a much better workflow.

The only downside is that it adds the possibility that a Skia change in WebKit might cause breakage of chromium browser or unit tests on the WebKit roll. I don't think we can really do anything much about that, but it does mean that the skia team will have to be aware when a flag change rolls into chromium.
Comment 4 bungeman 2012-10-22 09:44:52 PDT
>Am I correct in understanding that there is a matching chromium patch that adds the reference to this file?

See https://codereview.chromium.org/11191074

>To rebaseline, we would:

Your description is accurate as to the intended workflow. The entire process previous to this is something like.

-4. Make a change in Skia behind a build flag because it is known to need new baselines in WebKit.
-3. Roll Skia into Chromium with the code suppression flag in Chromium's skia.gyp.
-2. At any point here or previous, get the code suppression flag into the skia_webkit.gyp and roll (or wait for roll of) WebKit into Chromium.
-1. Remove suppression from Chromium's skia.gyp.
 0. Roll (or wait for roll of) Chromium into WebKit.

At this point the rebaseline can proceed as you describe.

>might cause breakage of chromium browser or unit tests on the WebKit roll
To mitigate this, we can send the full change to the trybots ahead of time to make sure that only layout tests are affected.
Comment 5 Dirk Pranke 2012-10-22 17:35:24 PDT
This change (and the corresponding Chromium-side change) look fine (well, frankly, it looks ugly but I don't think there's another way to solve this), but I'd like to hold off on landing it until after we can meet to discuss the flow on Wednesday, if that's okay.
Comment 6 Dirk Pranke 2012-10-24 12:21:27 PDT
Comment on attachment 169479 [details]
Patch

We agreed to land it :).
Comment 7 WebKit Review Bot 2012-10-24 12:31:23 PDT
Comment on attachment 169479 [details]
Patch

Clearing flags on attachment: 169479

Committed r132385: <http://trac.webkit.org/changeset/132385>
Comment 8 WebKit Review Bot 2012-10-24 12:31:27 PDT
All reviewed patches have been landed.  Closing bug.