webkit-patch optimize-baselines thinks it's succeeding, but it's not

• Component: Tools / Tests
• Status: NEW ---

[Ojan Vafai]

1. Sync to r130233 (currently tip of tree).
2. webkit-patch optimize-baselines fast/canvas/canvas-render-layer.html -v

Outputs:
Optimizing fast/canvas/canvas-render-layer.html.
webkitpy.common.checkout.baselineoptimizer: [DEBUG] No optimization found, optimal?
webkitpy.common.checkout.baselineoptimizer: [DEBUG] No optimization found, optimal?
webkitpy.common.checkout.baselineoptimizer: [DEBUG] No optimization found, optimal?

But, before and after I end up with:
./platform/gtk/fast/canvas/canvas-render-layer-expected.txt
./platform/gtk/fast/canvas/canvas-render-layer-expected.png
./platform/chromium-mac-snowleopard/fast/canvas/canvas-render-layer-expected.txt
./platform/chromium-win/platform/chromium/virtual/gpu/fast/canvas/canvas-render-layer-expected.png
./platform/chromium-win/fast/canvas/canvas-render-layer-expected.txt
./platform/chromium-win/fast/canvas/canvas-render-layer-expected.png
./platform/chromium-linux/fast/canvas/canvas-render-layer-expected.txt
./platform/chromium-win-xp/fast/canvas/canvas-render-layer-expected.txt
./platform/win/fast/canvas/canvas-render-layer-expected.txt
./platform/chromium-mac/platform/chromium/virtual/gpu/fast/canvas/canvas-render-layer-expected.png
./platform/chromium-mac/fast/canvas/canvas-render-layer-expected.txt
./platform/chromium-mac/fast/canvas/canvas-render-layer-expected.png
./platform/mac/fast/canvas/canvas-render-layer-expected.txt
./platform/mac/fast/canvas/canvas-render-layer-expected.png
./platform/efl/fast/canvas/canvas-render-layer-expected.txt
./platform/efl/fast/canvas/canvas-render-layer-expected.png
./platform/chromium/platform/chromium/virtual/gpu/fast/canvas/canvas-render-layer-expected.txt

All the -expected.txt are the same except the platform/mac one matches the chromium virtual/gpu one. So, since everyone falls back to platform/mac, we can't do better except the chromium -expected.txt results should all go in platform/chromium.

[Zan Dobersek]

(In reply to comment #0)
> All the -expected.txt are the same except the platform/mac one matches the chromium virtual/gpu one. So, since everyone falls back to platform/mac, we can't do better except the chromium -expected.txt results should all go in platform/chromium.

By stating 'since everyone falls back to platform/mac', do you mean the GTK baselines as well? If so, that's not correct.

r132121[1] has removed GTK baselines for svg/transforms/svg-css-transforms-clip-path.html and svg/transforms/svg-css-transforms-clip-path.html tests probably because the text baselines match the baselines in platform/mac. However, the GTK port in NRWT doesn't then fall back to these Mac baselines and the tests were being reported as missing results.

[1] http://trac.webkit.org/changeset/132121

[Ojan Vafai]

(In reply to comment #1)
> (In reply to comment #0)
> > All the -expected.txt are the same except the platform/mac one matches the chromium virtual/gpu one. So, since everyone falls back to platform/mac, we can't do better except the chromium -expected.txt results should all go in platform/chromium.
> 
> By stating 'since everyone falls back to platform/mac', do you mean the GTK baselines as well? If so, that's not correct.

I misspoke. I think now only Apple ports fallback to platform/mac.

> r132121[1] has removed GTK baselines for svg/transforms/svg-css-transforms-clip-path.html and svg/transforms/svg-css-transforms-clip-path.html tests probably because the text baselines match the baselines in platform/mac. However, the GTK port in NRWT doesn't then fall back to these Mac baselines and the tests were being reported as missing results.

Ugh. That's not good. :(

[Dirk Pranke]

(In reply to comment #2)
> (In reply to comment #1)
> > (In reply to comment #0)
> > > All the -expected.txt are the same except the platform/mac one matches the chromium virtual/gpu one. So, since everyone falls back to platform/mac, we can't do better except the chromium -expected.txt results should all go in platform/chromium.
> > 
> > By stating 'since everyone falls back to platform/mac', do you mean the GTK baselines as well? If so, that's not correct.
> 
> I misspoke. I think now only Apple ports fallback to platform/mac.
>

Right, so I'm not sure you're reporting a real bug here. 

> > r132121[1] has removed GTK baselines for svg/transforms/svg-css-transforms-clip-path.html and svg/transforms/svg-css-transforms-clip-path.html tests probably because the text baselines match the baselines in platform/mac. However, the GTK port in NRWT doesn't then fall back to these Mac baselines and the tests were being reported as missing results.
> 
> Ugh. That's not good. :(

This, however, I think is a real bug. I've noticed this too in some of my chromium mountain lion rebaselining; I had assumed that there were generic -expected files that were redundant with the gtk files, but I am sad to say I didn't actually verify this. I will dig into it more.

[Ojan Vafai]

This is still a real bug. The chromium -expected.txt results should be merged into platform/chromium.

[Dirk Pranke]

(In reply to comment #4)
> This is still a real bug. The chromium -expected.txt results should be merged into platform/chromium.

Oh, sorry, I missed that. I'll take a look at that, too.

[Dirk Pranke]

(In reply to comment #5)
> (In reply to comment #4)
> > This is still a real bug. The chromium -expected.txt results should be merged into platform/chromium.
> 
> Oh, sorry, I missed that. I'll take a look at that, too.

Okay, running webkit-patch optimize-baselines -v fast/canvas/canvas-render-layer.html (with slightly tweaked logging) now, I get:

Optimizing fast/canvas/canvas-render-layer.html.
No optimization found for canvas-render-layer-expected.png, optimal?
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-win: f9f3e2
  Tools/Scripts/webkitpy/LayoutTests/platform/mac: 07349f
  Tools/Scripts/webkitpy/LayoutTests/platform/efl: 148b27
  Tools/Scripts/webkitpy/LayoutTests/platform/qt: 79670b
  Tools/Scripts/webkitpy/LayoutTests/platform/gtk: a33e86
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-mac: b426b7
No baselines found for canvas-render-layer-expected.wav
Optimizing canvas-render-layer-expected.txt
before: 
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-win: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/mac: 439e3b
  Tools/Scripts/webkitpy/LayoutTests/platform/efl: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/qt: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-win-xp: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/gtk: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/win: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-mac-snowleopard: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-linux: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/chromium-mac: 0b0691
after: 
  Tools/Scripts/webkitpy/LayoutTests/platform/win-7sp0: 0b0691
  Tools/Scripts/webkitpy/LayoutTests/platform/mac: 439e3b
  Tools/Scripts/webkitpy/LayoutTests: 0b0691

So perhaps now that chromium-mac doesn't fall back to mac, things are working okay (in Ojan's case, at least)?

[Ojan Vafai]

Yeah. Presumably there's still a bug in the code logic. Not sure if it matters in practice anymore.

[Ahmad Saleem]

[e-mail] - Is this still an issue or needed? Thanks!

[Alexey Proskuryakov]

I don't know, but chances are that it is still an issue. I am not sure if `webkit-patch optimize-baselines` is being used much, perhaps we should add some telemetry.