Bug 122408 – Generate toCSSFooValue() for CSSCursorImageValue, CSSCubicBezierTimingFunctionValue, CSSStepsTimingFunctionValue and CSSUnicodeRangeValue

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Gyuyoung Kim]

As a step to use toCSSFooValue, this patch generates toCSSFooValue() for CSSCursorImageValue, CSSCubicBezierTimingFunctionValue, CSSStepsTimingFunctionValue and CSSUnicodeRangeValue. This will help to detect bad type cast.

[Gyuyoung Kim]

Created attachment 213507 [details]
Patch

[Gyuyoung Kim]

CC'ing Darin.

[WebKit Commit Bot]

Comment on attachment 213507 [details]
Patch

Clearing flags on attachment: 213507

Committed r156988: <http://trac.webkit.org/changeset/156988>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 122415

[Alexey Proskuryakov]

This broke debug build, rolled out in r156993.

http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20%28Build%29/builds/17023/steps/compile-webkit/logs/stdio

[Darin Adler]

Darn, too bad it was rolled out since I was about to fix it.

[Gyuyoung Kim]

Created attachment 213544 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #7)
> Darn, too bad it was rolled out since I was about to fix it.

Darin, that is really my fault. Patch is fixed on debug build. Sorry for annoying you again.

[Gyuyoung Kim]

Comment on attachment 213544 [details]
Patch

Kling, thank you for your review. I will check debug buildbot after landing this patch.

[WebKit Commit Bot]

Comment on attachment 213544 [details]
Patch

Clearing flags on attachment: 213544

Committed r157024: <http://trac.webkit.org/changeset/157024>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

> Darn, too bad it was rolled out since I was about to fix it.

Did re-landing this patch with added fix present a substantial difficulty? I'd like to know what are the cases where immediate rollout is not the best thing to do for anyone who notices breakage.

[Darin Adler]

(In reply to comment #13)
> Did re-landing this patch with added fix present a substantial difficulty?

My personal preference, since this had a (to me) obvious one-line fix, would be to roll it back in.

> I'd like to know what are the cases where immediate rollout is not the best thing to do for anyone who notices breakage.

I can’t really answer that question for the project.

I personally find all roll-outs and roll-back-ins quite confusing in project history. I especially dislike change log entries that identify a roll-out only by revision number, and the duplicate nearly-identical change logs we have when we roll back in. We did the WebKit project for many years with very few roll-outs, but it’s possible that the quick roll-outs solve some problems that we had in those days.

These past roll-outs and roll-back-ins often get in my way when understanding the code in the project and looking at file and function history.

I personally would almost never think an immediate roll-out is the right thing to do; I’d briefly look for an obvious fix before rolling *anything* out. But my tastes are not project policy.

[Darin Adler]

(In reply to comment #14)
> (In reply to comment #13)
> > Did re-landing this patch with added fix present a substantial difficulty?
> 
> My personal preference, since this had a (to me) obvious one-line fix, would be to roll it back in.

I meant to say, “would be to fix the problem instead of rolling out”.

[Alexey Proskuryakov]

> I meant to say, “would be to fix the problem instead of rolling out”.

I looked at the build failure, and it wasn't an obvious fix to me.

When saying "immediate", I intentionally omitted the step of briefly looking into fixing the failure. I don't want a person doing the rollout to ever be blamed for insufficient technical ability to resolve an issue.

It should be a good thing for anyone to look at the bots, and to fix them when noticing badness. There should be some standards to follow, like making a minimal attempt to contact the author and possibly current bot watchers, and of course providing good information about what was broken for future investigation. That last part is something we used to have trouble with when rollout bot was first implemented, and I think that it's become much better now.

I don't think that judging the job on whether the person had a fresh checkout, was able to spot the mistake, or even had sufficient time and energy for that will be best in practice.

> I personally find all roll-outs and roll-back-ins quite confusing in project history. I especially dislike change log entries that identify a roll-out only by revision number, and the duplicate nearly-identical change logs we have when we roll back in.

This sounds like something that can be improved. I now remember being hugely annoyed by such commit message and ChangeLog noise when I was reading all commit messages.

Perhaps this would work:
- Provide reverted change title in webkitbot-generated rollout ChangeLog.
- Don't add changed function and file lists to these ChangeLogs.

> We did the WebKit project for many years with very few roll-outs, but it’s possible that the quick roll-outs solve some problems that we had in those days.

It used to be that people more commonly ran regression tests locally, and running Mac Debug tests was practically sufficient to avoid negatively affecting everyone else's productivity.

Tests are still in a pretty bad shape to expect people to run them locally. I'm going to solve this by gradually cutting down on the number of unreliable tests, and by decreasing the likelyhood of making a checkout at a wrong time.

Very quick resolution of build and test failures is necessary for people to be confident about their checkout 
as a clean baseline. If fixing a regression takes 15 minutes, it's still 2-3 times worse than a rollout.

The issue with more modes that we have now (WK1 vs. WK2, and multiple platforms) is meant to be solved by EWS. It obviously doesn't help with debug builds now, and doesn't run the whole test suite that test bots run. But worst of all, it's slow, which makes people ignore it sometimes. UI could be improved, too.

I think that making EWS faster and better will decrease the need for rollouts, and once we have the system in a decent shape, we should add debug EWS bots too. Reliably good state of the tree helps EWS do its work quickly (we may be able improve EWS to be more tolerant of bad tree state, but we are certainly not there yet).