|Summary:||Implement CSS border radius properies in CSSStyleApplyProperty|
|Product:||WebKit||Reporter:||Luke Macpherson <macpherson>|
|Component:||New Bugs||Assignee:||Luke Macpherson <macpherson>|
|Severity:||Normal||CC:||dglazkov, eric, macpherson, pfeldman, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Luke Macpherson 2011-06-07 22:25:37 PDT
Implement CSS border radius properies in CSSStyleApplyProperty
Comment 2 Eric Seidel (no email) 2011-06-07 22:41:59 PDT
Comment on attachment 96381 [details] Patch Why are we moving away from const &? That's going to cause LengthSize to copy. Is LengthSize expensive to copy?
Comment 3 Luke Macpherson 2011-06-07 22:47:44 PDT
I should prehaps have pr-empted this question. LengthSize contains exactly two lengths, and we pass Lengths by value already. Length is pretty small (< 64 bits). Seems pretty safe to me on most any modern architecture.
Comment 4 Luke Macpherson 2011-06-09 17:16:39 PDT
Comment 5 Eric Seidel (no email) 2011-06-09 22:37:51 PDT
Comment on attachment 96381 [details] Patch Looks good. I'm not sure the removal of const & will be a win on all platforms... but I don't really know.
Comment 6 Eric Seidel (no email) 2011-06-09 22:38:22 PDT
What's the disadvantage of using the const&?
Comment 7 Luke Macpherson 2011-06-09 22:53:29 PDT
The disadvantage is that at the moment the types are a bit mix-and-match, so it made the templates easier to write / understand if the same parameter type can be used everywhere. This could also go the other way (all to const &) I suppose.
Comment 8 Eric Seidel (no email) 2011-06-10 10:37:29 PDT
Comment on attachment 96381 [details] Patch OK.
Comment 9 WebKit Review Bot 2011-06-10 11:33:51 PDT
Comment on attachment 96381 [details] Patch Rejecting attachment 96381 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: 395a87712d692b0fc87844413a797b237c0448a0 r88553 = 7ae9a37d521f373bda7f5faa56104624249d647b Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Updating chromium port dependencies using gclient... ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/8826458
Comment 11 Luke Macpherson 2011-06-13 23:44:43 PDT
Comment 12 Eric Seidel (no email) 2011-06-14 08:06:25 PDT
Comment on attachment 97072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97072&action=review > Source/WebCore/ChangeLog:1 > +2011-06-13 Luke Macpherson <firstname.lastname@example.org> For whatever reason there seems to be an extra space between your name and your email here.
Comment 13 WebKit Review Bot 2011-06-14 08:46:37 PDT
Comment on attachment 97072 [details] Patch Clearing flags on attachment: 97072 Committed r88804: <http://trac.webkit.org/changeset/88804>
Comment 14 WebKit Review Bot 2011-06-14 08:46:42 PDT
All reviewed patches have been landed. Closing bug.
Comment 15 Pavel Feldman 2011-06-14 08:49:04 PDT
Committed r88805: <http://trac.webkit.org/changeset/88805>
Comment 16 Luke Macpherson 2011-06-14 23:17:49 PDT
Ok, I'm confused. http://trac.webkit.org/changeset/88804 look like this patch http://trac.webkit.org/changeset/88805 has the description of this patch, but clearly isn't this patch if you look at the diffs. What happened?
Comment 17 Pavel Feldman 2011-06-14 23:28:02 PDT
(In reply to comment #16) > Ok, I'm confused. > http://trac.webkit.org/changeset/88804 look like this patch > http://trac.webkit.org/changeset/88805 has the description of this patch, but clearly isn't this patch if you look at the diffs. > > What happened? @eric: A webkit-patch glitch? I think I was using "webkit-patch land" with the patch from https://bugs.webkit.org/show_bug.cgi?id=62640 in my working tree. It did not go through, so I pulled from git and did "webkit-patch land" again.
Comment 18 Pavel Feldman 2011-06-14 23:31:58 PDT
> @eric: A webkit-patch glitch? I think I was using "webkit-patch land" with the patch from https://bugs.webkit.org/show_bug.cgi?id=62640 in my working tree. It did not go through, so I pulled from git and did "webkit-patch land" again. Oh, this is me to blame: my patch was based on the tree with my another patch being on top. ChangeLog resolves poorly in these cases and I just did not check it after pulling your change. We could fix ChangeLog resolving for this case though.