Re: [webkit-dev] Thank You
On Wed, Apr 3, 2013 at 2:02 PM, Eric Seidel e...@webkit.org wrote: p.s. Adam and I are happy to work with other reviewers to remove PLATFORM(CHROMIUM) code and other messes we may have caused over the years from webkit.org. Adam and I are still running queues.webkit.org and associated EWS/CQ/sherriff-bot and plan to do so for the next few weeks as we work to transition them to new owners. Ditto for the flakiness dashboard at test-results.appspot.com. Ping me if you'd like to be the new owner for it. :) ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Merging the iOS port to svn.webkit.org (Re: WebCore and interlocking threads)
On Sat, Feb 9, 2013 at 7:41 PM, Maciej Stachowiak m...@apple.com wrote: Hi Peter, On Feb 9, 2013, at 3:48 PM, Peter Kasting pkast...@google.com wrote: On Sat, Feb 9, 2013 at 11:52 AM, Maciej Stachowiak m...@apple.com wrote: There are certainly pros and cons to merging. It would be great get input from the broader WebKit community on the tradeoff of merging sooner vs avoidance of weird legacy code in the main tree. In the meantime, we'll stick to merging things that are not overly controversial as much as we can. For what my opinion is worth (probably near zero for a lot of you), I would like to see you guys merge sooner rather than later, even if it leads to awkwardness that needs cleanup. Over the years there has been a nonzero amount of friction due to the iOS port not being upstreamed, and I think it would be beneficial to WebKit as a whole to fix that sooner rather than later. And it seems more likely to me that upstream first, then decide how to re-architect as needed is going to result in high-quality discussions and designs, as opposed to figure out in private how to re-architect before upstreaming, which runs the risk of just never bothering to upstream at all. There is real compromise, and perhaps humility, needed from all sides to make such a task successful. I am reminded of Eric's recent email where he pleaded for more of an explicit effort to civility towards each other. Perhaps this is an opportunity for us to practice that effort. I really appreciate you saying that. I feel the same way. For years we've been saying that we need to fix N different things before upstreaming, and in the end we concluded that it was just delaying us from upstreaming at all. And we concluded that cleaning up some of the questionable choices in the open would lead to a better final outcome. +1. I think that allowing the V8 bindings into the tree is a fairly good analogy to this situation (although it's not exactly the same). The point is that it did put a burden on the rest of the project at the benefit of getting Chromium folk working on core code and giving the ability to discuss trade-offs more directly by looking at the code in question. At the same time, I think some scrutiny of what we are doing is fair, so I'll try to explain the threading issues in a bit more detail to the extent I can. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] PSA: review tool dropping comments
If you expand diff context and then publish comments, the review tool might drop some of your comments. I'll have this fixed today. In the meantime, if you avoid expanding context or reload the page before publishing, you should avoid hitting this bug. https://bugs.webkit.org/show_bug.cgi?id=105252 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] PSA: review tool dropping comments
Should be fixed. Let me know if you see problems. On Wed, Jan 2, 2013 at 11:41 AM, Ojan Vafai o...@chromium.org wrote: If you expand diff context and then publish comments, the review tool might drop some of your comments. I'll have this fixed today. In the meantime, if you avoid expanding context or reload the page before publishing, you should avoid hitting this bug. https://bugs.webkit.org/show_bug.cgi?id=105252 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to
On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund e...@chromium.org wrote: I'll have to disagree with you here. If the build is broken and the gardener/build cop has a strong reason to suspect that it was caused by a specific patch and the author is unavailable then rolling that patch out is the right thing to do. It author is unavailable is the key statement here. That said, if your strong reason turned out to be incorrect, you should recommit the patch, no? might inconvenience the author but it is the responsibility of the author and reviewer to make sure the patch didn't break anything. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Please avoid rolling out patches speculatively and reland them ASAP if you had to
On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund e...@chromium.org wrote: On Tue, Dec 11, 2012 at 12:19 PM, Ojan Vafai o...@chromium.org wrote: On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund e...@chromium.org wrote: I'll have to disagree with you here. If the build is broken and the gardener/build cop has a strong reason to suspect that it was caused by a specific patch and the author is unavailable then rolling that patch out is the right thing to do. It author is unavailable is the key statement here. Indeed. That said, if your strong reason turned out to be incorrect, you should recommit the patch, no? That seems like a bad idea, someone that understands the patch should recommit it. Ideally the original author. If it needs manual patching then you need to include the original author, but otherwise, I don't see why. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Adding main element to WebCore
On Tue, Nov 27, 2012 at 1:47 PM, Ojan Vafai o...@chromium.org wrote: As I said on the thread you link to, I don't think this element addresses any real use-cases. I think people are far too likely to misuse this for it to be useful for things like readability to use. If Apple really wants this, I won't object, but my preference would be to not implement this. In retrospect, I crafted this email a bit carelessly. I only mentioned Apple because Maciej had expressed some desire for main on the w3c threads. All I meant to say is that I wouldn't fight to block this feature if another reviewer felt there was enough agreement to r+ the patch. On Mon, Nov 26, 2012 at 2:01 PM, Steve Faulkner faulkner.st...@gmail.comwrote: Hi all, this is my first post to the list. I have submitted a patch [1] to add main element support to webkit and would appreciate your consideration. the main element is defined here: http://dvcs.w3.org/hg/html-extensions/raw-file/tip/maincontent/index.html current status unofficial draft, CFC [2] to publish as a first working draft via HTML WG ends today. Rationale and use cases: http://www.w3.org/html/wg/wiki/User:Sfaulkne/main-usecases#Introduction details of data set and data analysis conducted during development of feature: http://lists.w3.org/Archives/Public/public-html/2012Oct/0109.html There has been discussion and feedback provided on the WHATWG list [3] and IRC and at TPAC 2012 HTML WG meeting. I look forward to your comments! [1] https://bugs.webkit.org/show_bug.cgi?id=103172 [2] http://lists.w3.org/Archives/Public/public-html/2012Nov/thread.html#msg129 [3] threads start here: http://lists.w3.org/Archives/Public/public-whatwg-archive/2012Nov/thread.html#msg55 -- with regards Steve Faulkner Technical Director - TPG www.paciellogroup.com | www.HTML5accessibility.com | www.twitter.com/stevefaulkner http://www.paciellogroup.com/resources/wat-ie-about.html ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Adding main element to WebCore
As I said on the thread you link to, I don't think this element addresses any real use-cases. I think people are far too likely to misuse this for it to be useful for things like readability to use. If Apple really wants this, I won't object, but my preference would be to not implement this. On Mon, Nov 26, 2012 at 2:01 PM, Steve Faulkner faulkner.st...@gmail.comwrote: Hi all, this is my first post to the list. I have submitted a patch [1] to add main element support to webkit and would appreciate your consideration. the main element is defined here: http://dvcs.w3.org/hg/html-extensions/raw-file/tip/maincontent/index.html current status unofficial draft, CFC [2] to publish as a first working draft via HTML WG ends today. Rationale and use cases: http://www.w3.org/html/wg/wiki/User:Sfaulkne/main-usecases#Introduction details of data set and data analysis conducted during development of feature: http://lists.w3.org/Archives/Public/public-html/2012Oct/0109.html There has been discussion and feedback provided on the WHATWG list [3] and IRC and at TPAC 2012 HTML WG meeting. I look forward to your comments! [1] https://bugs.webkit.org/show_bug.cgi?id=103172 [2] http://lists.w3.org/Archives/Public/public-html/2012Nov/thread.html#msg129 [3] threads start here: http://lists.w3.org/Archives/Public/public-whatwg-archive/2012Nov/thread.html#msg55 -- with regards Steve Faulkner Technical Director - TPG www.paciellogroup.com | www.HTML5accessibility.com | www.twitter.com/stevefaulkner http://www.paciellogroup.com/resources/wat-ie-about.html ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] do you want WK1 and WK2 keywords in the TestExpectations files?
I don't have strong opinions on this, but one advantage of using the keywords and getting rid of the dedicated TestExpectations files would be to make the fallback graph actually be a tree instead of a DAG. This would simplify the rebaselining tooling considerably and allow us to make a bunch of cases work better that don't work great right now. On Tue, Nov 13, 2012 at 11:41 AM, Dirk Pranke dpra...@chromium.org wrote: Hi all, I occasionally get asked if the TestExpectations syntax supports a way to distinguish different results for WebKit1 and WebKit2 via keywords. We currently don't do this, and different ports have worked around this in slightly different ways by using dedicated wk2-specific TestExpectations files and sometimes wk1-specific TestExpectations files. However, this is a little awkward and gets worse if you also need to support different expectations for multiple different configs (e.g., mac-lion vs mac-snowleopard vs mac-mountainlion). So, it seems like WK1 and WK2 keywords might be useful. However, I don't really want to add more divergence between ports, so it would be nice to have everyone agree to use it if we were to add it. What do you all think? Would you like this feature, and would you all use it ? (Since I don't regularly switch between WK1 and WK2 I don't have a strong feeling here beyond what I've written above). -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] The --test-list option behavior in NRWT
On Thu, Nov 8, 2012 at 2:00 AM, Žan Doberšek zandober...@gmail.com wrote: I'd propose an --ordering option, with three possible values: - random, randomizes the test list - natural, orders the test list in natural order, which is the current behavior - none, keeps the order that was specified in the arguments or test list, default (not certain about the name, though) The --randomized option would be kept for backwards compatibility, but perhaps deprecated. I'll build up a patch that's based on the consensus. Sounds good to me. I don't think we need to keep the --randomized option though. Fewer flags == better. We already have too many flags. -Z On Thu, Nov 8, 2012 at 10:43 AM, Balazs Kelemen kbal...@webkit.orgwrote: On 11/08/2012 02:51 AM, Ojan Vafai wrote: On Wed, Nov 7, 2012 at 12:41 PM, Dirk Pranke dpra...@chromium.orgwrote: On Tue, Nov 6, 2012 at 11:58 PM, Žan Doberšek zandober...@gmail.com wrote: Hi WebKitties, I'd like to see in what scale the --test-list option in NRWT is used and whether anyone would object a change in how its usage behaves. Currently the test gathering takes into account any paths that were passed in through the command line and any paths listed in the file to which a possible --test-file option points[1]. These paths are then expanded if necessary (due to platform-specific tests and virtual test suites) and all the tests to which these paths apply are gathered[2]. All the gathered tests are then either sorted into a natural order or randomized (if the --randomize-order option is used)[3]. I'd like to propose a change in the behavior when using the --test-list option. When used, the tests would be gathered only from the paths listed in the file to which the option points. Also, the gathered tests would be sorted to follow the order of the paths in the test list file. For instance, consider the following two entries being placed in the test list file: c/d/e.html a/b/ The current behavior would gather all the tests to which these two paths apply and sort them in this order (on the premise that no other paths are passed through the command line arguments): a/b/1.html a/b/2.html c/d/e.html The new behavior would place the c/d/e.html test at the top of the list but sort the other two tests in order, like this: c/d/e.html a/b/1.html a/b/2.html The idea behind this is to be able to specify the exact order in which the tests should be run. I'm currently playing around with a tool that randomly chooses a certain number of tests, runs them and bisects down the first regression that occurs (if any) to the smallest possible ordered list of tests that reproduces that regression. The proposed change makes this a simple task from the test listing perspective and I'd argue that exact test ordering is the main point of an option such as --test-list. Currently I'm using an untested workaround of adding a new option that specifies the tests should be reordered back to the original order that the test list file provided, but I'd like to move on with implementing the change in behavior if everyone feels it's OK and won't tamper with his/her workflow. -Z [1] http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder.py#L46 [2] http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py#L576 [3] http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py#L309 Hi, I have used the option from time to time, and I have also wanted from time to time the ability to run tests in a specific order. This change would be fine by me. If people want to ensure that tests run in the old order, they can always sort the test list. I have used this in the past to identify which test caused test flakiness (e.g. I have 100 tests and I know the last test depends on one of the other 99 to run first in order to pass). The problem with this last sentence is that it's hard to know what order run-webkit-tests would choose, so it's hard to replicate it. It would make that use-case a little harder if we made this change. Otherwise, I don't care either way. Exactly for the reason Ojan mentioned I think the best would be to add an option for defining the desired ordering behavior. Paths on command line and in --test-list should imply to use the ordering as it is passed but it would be possible to force the current method. I hope it would not add too much complexity. -kbalazs ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] moving focus when clicking on scrollbars
On Thu, Nov 1, 2012 at 10:13 AM, Elliott Sprehn espr...@chromium.orgwrote: On Thu, Nov 1, 2012 at 4:42 AM, Maciej Stachowiak m...@apple.com wrote: I like the idea of being more like native control behavior. I agree. I'll update the patch to match Gecko's mostly native, but web friendly behavior. Sounds like we all agree here. The separate question is whether we should avoid moving focus even when you click on the scrollbar of a focusable element. That fully matches native, but doesn't match any existing browser. Maciej, would you prefer that? I'm on the fence and could easily be swayed either direction. In the meantime, Elliott's patch matching the Gecko behavior seems uncontroversial. I'll r+ once it's ready for review. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] moving focus when clicking on scrollbars
On Thu, Nov 1, 2012 at 12:32 PM, Peter Kasting pkast...@chromium.orgwrote: It seems worth noting over here that on the whatwg discussion for this, native really means Mac and Ubuntu. Notably it's not clear whether this matches Windows, which is 90+% of the userbase for Chrome. I am a little nervous making blanket statements like this is native behavior when we're not sure whether it is for such large user groups. I'm not sure how to test this on Windows, though. We know Windows doesn't move focus when you click on the scrollbar of the top-level window (e.g. if something inside the window has focus, it doesn't clear focus when you click on the scrollbar), right? We just don't know of a way of testing nested, native scrollbars on Windows. So, I still think it's accurate to call this the Windows native scrollbar behavior. FWIW, see the related http://crbug.com/6759http://code.google.com/p/chromium/issues/detail?id=6759 . Gecko's behavior makes me a little less worried than the never move focus behavior in the absence of data to answer the above question. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] moving focus when clicking on scrollbars
I'd like to r+ https://bugs.webkit.org/show_bug.cgi?id=96335, but wanted to give a heads up in case anyone wants to object. Every native platform that has scrollbars does *not* move focus when you click on them. Every browser engine, except Gecko, moves focus when you click on scrollbars *unless* you're clicking on the viewport scrollbar (e.g. clicking on the scrollbar of an scrollable div that fills the viewport will move focus). Gecko does not move focus when you click on any scrollbar unless you are clicking on the scrollbar of a form control (e.g. textarea) scrollbar. I'd like to change our behavior to either match Gecko or go fully native and never move focus when clicking on scrollbars. The latter sounds better to me, but either solution would satisfy me. Any strong opinions/objections? We've already discussed this on whatwg and the feedback has been that this is up to browser vendors to match the platform conventions: http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-October/037676.html . Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] moving focus when clicking on scrollbars
On Wed, Oct 31, 2012 at 2:29 PM, Peter Kasting pkast...@chromium.orgwrote: On Wed, Oct 31, 2012 at 1:32 PM, Ojan Vafai o...@chromium.org wrote: Every native platform that has scrollbars does *not* move focus when you click on them. Every browser engine, except Gecko, moves focus when you click on scrollbars *unless* you're clicking on the viewport scrollbar (e.g. clicking on the scrollbar of an scrollable div that fills the viewport will move focus). Gecko does not move focus when you click on any scrollbar unless you are clicking on the scrollbar of a form control (e.g. textarea) scrollbar. I'd like to change our behavior to either match Gecko or go fully native and never move focus when clicking on scrollbars. The latter sounds better to me, but either solution would satisfy me. Is there rationale for Gecko's behavior? It sounds a bit strange. Not that I know of. I haven't talked to anyone at Gecko about it though. In theory, I could conceive of the web depending on this. My preference would be to make all scrollbars not move focus and see if there is a web compat dependency since that solution is simpler and more consistent. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] moving focus when clicking on scrollbars
On Wed, Oct 31, 2012 at 3:14 PM, Peter Kasting pkast...@chromium.orgwrote: On Wed, Oct 31, 2012 at 2:40 PM, Ojan Vafai o...@chromium.org wrote: On Wed, Oct 31, 2012 at 2:29 PM, Peter Kasting pkast...@chromium.orgwrote: Is there rationale for Gecko's behavior? It sounds a bit strange. Not that I know of. I haven't talked to anyone at Gecko about it though. Might be nice to try and find someone appropriate there to ping. Surprised the topic didn't come up as path of the whatwg discussions you mentioned (since it's usually good to understand why the world is the way it is as a starting point). roc clarified that the Mozilla behavior is to move focus if the element is focusable. I'm OK with changing our behavior to match Gecko since that's a strict improvement in my view and it's arguable whether we should or shouldn't move focus when you click in the scrollbar of a mouse-focusable element. To be clear, the only change from our current behavior would be that when you click on a scrollbar of an element that is not mouse-focusable, we wouldn't move focus. This seems clearly superior to our current behavior and matches what we do for viewport scrollbars. Whether we should extend this to scrollbars of mouse-focusable elements can be a separate discussion. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] WebKit DOM 3 Events
FWIW, here's an example of something from the spec that I think is stable enough to implement without further waiting: https://bugs.webkit.org/show_bug.cgi?id=100785. On Wed, Oct 24, 2012 at 5:10 PM, Ojan Vafai o...@chromium.org wrote: On Thu, Oct 18, 2012 at 7:00 PM, Wez w...@chromium.org wrote: Hello WebKitters, The DOM 3 Events spec seems to have reached a reasonably stable state. Is there interest in updating WebKit to match the spec, e.g. in areas like keyboard input, where WebKit currently implements an older draft? In general, yes, I think we should implement this. In an ideal world, I'd like to have a bit more confidence that the key/char properties won't be changing. If anyone working on WebKit cares/knows about this stuff, it'd be great if you could chime in on the relevant threads. Specifically, I'd like to see some rough resolution to the following threads: http://lists.w3.org/Archives/Public/www-dom/2012OctDec/0010.html http://lists.w3.org/Archives/Public/public-webapps/2012JulSep/0713.html http://lists.w3.org/Archives/Public/www-dom/2012JulSep/0103.html http://lists.w3.org/Archives/Public/www-dom/2012OctDec/0030.html ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Adding high resolution platform timestamp to DOM events
I don't understand. The www-dom discussion ends with a clear consensus to use a new property name. There were no objections to systemTime. The public-web-perf discussion didn't have an objections and just wanted to wait until the V2 spec: http://lists.w3.org/Archives/Public/public-web-perf/2012Oct/0046.html. In what way does this not meet the bar for being checked in behind a flag? Having an editor's draft spec has not historically (even recently) been a requirement for checking things into WebKit behind a flag. On Thu, Oct 25, 2012 at 10:12 AM, Robert Flack fla...@chromium.org wrote: FWIW, I don't plan to have it enabled by default on any platforms until we're sure what we want. The discussion with Anne about reusing timestamp was before she found out that timestamp calls for a 1970 based time by spec (http://lists.w3.org/Archives/Public/www-dom/2012OctDec/0022.html). I think we need a new property for this and systemTime has some consensus (as per the post from Anne), but I will of course not expose it to the public until we've reached a consensus and a proposed standard. On Thu, Oct 25, 2012 at 12:59 PM, Ryosuke Niwa rn...@webkit.org wrote: As Elliott pointed out, this property doesn't seem to be on any working draft or editor's draft yet. And it doesn't seem like either thread on www-dom or www-perf reached a consensus. I'd appreciate if you waited until either thread reached a rough consensus about the feature. Namely, www-dom thread discussion leads me to believe that some people think we can simply modify timeStamp IDL attribute instead of adding new one. - Ryosuke On Wed, Oct 24, 2012 at 6:17 PM, Robert Flack fla...@chromium.orgwrote: Hi webkit-dev, I would like to add platform timestamps to DOM events as the systemTime property. I have a patch implementing the feature: https://bugs.webkit.org/show_bug.cgi?id=94987. This will let us know the time at which the system received an event to be able to accurately handle it, whereas the timestamp property gives the time the DOM event was created in an inaccurate milliseconds since 1970 form. This has been discussed on www-dom ( http://lists.w3.org/Archives/Public/www-dom/2012OctDec/0028.html) and www-perf ( http://lists.w3.org/Archives/Public/public-web-perf/2012Oct/0046.html) and use cases for this have been discussed ( http://lists.w3.org/Archives/Public/www-dom/2012AprJun/0092.html). The platform timestamp comes in as a monotonic timestamp. Since the DOM Event spec requires that timeStamp() be a 1970-epoch based timestamp it is not sufficient for a high resolution precise time delta on event delivery. Instead, we add a systemTime property which uses the Performance API spec ( http://dvcs.w3.org/hg/webperf/raw-file/tip/specs/HighResolutionTime/Overview.html) for high res timestamps (time since document load timestamp to avoid user fingerprinting) and provide the platform's high resolution timestamp to ECMAScript. Let me know if you have any suggestions. I look forward to everyone's feedback, cheers! - Rob ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] FYI: change to fake mouse events and hover updating during scroll
When you do a scroll, we fire fake mouse events and update hover state. We currently try to throttle those events, but do very little throttling in practice. The patch below makes it so that we only fire a single set of fake mouse events and only update the hover state once during a scroll. This behavior matches Firefox and generally feels like a better user-experience to me. In addition, this patch makes it so that we dynamically modify the throttling rate (instead of hard-coded values) to workaround slow mouse event handlers that make scrolling slow. In my local tests, this was a clear win and the new code is only marginally more complicated than the old code. https://bugs.webkit.org/show_bug.cgi?id=99940 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] add NeedsRebaseline keyword to TestExpectations as a way to hande updating pixel tests?
TL;DR: We should add a NeedsRebaseline keyword to TestExpectations and add garden-o-matic tooling for it for the cases where someone commits a change/test that they know will need new results for different ports (e.g. any patch that changes the rendering of pixel tests). A common pattern that I see across ports is that someone will add something like the following in a patch that changes the results of a pixel test: // Needs rebaseline after r23456 webkit.org/b/12345 path/to/test.html [ Failure ] This has a couple problems: -Often the correct expectation is something like [ Missing Failure ImageOnlyFailure ]. So, even though the test is listed, the bot turns red. -The tooling can't give you a list of all the tests that are expected to only need a rebaseline. -Related to the above, people often forget about these lines and don't do the rebaseline. We should add [ NeedsRebasline ], which is equivalent to [ Missing Failure ImageOnlyFailure ]. I'm thinking it should not include Timeout/Crash since those would need a solution other than a rebaseline (e.g. something is wrong with the test or patch). In garden-o-matic, we can make a tab specifically for tests that need rebaseline and give some indication whether the original patch that line was added in has run on all the relevant bots. This way the people keeping the tree green can also make sure that NeedsRebaseline lines don't get forgotten. If it continues to be a problem we could even setup and automated nag bot to email people who leave in NeedsRebaseline lines for more than a week. In the long-run, we should make it so you can grab the new results of the EWS bots and don't need to add lines to TestExpectations at all. In the short-term though, this is a way we can handle pixel tests without making the tree red all the time. As a side note, we should also get rid of Missing as a valid expectation. A test should either be NeedsRebaseline or have an expected result. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] PSA: Chromium Lion layout test bots are now 10.7.5
bcc: chromium-dev If you are still running 10.7.4, you'll start getting pixel layout test failures due to different anti-aliasing. See http://trac.webkit.org/changeset/130233 for the list of tests that would fail. FWIW, the build.webkit.org Chromium Mac bot is 10.6. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Improving DOM Exception console messages.
This isn't spec'ed anywhere. I agree it would be ideal to get a spec for
this, but I don't think we need to block on that in this instance. This is
an area where browsers are completely incompatible already. I don't see
much benefit from blocking on creating a specification to make this
situation better for web developers. It's actually not that big of a deal
if the error messages from different browsers are different.
On Mon, Oct 1, 2012 at 4:56 PM, Ojan Vafai o...@google.com wrote:
This isn't spec'ed anywhere. I agree it would be ideal to get a spec for
this, but I don't think we need to block on that in this instance. This is
an area where browsers are completely incompatible already. I don't see
much benefit from blocking on creating a specification to make this
situation better for web developers. It's actually not that big of a deal
if the error messages from different browsers are different.
On Mon, Oct 1, 2012 at 10:52 AM, Ryosuke Niwa rn...@webkit.org wrote:
Where is this spec'ed? If we're exposing this to the Web, we definitely a
spec. for this BEFORE implementing it in WebKit.
On Mon, Oct 1, 2012 at 2:57 AM, Mike West mk...@chromium.org wrote:
I had a brief conversation about this with Adam and Maciej on IRC
about this, and Ojan chimed in on the bug[1]. Since I'm in the wrong
time-zone for discussion (and I'd like a wider audience anyway), I'm
pulling this back to the list.
1. Maciej raised the concern that we might be exposing sensitive
information to JavaScript by making the exception's message more
detailed. For example, if if a resource allowed by CSP redirects to a
resource disallowed by CSP, we shouldn't give JavaScript access to the
latter URL.
2. Adam suggested storing the context on the exception object, using
it for rendering in the console but not exposing it to JavaScript. The
current patch does this for JSC[2].
3. On the bug, Ojan noted that many apps on the web send stack traces
back to the server for analysis, and exposing context only to the
Inspector would deprive these apps of context.
I'd like to ensure that we end up with a solution that WebKit
developers will actually use.
One solution would be to treat SECURITY_ERR differently than other DOM
exceptions. For instance, SYNTAX_ERR is thrown in a variety of
contexts where security seems to be unaffected ([3] for instance); for
that error, I don't think there's any concern with providing the same
string to JavaScript and to the Inspector. For security errors
involving redirects and other information that JavaScript shouldn't
have access to, we could go the more complicated route of providing
one string to JavaScript, and the other to the Inspector.
Concretely, this might involve adding two properties to ExceptionBase:
'context' and 'totallySektitContext' (or similar... :) ). The first
would always be appended to the message generated in ExceptionBase's
constructor, the latter only when the error is reported to the
console. At each setDOMException callsite, we'd have to provide some
mechanism for setting one or both strings. This might involve turning
ExceptionCode enum into a struct containing the code and slots for two
strings.
Does that seem like a tenable solution?
-Mike
[1]: https://bugs.webkit.org/show_bug.cgi?id=97654
[2]: https://bugs.webkit.org/attachment.cgi?id=166300action=prettypatch
[3]: https://bugs.webkit.org/show_bug.cgi?id=97984
On Wed, Sep 26, 2012 at 5:12 PM, Mike West mk...@chromium.org wrote:
Hello, webkit-dev!
TL;DR: I'd like to improve the console messages displayed for DOM
exceptions. I'm hoping that no one thinks this is a miserable,
miserable idea.
At the moment, the following code generates an exception with a
message reading SECURITY_ERR: DOM Exception 18.
8--8--8--
!doctype html
html
head
titleBug 152212/title
meta http-equiv=X-WebKit-CSP content=connect-src 'none'
/head
body
script
var xhr = new XMLHttpRequest();
var url = '
https://api.twitter.com/1/trends/daily.json?exclude=hashtags';
xhr.open('GET', url, true);
xhr.send();
/script
/body
/html
8--8--8--
In this case, the exception is generated because Content Security
Policy blocks the XMLHttpRequest, but it's tough to tell, because the
exact same error crops up for 31 different exception cases[1]. It
might be CSP, it might be a disallowed HTTP method, who knows. Because
of the variety, searching for the exception message is unhelpful. In
this case, I end up on StackOverflow, reading about cookies and
jQuery[2], which is unlikely to be of service.
This isn't a great experience, and while SECURITY_ERR is particularly
widespread, the scenario is similar for the other DOM exceptions
WebKit throws. Each ends up in ExceptionBase, where a message is
assembled from the exception type and code, and the developer is left
more than
Re: [webkit-dev] Should we close the tree? (was: Re: the new TestExpectations syntax is landing soon)
Fixing the problem will likely take less time at this point than rolling the patch out as there are a number of dependent patches. The fix should be in soon. On Thu, Sep 20, 2012 at 11:10 AM, Geoffrey Garen gga...@apple.com wrote: I'd prefer to see the patch rolled out. Geoff On Sep 20, 2012, at 11:07 AM, Alexey Proskuryakov a...@webkit.org wrote: Now tracked as https://bugs.webkit.org/show_bug.cgi?id=97182. I think that we should close the tree if resolving this takes any significant time. Not being able to see how exactly tests are failing on other platforms is unacceptable. - WBR, Alexey Proskuryakov 20.09.2012, в 3:54, Osztrogonac Csaba o...@inf.u-szeged.hu написал(а): Unfortunately r129047 broke the results.html, see https://bugs.webkit.org/show_bug.cgi?id=96845#c9 for details. Dirk Pranke írta: These changes are now starting to land ... as of r129047, TEXT, IMAGE+TEXT, and AUDIO are no longer legal keywords in the TestExpectations syntax ... you should use FAIL instead. I will be landing the support for the new syntax as quickly as I can to minimize the transition period. Apologies for the inconvenience. -- Dirk On Wed, Sep 12, 2012 at 4:29 PM, Dirk Pranke dpra...@chromium.org wrote: Hi all, The new format of the much-debated TestExpectations syntax will be landing soon (hopefully in the next couple days). For those of who have forgotten / repressed the earlier debates, the new syntax looks something like: webkit.org/b/12345 [ Mac Vista] fast/html/keygen.html [ ImageOnlyFailure ] Andis documented in full at https://trac.webkit.org/wiki/TestExpectations#NewSyntaxNotquiteyetlanded . ( The [ and ] characters are delimiters, not EBNF optional markers, although those sections are in fact optional :) ). Note that the new syntax means that Skipped files are a syntactic subset of TestExpectations files, and I plan to convert all of the Skipped files to TestExpectations files via copy and paste shortly after the new syntax is landed, and then drop support for Skipped files (I will update ORWT to use the new files and treat any entry as a Skip). The plan for landing these changes is: 1) Add support for parsing the new lines and converting them back into the old format (internally) so that both syntaxes are supported 2) Convert all the existing files over 3) Make sure things aren't broken :) 4) Drop support for the old syntax I plan for this to all happen quickly, in less than a day. This means that if you have patches posted that modify those files they may become stale and need to be updated. Changes from the old syntax: 1. We use URLs (a specific whitelisted set; let me know if you want to add to it) instead of BUGWK12345 etc. 2. We use bug(dpranke) instead of BUGDPRANKE 3. We use CamelCase instead of SHOUTING 4. We use Failure to represent what used to be TEXT, IMAGE+TEXT, and AUDIO - these failures will be indistinguishable in the new world, meaning that you can't distinguish between text only and both image and text. Since only Chromium runs pixel tests by default, this shouldn't be a big deal. 5. We use ImageOnlyFailure to represent what used to be IMAGE 6. We use [ and ] for delimiters instead of : and = 7. We use # instead of // as a comment 7. WontFix will now imply Skip, i.e., tests marked WontFix will automatically be Skipped 8. WontFix and Skip will not require (or even allow) any other expectations, i.e., you can't say [ WontFix Crash ]. If you want to indicate that the test will crash if you actually run it, use a comment. 9. WontFix, Skip, Slow, and Rebaseline all move from the left hand side to the right. The only keywords on the left restrict which configurations the lines apply to. I will send out follow-up emails as this stuff lands. Please let me know if you have any questions. Thanks! -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Bug numbers in TestExpectations
In practice, with the chromium TestExpectations, I've found that it forces people to document the history of why a test was added and gives a forum for discussing fixes (e.g. for flaky tests). It's hard to do this with a single comment. It has been a net positive in my opinion. Take the following example (one of many) from the platform/mac/Skipped file: # --- Media --- media/controls-styling.html media/media-document-audio-repaint.html media/video-zoom-controls.html That doesn't tell you anything about why those lines were put there, when they started failing, etc. While it's true that a bug doesn't force you to say anything useful either, in my experience, people put useful descriptions in the bugs for these lines. On Thu, Sep 20, 2012 at 2:28 PM, Ryosuke Niwa rn...@webkit.org wrote: I support making bug URL or Bug(~) optional. On Thu, Sep 20, 2012 at 10:34 AM, Alexey Proskuryakov a...@webkit.orgwrote: I've been repeatedly finding it that bug numbers in TestExpectations don't lead to bugs that are helpful in the context of the specific test. Often the bug doesn't have any information beyond what's already in the expectation (that the test is skipped of failing, without a posted diff or any other useful detail). Other times, the linked bug is one that the person was working on at the time of changing expectations, so the bug link is more like see related bug , and try to figure out how exactly it's related, not this failure is tracked by . Free form comments worked great for both cases in Skipped files. It appears that the rigid format that requires putting a bug URL is causing more harm than good in practice. I suggest making the URL optional. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Removing the prefix from webkitPostMessage
On Thu, Sep 13, 2012 at 12:06 PM, Dirk Pranke dpra...@chromium.org wrote: On Wed, Sep 12, 2012 at 11:00 PM, Maciej Stachowiak m...@apple.com wrote: On Sep 12, 2012, at 10:36 PM, Ojan Vafai o...@chromium.org wrote: On Wed, Sep 12, 2012 at 6:40 PM, Maciej Stachowiak m...@apple.com wrote: - Is this approach substantially less time and effort than adding a histogram-style metric? I expect you have added a histogram to Chrome at some point, and so can comment on the relative difficulty and time to produce an answer. (BTW we have the capability to do this type of thing in Safari as well, and it is what I ask Apple engineers to do when they want to remove a feature, even a purely Mac-specific one.) FWIW, histograms can only tell you a percentile. We never report back URLs due to privacy concerns. So, it's somewhat underpowered compared to experimentally removing a feature and seeing what sites break. I'm not saying that's not a useful signal, just clarifying what data is possible for Chromium to gather in the wild. That's about what the equivalent Safari mechanism does too. What it can tell you is stuff like feature x is used very little by sites users actually visit without risk of breakage. But if there is significant use, it won't tell you what sites it is on or whether it's critical. Technically we can build histograms that either give you a percentage of sites or a percentage of page views without compromising privacy overly. The combination of the two is a pretty good approximation of importance. It would be interesting to build a generic mechanism for monitoring prefixed APIs ... I wonder how hard that would be. FWIW, Tab has adding roughly this for prefixed CSS properties. Not sure what the status of that is, but the code has been committed AFAIK. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Removing the prefix from webkitPostMessage
On Wed, Sep 12, 2012 at 6:40 PM, Maciej Stachowiak m...@apple.com wrote: - Is this approach substantially less time and effort than adding a histogram-style metric? I expect you have added a histogram to Chrome at some point, and so can comment on the relative difficulty and time to produce an answer. (BTW we have the capability to do this type of thing in Safari as well, and it is what I ask Apple engineers to do when they want to remove a feature, even a purely Mac-specific one.) FWIW, histograms can only tell you a percentile. We never report back URLs due to privacy concerns. So, it's somewhat underpowered compared to experimentally removing a feature and seeing what sites break. I'm not saying that's not a useful signal, just clarifying what data is possible for Chromium to gather in the wild. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Initializing String and AtomicString with literals
On Fri, Aug 24, 2012 at 11:19 PM, Benjamin Poulain benja...@webkit.orgwrote: On Fri, Aug 24, 2012 at 10:18 PM, Adam Barth aba...@webkit.org wrote: [[[ The difference between the version is if the length of the string is included or not. Having the size given in the constructor makes the constructor faster. Having the size also makes the code bigger, which is a problem when the code is executed infrequently. ]]] That description isn't 100% clear to me, but my reading of it is that the ConstructFromLiteral version has the length of the string embedded at the call site and therefore makes the code faster by increases the code size. Initially I thought avoiding strlen() would always be a win. In practice, I have learned the hard way that code growth can hurt us a lot on ARM so I wanted to provide a no-compromise option and added ASCIILiteral. If you use ASCIILiteral, the code does not grow a single byte. You can safely use it in any place where the default constructor was used and it always make the code faster. If you are in a place where the performance matter, or if the string is long and will not be used anytime soon, ConstructFromLiteral will give you better performance. So, is the rule something like Use ASCIILiteral unless you can show improvement on a benchmark by using ConstructFromLiteral. Could you add some simple explanation like that to the wiki page? As someone who doesn't understand the implementation details, what's on the page right now doesn't give me a clear enough rule of when to use which. Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Subpixel layout - requesting help for big rebaseline
On Fri, Aug 24, 2012 at 4:42 AM, Žan Doberšek zandober...@gmail.com wrote: On Fri, Aug 24, 2012 at 4:35 AM, Dominik Röttsches dominik.rottsc...@intel.com wrote: Hi Dirk, On 08/22/2012 10:49 PM, Dirk Pranke wrote: The Chromium canaries now exit after 5000 failures or 1000 crashes/timeouts. Can the failure limit be increased to 1 for example? Levi/Emil were saying the rebaseline touches about 8000 cases. Otherwise we'd have to go through a more complicated process like Zan explains. Despite the testing exiting after a certain number of failures, the newly-registered failures would still be possible to rebaseline. So technically you could do three or four rebaseline cycles and get the bots back into a normal state. Both of these are fine solutions. I don't think we need to do the more complicated approach. As long as the gardener on duty is informed and ready. Again, it's a huge simplification that Emil and Levi have already audited the new results since the gardener can just bulk rebaseline them. Dominik __**_ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/**mailman/listinfo/webkit-devhttp://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Subpixel layout - requesting help for big rebaseline
IIRC, the chromium bots no longer exit early due to a large number of failures (right Dirk?), only due to a large number of crashes/timeouts. So, it should be possible to commit this, wait for the bots to cycle, do all the rebaselines at once and commit that. The only delay is that we'd have to wait for the slowest of the bots to cycle. Given that Emil/Levi have already verified the correctness of the new results, the gardener can just hit the rebaseline all button in garden-o-matic, so it shouldn't be too arduous as long as there aren't a bunch of commits at the same time that also break a bunch of tests. In short, I don't think committing this is too big of a deal if you are willing to coordinate with the Chromium gardener to find a good time. The PST gardener today and tomorrow is Ken Russell. If you're in a non-PST time zone, the gardener though Monday is Dominic Cooney. My intuition is that early in the morning or late at night would be best since 1-6pm PST is when the majority of WebKit commits go in, causing other failures that might be confused by this patch, but that's really up to Ken/Dominic and you to figure out what's best. On Wed, Aug 22, 2012 at 6:02 AM, Dominik Röttsches dominik.rottsc...@intel.com wrote: Hi, As another alternative, could we gather all the Chromium bot admins and ask them to temporarily take the bot offline in non-peak hours, create rebaselines per bot with the patch applied, collect those as binary-compatible diffs in one place, and manually land those collected baselines, in short succession together with the patch? Dominik __**_ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/**mailman/listinfo/webkit-devhttp://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] A simpler proposal for handling failing tests WAS: A proposal for handling failing layout tests and TestExpectations
On Mon, Aug 20, 2012 at 6:03 PM, Maciej Stachowiak m...@apple.com wrote: Here's how I imagine the workflow when a sheriff or just innocent bystander notices a deterministically failing test. Follow this two-step algorithm: 1) Are you confident that the new result is an improvement or no worse? If so, then simply update -expected.txt. 2) Otherwise, copy the old result to -whatever-we-call-the-unexpected-pass-result.txt, and check in the new result as -whatever-we-call-the-expected-failure-result.txt. I think we should do this. I don't care much about the naming. This replaces all other approaches to marking expected failures, including the Skipped list, overwriting -expected even you know the result is a regression, marking the test in TestExpectations as Skip, Wontfix, Image, Text, or Text+Image, or any of the other legacy techniques for marking an expected failure reult. Don't forget suffixing the test with -disabled! We have 109 such tests at the moment according to http://code.google.com/searchframe#search/exact_package=chromiumq=file:third_party/WebKit/LayoutTests/.*%5C-disabled$type=cs. I think we should also get rid of this. If we need a way to disable a test across ports (e.g. because it crashes in cross-platform code), we should make a Skipped/TestExpectations file in LayoutTest/platform instead of renaming the test file. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] PSA: removing CSS3_FLEXBOX define
I'm not sure. A quick search through the codebase shows a bunch of ENABLE_CSS_FLEXBOX=1. So, it looks like most, if not all ports have it enabled on trunk. On Sat, Aug 18, 2012 at 2:53 AM, Eric Seidel e...@webkit.org wrote: Do we know what ports have CSS3_FLEXBOX enabled? On Fri, Aug 17, 2012 at 6:47 PM, Ojan Vafai o...@chromium.org wrote: Next week we plan to remove the CSS3_FLEXBOX define again. We had added it back in because the spec was about to change a lot (mostly renaming). At this point the spec is stable and has been approved to go to CR. Also, our implementation has been fuzz-tested for crashes/memory errors. I'm fairly confident in the code and API stability at this point. Is there any port that doesn't want this enabled? If not, we'll remove the define. Sometime in the next couple months we also plan to check our compatibility with other implementations (if there are any shipping by that time) and remove the vendor prefix. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] A simpler proposal for handling failing tests WAS: A proposal for handling failing layout tests and TestExpectations
Asserting a test case is 100% correct is nearly impossible for a large percentage of tests. The main advantage it gives us is the ability to have -expected mean unsure. Lets instead only add -failing (i.e. no -passing). Leaving -expected to mean roughly what it does today to Chromium folk (roughly, as best we can tell this test is passing). -failing means it's *probably* an incorrect result but needs an expert to look at it to either mark it correct (i.e. rename it to -expected) or figure out how the root cause of the bug. This actually matches exactly what Chromium gardeners do today, except instead of putting a line in TestExpectations/Skipped to look at later, they checkin the -failing file to look at later, which has all the advantages Dirk listed in the other thread. Like Dirk's proposal, having both a -failing and a -expected in the same directory for the same test will be disallowed by the tooling. The reason I like this is that it's more use-case driven. -failing is a clear todo list for anyone wanting to fix layout tests. On Fri, Aug 17, 2012 at 11:52 AM, Dirk Pranke dpra...@chromium.org wrote: On Fri, Aug 17, 2012 at 11:29 AM, Ryosuke Niwa rn...@webkit.org wrote: On Fri, Aug 17, 2012 at 11:06 AM, Dirk Pranke dpra...@chromium.org wrote: On the other hand, the pixel test output that's correct to one expert may not be correct to another expert. For example, I might think that one editing test's output is correct because it shows the feature we're testing in the test is working properly. But Stephen might realizes that this -expected.png contains off-by-one Skia bug. So categorizing -correct.png and -failure.png may require multiple experts looking at the output, which may or may not be practical. Perhaps. Obviously (a) there's a limit to what you can do here, and (b) a test that requires multiple experts to verify its correctness is, IMO, a bad test :). With that argument, almost all pixel tests are bad tests because pixel tests in editing, for example, involve editing, rendering, and graphics code. If in order to tell a pixel test is correct you need to be aware of how all of that stuff works, then, yes, it's a bad test. It can fail too many different ways, and is testing too many different bits of information. As Filip might suggest, it would be nice if we could split such tests up. That said, I will freely grant that in many cases we can't easily do better given the way things are currently structured, and splitting up such tests would be an enormous amount of work. If the pixel test is testing whether a rectangle is actually green or actually red, such a test is fine, doesn't need much subject matter expertise, and it is hard to imagine how you'd test such a thing some other way. I don't think any single person can comprehend the entire stack to tell with a 100% confidence that the test result is exactly and precisely correct. Sure. Such a high bar should be avoided. I think we should just check-in whatever result we're currently seeing as -expected.png because we wouldn't at least have any ambiguity in the process then. We just check in whatever we're currently seeing and file a bug if we see a problem with the new result and possibly rollout the patch after talking with the author/reviewer. This is basically saying we should just follow the existing non-Chromium process, right? Yes. In addition, doing so will significantly reduce the complexity of the current process. This would seem to bring us back to step 1: it doesn't address the problem that I identified with the existing non-Chromium process, namely that a non-expert can't tell by looking at the checkout what tests are believed to be passing or not. What is the use case of this? I've been working on WebKit for more than 3 years, and I've never had to think about whether a test for an area outside of my expertise has the correct output or not other than when I was gardening. And having -correct / -failing wouldn't have helped me knowing what the correct output when I was gardening anyway because the new output may as well as be new -correct or -failing result. I've done this frequently when gardening, when simply trying to learn how a given chunk of code works and how a given chunk of tests work (or don't work), and when trying to get a sense of how well our product is or isn't passing tests. Perhaps this is the case because I tend to more work on infrastructure and testing, and look at stuff shallowly across the whole tree rather than in depth in particular areas as you do. I don't think searching bugzilla (as it is currently used) is a workable alternative. Why not? Bugzilla is the tool we use to triage and track bugs. I don't see a need for an alternative method to keep track of bugs. The way we currently use bugzilla, it is difficult if not impossible to find a
Re: [webkit-dev] A simpler proposal for handling failing tests WAS: A proposal for handling failing layout tests and TestExpectations
+1 On Fri, Aug 17, 2012 at 5:36 PM, Ryosuke Niwa rn...@webkit.org wrote: That's my expectation although we probably can't do that for flaky tests :( e.g. sometimes fails with image diff. On Fri, Aug 17, 2012 at 5:35 PM, Filip Pizlo fpi...@apple.com wrote: +1, contingent upon the following: are we agreeing that all current uses of TEXT, IMAGE, and so forth in TestExpectations should be in the *very near term* following Dirk's change be turned into -failing files? -Filip On Aug 17, 2012, at 5:01 PM, Ryosuke Niwa rn...@webkit.org wrote: On Fri, Aug 17, 2012 at 4:55 PM, Ojan Vafai o...@chromium.org wrote: Asserting a test case is 100% correct is nearly impossible for a large percentage of tests. The main advantage it gives us is the ability to have -expected mean unsure. Lets instead only add -failing (i.e. no -passing). Leaving -expected to mean roughly what it does today to Chromium folk (roughly, as best we can tell this test is passing). -failing means it's *probably* an incorrect result but needs an expert to look at it to either mark it correct (i.e. rename it to -expected) or figure out how the root cause of the bug. This actually matches exactly what Chromium gardeners do today, except instead of putting a line in TestExpectations/Skipped to look at later, they checkin the -failing file to look at later, which has all the advantages Dirk listed in the other thread. I'm much more comfortable with this proposal. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] PSA: removing CSS3_FLEXBOX define
Next week we plan to remove the CSS3_FLEXBOX define again. We had added it back in because the spec was about to change a lot (mostly renaming). At this point the spec is stable and has been approved to go to CR. Also, our implementation has been fuzz-tested for crashes/memory errors. I'm fairly confident in the code and API stability at this point. Is there any port that doesn't want this enabled? If not, we'll remove the define. Sometime in the next couple months we also plan to check our compatibility with other implementations (if there are any shipping by that time) and remove the vendor prefix. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] A simpler proposal for handling failing tests WAS: A proposal for handling failing layout tests and TestExpectations
That matches my understanding. You proposed modification sounds fine to me. On Fri, Aug 17, 2012 at 6:40 PM, Maciej Stachowiak m...@apple.com wrote: My understanding of the current proposal is this: 1) This applies to tests that fail deterministically, for reasons other than a crash or hang. 2) If the test has a new result that you're confident is a progression (or neither better or worse), you simply update the -expected.txt file. 3) If the test has a new result that you're confident is a regression, you delete the -expected.txt file and check in the new results as -failing.txt. 4) Ditto points 2 and 3 with respect to -expected.png, for image diffs. 5) We would stop using all other ways of marking tests that fail deterministically, including Skipped and the many things you could enter in TestExpectations. Is that correct? If so, I'd like to suggest a minor modification. In place of point 3 above, I propose the following: 3) If the test has a new result that you're confident is a regression, you rename the -expected.txt file to -previous.txt (or maybe -correct.txt or -pre-expected.txt or something) and check in the new results as -expected.txt (unless there is already a -previous.txt, in which case just update -expected and leave -previous). I propose this for the following reasons: - It maintains the longstanding approach that -expected.txt reflects what is currently *expected* to happen, not whether it is right or wrong in some abstract sense. It is an expectation, not a reference. - It still leaves a clear indication of tests that somebody needs to look at further, to determine if a regression occurred. - It leaves both old and new result in place for easy comparison by an expert. Regards, Maciej On Aug 17, 2012, at 6:06 PM, Ojan Vafai o...@chromium.org wrote: +1 On Fri, Aug 17, 2012 at 5:36 PM, Ryosuke Niwa rn...@webkit.org wrote: That's my expectation although we probably can't do that for flaky tests :( e.g. sometimes fails with image diff. On Fri, Aug 17, 2012 at 5:35 PM, Filip Pizlo fpi...@apple.com wrote: +1, contingent upon the following: are we agreeing that all current uses of TEXT, IMAGE, and so forth in TestExpectations should be in the *very near term* following Dirk's change be turned into -failing files? -Filip On Aug 17, 2012, at 5:01 PM, Ryosuke Niwa rn...@webkit.org wrote: On Fri, Aug 17, 2012 at 4:55 PM, Ojan Vafai o...@chromium.org wrote: Asserting a test case is 100% correct is nearly impossible for a large percentage of tests. The main advantage it gives us is the ability to have -expected mean unsure. Lets instead only add -failing (i.e. no -passing). Leaving -expected to mean roughly what it does today to Chromium folk (roughly, as best we can tell this test is passing). -failing means it's *probably* an incorrect result but needs an expert to look at it to either mark it correct (i.e. rename it to -expected) or figure out how the root cause of the bug. This actually matches exactly what Chromium gardeners do today, except instead of putting a line in TestExpectations/Skipped to look at later, they checkin the -failing file to look at later, which has all the advantages Dirk listed in the other thread. I'm much more comfortable with this proposal. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] A proposal for handling failing layout tests and TestExpectations
On Thu, Aug 16, 2012 at 2:32 PM, Filip Pizlo fpi...@apple.com wrote: 1) Switching to skipping flaky tests wholesale in all ports would be great, and then we could get rid of the flakiness support. Then you don't notice when a flaky tests stops being flaky. The cost of flakiness support on the project does not seem large to me and it's pretty frequent that a test will only be flaky for a few hundred runs (e.g. due to a bad checkin that gets fixed), so we can then remove it from TestExpectations and run the test as normal. 2) The WONTFIX mode in TestExpectations feels to me more like a statement that you're just trying to see if the test doesn't crash. Correct? Either way, it's confusing. WONTFIX is for tests that don't make sense to fix for the given port (e.g. dashboard-specific tests for non-Apple ports). It's a way of distinguishing tests that we're skipping because of a bug on our part vs. tests that we're skipping because the test doesn't apply to the port. 3) Your new mechanism feels like it's already covered by our existing use of -expected files. I'm not quite convinced that having -failing in addition to -expected files would be all that helpful. But there are many cases where we *know* the result is incorrect, e.g. it doesn't match a spec. Sure, there are also many cases where it's not clear what the correct behavior is. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] A proposal for handling failing layout tests and TestExpectations
I think this is the best compromise between the benefits of the historically chromium approach (i.e. add to TestExpectations) and the historically non-chromium approach (either skip the test or check in a failing result, usually the latter). The only thing Dirk's proposal changes for these ports (i.e. all but Chromium) is that an expectation file's suffix can be -passing or -failing in addition to the current -expected. It makes for a clear list of broken tests that need fixing without compromising the test suites effectiveness at detecting new regressions. I agree that we just need to try it and see how it turns out. For non-chromium ports, it will be easy to switch back if we decide it's too much overhead and Chromium will just need to come up with an alternate way of keeping track of the list of tests that are failing. On Wed, Aug 15, 2012 at 1:36 PM, Michael Saboff msab...@apple.com wrote: It seems to me that there are two issues here. One is Chromium specific about process conformity. It seems to me that should stay a Chromium issue without making the mechanism more complex for all ports. The other ports seem to make things work using the existing framework. The other broader issue is failing tests. If I understand part of Filip's concern it is a signal to noise issue. We do not want the pass / fail signal to be lost in the noise of expected failures. Failing tests should be fixed as appropriate for failing platform(s). That fixing might involve splitting off or removing a failing sub-test so that the remaining test adds value once again. Especially a pass becoming a fail edge. For me, a test failing differently provides unknown value as the noise of it being a failing test likely exceeds the benefit of the different failure mode signal. It takes a non-zero effort to filter that noise and that effort is likely better spent fixing the test. Historically, all WebKit ports other than Chromium (especially the Apple port) have taken exactly this approach. What you're arguing against is the long-standing approach WebKit has taken to testing. I don't think discussing this broader issue is useful since noone is asking the non-Chromium ports to change this aspect. This has been discussed to death on webkit-dev, with many people (mostly at Apple) being strong proponents of checking in failing expectations. This just brings the Chromium port inline with other ports without losing the benefit the Chromium approach currently has of keeping a list of failing tests that need fixing. - Michael On Aug 15, 2012, at 12:48 PM, Dirk Pranke dpra...@chromium.org wrote: I've received at least one bit of feedback off-list that it might not have been clear what problems I was trying to solve and whether the solution would add enough benefit to be worth it. Let me try to restate things, in case this helps ... The problems I'm attempting to solve are: 1) Chromium does things differently from the other WebKit ports, and this seems bad :). 2) the TestExpectations syntax is too complicated 3) When you suppress or skip failures (as chromium does), you can't easily tell when test changes behavior (starts failing differently). 4) We can't easily tell if a test's -expected output is actually believed to be correct or not, which makes figuring out whether to rebaseline or not difficult, and makes figuring out if your change that is causing a test to fail is actually introducing a new problem, just failing differently, or even potentially fixing something. I agree that it's unclear how much churn there will be and how much benefit there will be, but we've been talking about these problems for years and I think without actually trying *something* we won't know if there's a better way to solve this or not, and I personally think it's worth trying. The solution I've described is the least intrusive mechanism we can try that I've yet come up with. -- Dirk On Wed, Aug 15, 2012 at 12:22 PM, Dirk Pranke dpra...@chromium.org wrote: Hi all, As many of you know, we normally treat the -expected files as regression tests rather than correctness tests; they are intended to capture the current behavior of the tree. As such, they historically have not distinguished between a correct failure and an incorrect failure. The chromium port, however, has historically often not checked in expectations for tests that are currently failing (or even have been failing for a long time), and instead listed them in the expectations file. This was primarily motivated by us wanting to know easily all of the tests that were failing. However, this approach has its own downsides. I would like to move the project to a point where all of the ports were basically using the same workflow/model, and combine the best features of each approach [1]. To that end, I propose that we allow tests to have expectations that end in '-passing'
[webkit-dev] DRT/WTR should clear the cache at the beginning of each test?
See https://bugs.webkit.org/show_bug.cgi?id=93195. media/W3C/video/networkState/networkState_during_progress.html and media/video-poster-blocked-by-willsendrequest.html are flaky on all platforms because they behave differently if the loaded resource is cached. Every time I've taken a stab at reducing test flakiness, I've come across at least a few tests that pass when run as part of the test suite, but fail when run by themselves (or in parallel) because they accidentally expect an image or something to be in the cache. I think it would make the tests more maintainable if we cleared the cache before each test run. This is *not* before each page load though. So tests that do multiple page loads will still test cross-navigation caching behavior. While it's true that we could one-off fix each of these tests, it's usually very time consuming to figure out that caching is the problem, that's assuming anyone takes the time to look into why the test is flaky in the first place. Any objections? Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Can we remove webkit prefix from Battery Status API and from Vibration API?
On Tue, Jul 31, 2012 at 10:05 PM, Kihong Kwon vim...@gmail.com wrote: Has a test suite been published we can test against? I didn't see a test suite which is opened to everyone yet. Is this a problem for dropping prefix? It's not strictly required for dropping the prefix. But it would be good to have some confidence that our implementation is interoperable with other implementations. Can you run our tests against Firefox and see that we get the same results? Also, could you see if Firefox has any tests that we fail? You can report the results on the bugs directly so reviewers have that information. Are there any other implementations of the APIs? Besides the time-based requirement of dropping vendor prefixes when a specification goes to Candidate Recommendation status, unprefixing also implies operability with both the specification and other implementations. It'll be good to confirm that. Firefox already implements these APIs. https://wiki.mozilla.org/WebAPI https://developer.mozilla.org/en/DOM/window.navigator.battery https://developer.mozilla.org/en/DOM/window.navigator.battery And these APIs have been entered CR as you know. BR, Kihong. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] PSA: rebaseline tooling
https://trac.webkit.org/wiki/Rebaseline I've recently consolidated the various rebaseline commands and made the tool work for non-Chromium ports. I especially like webkit-patch rebaseline path/to/test/i/just/broke.html, which lets you easily rebaseline the test for all ports once the bots have cycled. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] PSA: rebaseline tooling
If someone feels like doing extra hacking to improve this, I'm happy to walk you through the code. It'd be awesome if you could expand out that section to show which revision has been run on each bot. https://bugs.webkit.org/show_bug.cgi?id=91163 On Thu, Jul 12, 2012 at 4:18 PM, Peter Kasting pkast...@google.com wrote: On Thu, Jul 12, 2012 at 4:16 PM, Dirk Pranke dpra...@chromium.org wrote: At the top of the garden-o-matic page there is a line like Latest revision processed by every bot: 122499 (trunk is at 122524). I think that does what you want? Ah, I hadn't noticed that. That sounds promising! PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Comments in the code (Was Please include function-level comments in change log entries)
On Wed, Jul 11, 2012 at 8:43 AM, John Mellor joh...@chromium.org wrote: On Wed, Jul 11, 2012 at 4:21 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Jul 11, 2012 at 6:54 AM, John Mellor joh...@chromium.org wrote: Even obvious (to some) concepts like InlineBox have subtleties, for example not all inline-level elements have inline boxes. An unambiguous class-level comment could make this clearer, for example: // An inline box represents a rectangle that occurs on a line, corresponding to // all or part of some RenderObject. It must be inline-level and its contents // must participate in its containing inline formatting context. For example a // non-replaced element with a 'display' value of 'inline' generates an inline // box, as does an anonymous inline element (text directly contained inside a // block container element, not inside an inline element). But atomic // inline-level boxes (such as replaced inline-level elements, inline-block // elements, inline-table elements, and ruby elements) are not inline boxes // since they participate in their inline formatting context as a single // opaque box; these are handled by insert class that deals with these. // http://www.w3.org/TR/2011/REC-CSS2-20110607/visuren.html#inline-boxes What's the point of adding this comment when the URL contains all the information? All we need is the URL. If anything, we should be describing the difference between the inline boxes in CSS2.1 and our implementation instead. That would be great! I agree that there's probably limited value in just copy/pasting from specs like I did. Linking to the spec something is based on and describing the differences would add a lot of value. Also, with that argument, we can start adding a WHOLE bunch of comments to WebCore like what DOM node is, and what mutation means, what content attribute is, etc... But we don't do that because we expect people to have some domain-specific knowledge. Of course, I'm not opposed to adding reference URLs as necessary since that'll be actually useful for some obscure concepts. It's also often not obvious if the WebCore object is actually intended to map to the equivalent CSS/DOM/HTML concept. Sometimes it is and sometimes it's not. Clearly Attr/Attribute could use a why-style comment for example. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...
On Thu, Jun 14, 2012 at 4:34 PM, Dirk Pranke dpra...@chromium.org wrote:
On Thu, Jun 14, 2012 at 4:22 PM, Maciej Stachowiak m...@apple.com wrote:
On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa rn...@webkit.org wrote:
On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting pkast...@chromium.org
wrote:
On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger epo...@chromium.org
wrote:
Can someone please remind me why IMAGE+TEXT even exists?
Wouldn't it be simpler to just mark a test as follows?
IMAGE : allow image failure; go red if there is a text failure
TEXT: allow text failure; go red if there is an image failure
IMAGE TEXT: allow text and/or image failure
The distinction is that IMAGE TEXT will allow image, text, or both to
fail, thus making transitions among the three generate no events.
IMAGE+TEXT says specifically that we expect both to fail and that if
one
starts passing, someone should do something. (For example, maybe
someone
checks in a partial rebaseline where they miss the image expectations.)
Not to bike-shed on anything, but I think we should rename Text and
Image to
TextOnly and ImageOnly. Every single person I know, including myself, had
never got the distinction between IMAGE TEXT and IMAGE+TEXT without
someone
explaining it to him/her .
I think IMAGE+TEXT is not a very useful distinction from TEXT either. I
checked for uses of TEXT that is not IMAGE+TEXT in the Chromium
TextExpectations, and it seems that nearly all instances fall into one of
the two following categories:
1) text-only test, so IMAGE+TEXT would not have different semantics from
TEXT (the vast majority)
2) Flaky test that may actually pass, so distinguishing what happens with
the image result is of limited utility (most of these are also text-only
tests; only a small subset even have an image result)
Thus, I think Fail and ImageOnlyFail would be more useful and
understandable
categories than {TEXT, IMAGE, TEXT+IMAGE, TEXT IMAGE}. Fail would have
the
semantic that a text failure is expected, and image result if any can
either
pass or fail.
This is perhaps true, but if it's okay I would like to treat that
feature request separately from the other syntactic changes we've been
discussing. So far the rest of the changes have not really implied any
changes to how we actually track which changes fail and how (note that
FAIL is different and we've fixed that separately from these changes
as well).
Lets have the separate bikeshed. While this is less precise, I agree that
Fail and ImageOnlyFail would capture the vast majority use-case and remove
a frequent source of confusion and error. The big downside of this approach
is that a text-only failure that also starts failing the pixel result make
genuinely indicate a new bug. I think that happens rarely enough that I'm
OK with it for the added simplicity.
A couple open questions:
-Does Fail also replace Audio? Seems reasonable to me.
-What about reftest failures where there is no text comparison? I'd be fine
with saying you can do Fail or ImageOnlyFail and they mean the same thing
here.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...
On Thu, Jun 14, 2012 at 9:00 PM, Adam Barth aba...@webkit.org wrote:
On Thu, Jun 14, 2012 at 8:56 PM, Ojan Vafai o...@chromium.org wrote:
On Thu, Jun 14, 2012 at 4:34 PM, Dirk Pranke dpra...@chromium.org
wrote:
On Thu, Jun 14, 2012 at 4:22 PM, Maciej Stachowiak m...@apple.com
wrote:
On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa rn...@webkit.org wrote:
On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting pkast...@chromium.org
wrote:
On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger epo...@chromium.org
wrote:
Can someone please remind me why IMAGE+TEXT even exists?
Wouldn't it be simpler to just mark a test as follows?
IMAGE : allow image failure; go red if there is a text failure
TEXT: allow text failure; go red if there is an image failure
IMAGE TEXT: allow text and/or image failure
The distinction is that IMAGE TEXT will allow image, text, or both to
fail, thus making transitions among the three generate no events.
IMAGE+TEXT says specifically that we expect both to fail and that if
one
starts passing, someone should do something. (For example, maybe
someone
checks in a partial rebaseline where they miss the image
expectations.)
Not to bike-shed on anything, but I think we should rename Text and
Image to
TextOnly and ImageOnly. Every single person I know, including myself,
had
never got the distinction between IMAGE TEXT and IMAGE+TEXT without
someone
explaining it to him/her .
I think IMAGE+TEXT is not a very useful distinction from TEXT either.
I
checked for uses of TEXT that is not IMAGE+TEXT in the Chromium
TextExpectations, and it seems that nearly all instances fall into one
of
the two following categories:
1) text-only test, so IMAGE+TEXT would not have different semantics
from
TEXT (the vast majority)
2) Flaky test that may actually pass, so distinguishing what happens
with
the image result is of limited utility (most of these are also
text-only
tests; only a small subset even have an image result)
Thus, I think Fail and ImageOnlyFail would be more useful and
understandable
categories than {TEXT, IMAGE, TEXT+IMAGE, TEXT IMAGE}. Fail would have
the
semantic that a text failure is expected, and image result if any can
either
pass or fail.
This is perhaps true, but if it's okay I would like to treat that
feature request separately from the other syntactic changes we've been
discussing. So far the rest of the changes have not really implied any
changes to how we actually track which changes fail and how (note that
FAIL is different and we've fixed that separately from these changes
as well).
Lets have the separate bikeshed. While this is less precise, I agree that
Fail and ImageOnlyFail would capture the vast majority use-case and
remove a
frequent source of confusion and error. The big downside of this
approach is
that a text-only failure that also starts failing the pixel result make
genuinely indicate a new bug. I think that happens rarely enough that
I'm OK
with it for the added simplicity.
A couple open questions:
-Does Fail also replace Audio? Seems reasonable to me.
Yeah, audio tests can fail only in one way.
-What about reftest failures where there is no text comparison? I'd be
fine
with saying you can do Fail or ImageOnlyFail and they mean the same thing
here.
Similarly, I'd say that we should just Fail here. Reftests can fail
only in one way.
Seems like it will be a common error to mark a reftest failure as
ImageOnlyFail and then be confused why it's not working, no?
In this view, ImageOnlyFail is a special case for pixel tests because
they're so fragile.
Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...
On Thu, Jun 14, 2012 at 9:20 PM, Maciej Stachowiak m...@apple.com wrote: On Jun 14, 2012, at 9:06 PM, Adam Barth aba...@webkit.org wrote: On Thu, Jun 14, 2012 at 9:02 PM, Ojan Vafai o...@chromium.org wrote: Seems like it will be a common error to mark a reftest failure as ImageOnlyFail and then be confused why it's not working, no? Maybe that can be solved with another name, like PixelOnlyFailure. I'm OK with trying it this way. We can always have another strikebikeshed/strike fruitful discussion if it turns out to be a frequent cause of confusion in practice. Not sure one name is any more clear than the other. PixelOnlyFailure seems fine to me since others have expressed a preference in the past for Pixel over Image. That sounds good. We could also make it an error to apply PixelOnlyFailure (or what have you) to a text-only test, a reftest, or an audio test. Error in the sense that it would be reported as a failure, with an informative diagnostic saying it does not apply. We already have a mechanism for errors like this. They are reported when you run the tests or when you run with --lint-test-files. At least on the chromium bots, this runs as a separate step that turns red when when you cause a lint failure. That way errors get noticed and addressed quickly (the lint step takes ~2 seconds to run). Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] DOM tree traversal on detached nodes
On Mon, Jun 11, 2012 at 9:32 PM, Filip Pizlo fpi...@apple.com wrote: I think that a lot of the performance penalty can be alleviated by: 1) Moving rarely executed paths into non-inlineable methods. 2) Marking the various refing methods ALWAYS_INLINE, after you've moved as much as possible out of line. 3) Using LIKELY and UNLIKELY where appropriate. We should only add these when we can point to a measurable benefit IMO (maybe that's what you meant by appropriate?). My experience is that marking codepaths LIKELY/UNLIKELY very rarely has a measurable performance improvement with modern compilers + x86 processors. I can't speak for arm since I have less direct experience optimizing for arm. The reason I suggest these things is that you're adding a lot of code in a bunch of methods, but a lot of that logic looks like it will execute infrequently. That reduces inlining opportunities. And in places where the methods are still inlined, you're adding code bloat that reduces icache locality and may blow out branch prediction caches. I'm also not sure that your benchmark is conclusive. What does the perf test have in common with things we know we care about, like say PLT? -Filip On Jun 11, 2012, at 8:45 PM, Kentaro Hara hara...@chromium.org wrote: Thanks ggaren! I filed a bug here: https://bugs.webkit.org/show_bug.cgi?id=88834 I believe you could achieve (a) (i.e., preserve all reachable nodes without help from the JavaScript garbage collector) with these semantics in the C++ DOM: = Design = ref(): ++this-refCount ++root()-refCount deref(): --this-refCount --root()-refCount Actually I've tried the approach yesterday but confirmed 25.9% performance regression, even if I have TreeShared hold a pointer to the root. Please look at the WIP patch and the performance test in https://bugs.webkit.org/show_bug.cgi?id=88834. What I've learned is that we must not insert any line to ref() and deref(), which are performance sensitive:) I am now trying another approach that hacks Node destruction. Let's discuss the implementation details in the bug. Thanks! On Tue, Jun 12, 2012 at 12:18 PM, Geoffrey Garen gga...@apple.com wrote: IMHO, (a) would be the best semantics. I agree, and I think that the specification actually does require this. The real issue here is how to fix this bug efficiently. I believe that Geoff Garen has been contemplating this and also has a proposal for how to do it. I believe you could achieve (a) (i.e., preserve all reachable nodes without help from the JavaScript garbage collector) with these semantics in the C++ DOM: = Design = ref(): ++this-refCount ++root()-refCount deref(): --this-refCount --root()-refCount if !root()-refCount: assert(!this-refCount) for each descendent of root(): delete descendent delete root() Remove 'this' from tree: assert(this-refCount) // clients must put a node into a RefPtr to remove it from a tree root()-refCount -= this-refCount for each descendent of this: this-refCount += descendent-refCount Insert 'this' into tree: root()-refCount += this-refCount for each descendent of this: this-refCount -= descendent-refCount What is root()?: If this is in a document: root() == document() else: root() == the highest link in the parentNode chain = FAQ = Cycles? No cycles are possible because the DOM API does not allow cycles in a DOM tree. Is O(n) subtree insertion and removal OK? I believe these operations are already O(n). Is average case O(log n) access to disconnected root() OK? If n is small and/or ref/deref of disconnected subnodes are rare, then yes. Otherwise, we can optimize this case by putting a direct pointer to root() in rare data. Geoff -- Kentaro Hara, Tokyo, Japan (http://haraken.info) ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?
On Tue, Jun 12, 2012 at 9:50 AM, Darin Adler da...@apple.com wrote: On Jun 11, 2012, at 7:33 PM, Mike Lawther wrote: Does having the 'fast' directory still serve a useful purpose? Not sure it does. The original intent was to put more extensive complete tests that were slow but had greater coverage outside the “fast” directory, so that you could run the majority of the most useful tests in a few seconds. Times to run tests have increased so much, despite the parallel test runner, that it’s no longer a useful distinction. A new logical organization for tests would be welcome, and it’s a big project where I am not sure we have a clear enough plan to start on it incrementally. Here are a few considerations: - Moving test files around could cause some huge churn, with some super-slow check-outs for everyone working on the project and many old patches potentially needing to be rebased. This is especially true when image results are involved. - Each time we change the tests we need to make sure we haven’t broken new tests on various platforms and that we’ve updated the skipped or test expectations files properly. If we could get more EWS bots running the tests (currently only Chromium Linux does), that would make this a lot easier. It would also improve general development on WebKit. Fewer cases of people being surprised by broken layout tests when a patch gets committed. One more item I'd add to this list: -Replace js-test-post.js with an onload handler in js-test-pre.js so we can get rid of that file entirely. - Calling the entire directory that hosts our regression test suite LayoutTests seems wrong; that name was apropos when most of the tests were for layout issues, very early in the life of the test machinery. - Imported test suites ought to be gathered together in a single directory then grouped by where they were imported from by having a directory for each source. - We want to continue the practice of keeping copies imported test suites as close to the original as possible. In some cases in the past that meant preserving incorrect imported tests with expected “failures” that weren’t really failures. Often we’d put corrected copies elsewhere in the test tree while waiting for or working to get the imported test to fixed at the source. - Keeping the tests that require the HTTP server in a separate directory still seems like it might serve a purpose, although there seems to be an extra directory level in the test hierarchy there that is not helpful. - When rearranging tests that share the test shell JavaScript files we’d likely need to update relative paths. Might also be nice to put that test shell in a more logical location. - Making JavaScript-only tests runnable without WebKit’s involvement would be a welcome feature; great for anyone working on JavaScriptCore. That means we’d want to be sure to put those into their own directories. Ideally that would eventually be coupled by a project to move the Mozilla test suite currently in JavaScriptCore into the main WebKit regression tests. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] removing js-test-post.js WAS: Can we distinguish imported tests in LayoutTests/css3 ?
On Tue, Jun 12, 2012 at 10:18 AM, Alexey Proskuryakov a...@webkit.org wrote: 12.06.2012, в 9:59, Ojan Vafai написал(а): One more item I'd add to this list: -Replace js-test-post.js with an onload handler in js-test-pre.js so we can get rid of that file entirely. I think that this is a separate discussion to test directory reorganization. Makes sense. Changed the subject. Seemed related to me in that it might simplify moving tests that include it. Also, I'd question that getting rid of some boilerplate in autogenerated code is a worthy goal, given that it will make tests more fragile. I'm sympathetic to this argument. In this case, the only way it makes the tests more fragile is that it makes them depend on us firing the onload event. I feel like so many things would break if you break onload, that it's not really making the tests more fragile in practice. Increasingly, js-test-pre.js is not being used by autogenerated code. This is something we should encourage. By standardizing on this for most dump-as-text tests, we simplify test code and simplify making sense of test failures (e.g. you don't need to understand the custom test harness in the test to figure out a failure). We should only use autogenerated script-tests for cases where we need to isolate out the pure JS code (e.g. pure JS tests or tests that need to be run both inside and outside a Worker) since they add the extra overhead of dealing with an extra file and need the extra work of running the autogeneration script. In practice, what happens right now is that people often leave out js-test-post.js and you lose a significant bit of the test harness's functionality (i.e. checking for unintentional parse errors). The benefits far outweigh the risks here IMO. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] are fuzzer tests appropriate layout tests?
See https://bugs.webkit.org/show_bug.cgi?id=87772. It's great to use a fuzzer in order to find cases where we're broken and then make reduced layout tests from those. The viewspec-parser tests are themselves just a fuzzer though. Granted, they are deterministic by avoiding using an actual random function, but I don't think throwing randomly generated bits at a parser is appropriate for layout testing. If nothing else it's very slow. These tests regularly timeout on the Chromium debug bots and occasionally timeout on the Apple Lion bots. Even on the bots where they don't timeout, they're slow. I don't it makes sense to spend 1+ minutes running these 5 tests when more targeted reductions could get the same effective coverage much faster. Am I wrong? If not, does anyone object to moving these tests over to ManualTests or just deleting them entirely? Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] can we stop using Skipped files?
On Sun, Jun 10, 2012 at 4:54 AM, Balazs Kelemen kbal...@webkit.org wrote: So the unit tests are superfluous. In particular, if I had to pick between only having unit tests or only having regression tests, I might pick unit tests. But if I already have regression tests then I'm unlikely to want to incur technical debt to build unit tests, particularly since unit tests requiring changing the infrastructure to make the code more testable, which then leads to the problems listed above. There are many code paths are used rarely. In practice, we were having regressions frequently when people modified the code. Since the codebase has been unittested, the rate of regressions has gone down considerably. The time you spend dealing with tests is considerably less than the time you spend rolling patches in an out as you encounter different edge cases that different configurations/flags hit. A quick note to unittests. I think it's easy to define a hard limit for unittests, which is that: if I want to add a feature, or some customizing option for a particular port, it should be less effort to write the unittest than to write the actual code. I heard from my colleges a few times that it's not always the case with nrwt. I can imagine that it's not trivial to setup the unittest system for a module that has not been unittested so far but I think it should rather be the job of those who are actively working on the test harness, not of those who just need some work to be done for their port. While this is a nice ideal to strive for, I don't think this ever plays out for testing on any project, e.g. it is very frequently harder to write tests for my WebCore changes than to make the change itself. Certainly anything we can do to make testing easier is better, but I don't see NRWT as more difficult to test than any other code in the WebKit project. WebKit has a policy of every change requiring tests. I don't see why tooling should be any different. It's unfortunate that NRWT started with 0 tests, so there are still (very few now!) parts that aren't tested. It's hard to test those parts if that's what your modifying. However, it's *especially* for the cases of port-specific code that need testing. Those are exactly the codepaths that break from lack of testing. Untested code is inherently harder to maintain in my experience. Most of the time, committing untested code is just implanting technical debt that someone will have to pay later. There's noone whose fulltime job is it to maintain WebKit tooling. It's a shared effort supported by everyone in the project. It's long been a part of WebKit culture as long as I've been involved in the project that sometimes you get stuck reworking code you didn't write in order to make your change because your change just barely pushes the existing messy code over the edge of too messy. It's true that it makes your simple change much harder, but it's better for the project overall, even if occasionally a patch doesn't get checked in because the associated work was more than the contributor was willing to do. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] GTK+ port's help needed to get rid of FAIL test expectation
Can you just expand out FAIL to it's equivalent longform? FAIL == TEXT IMAGE IMAGE+TEXT. I don't see a need to block the infrastructure change on this as the semantics are exactly the same. You can just find/replace every instance of FAIL. On Fri, Jun 8, 2012 at 10:12 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, In the discussion to rename FAIL to DIFF, the consensus appeared that we should get rid of it altogether in the favor of more specific test failure expectations. I've done that in http://trac.webkit.org/changeset/119892 and http://trac.webkit.org/changeset/119889 for all but GTK+ ports. However, GTK+ port's test expectation file contains hundreds of FAIL test expectations, of which I'm not comfortable replacing all by myself. Could someone from GTK+ port assist me in replacing those with more specific test expectations? Best, Ryosuke Niwa Software Engineer Google Inc. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] can we stop using Skipped files?
On Fri, Jun 8, 2012 at 12:46 AM, Osztrogonac Csaba o...@inf.u-szeged.huwrote: Hi, Dirk Pranke írta: I believe most if not all of the ports have started using either TestExpectations files or a combination of TestExpectations files (except for the Apple Win port). Can we explicitly switch to the TestExpectations files at this point and drop support for Skipped files on the other ports (and perhaps disable old-run-webkit-tests for all but apple win)? Until NRWT can't handle cascaded TestExpectations - https://bugs.webkit.org/show_**bug.cgi?id=65834https://bugs.webkit.org/show_bug.cgi?id=65834 , Qt port can't drop supporting Skipped files. We have many tests skipped in qt-5.0, qt-5.0-wk1, qt-5.0-wk2, wk2 Skipped lists. We can't migrate all of them to the only one TestExpectations. And I disagree with disabling ORWT at all. Qt port still support using ORWT locally. It is better for gardening than NRWT. NRWT regularly has problems with generating new results for a given platform dir (qt,qt-5.0,qt-5.0-wk1,...), it doesn't support the good --skipped=only option . If folks don't want to use it, just not use, but disabling for everyone by fiat isn't a friendly thing. Can you file bugs for these explaining where it does something different from ORWT? Dirk is actively working on the cascading issue, but I don't think these other issues are known. I don't see why it would make sense to keep two parallel tools for this once all the workflow bugs people have are addressed. If not, what needs to happen so that we can do that? Do we need to land more of the discussed changes to the syntax of the TestExpectations file? Anything else? It would be nice to get rid of the complexity (both in the code and in our heads) if we can. -- Dirk br, Ossy __**_ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/**mailman/listinfo.cgi/webkit-**devhttp://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] can we stop using Skipped files?
On Fri, Jun 8, 2012 at 9:25 AM, Filip Pizlo fpi...@apple.com wrote: On Jun 8, 2012, at 9:16 AM, Balazs Kelemen wrote: On 06/08/2012 05:21 PM, Filip Pizlo wrote: On Jun 8, 2012, at 4:38 AM, Balazs Kelemenkbal...@webkit.org wrote: On 06/08/2012 09:46 AM, Osztrogonac Csaba wrote: Hi, Dirk Pranke írta: I believe most if not all of the ports have started using either TestExpectations files or a combination of TestExpectations files (except for the Apple Win port). Can we explicitly switch to the TestExpectations files at this point and drop support for Skipped files on the other ports (and perhaps disable old-run-webkit-tests for all but apple win)? Until NRWT can't handle cascaded TestExpectations - https://bugs.webkit.org/show_bug.cgi?id=65834, Qt port can't drop supporting Skipped files. We have many tests skipped in qt-5.0, qt-5.0-wk1, qt-5.0-wk2, wk2 Skipped lists. We can't migrate all of them to the only one TestExpectations. And I disagree with disabling ORWT at all. Qt port still support using ORWT locally. It is better for gardening than NRWT. NRWT regularly has problems with generating new results for a given platform dir (qt,qt-5.0,qt-5.0-wk1,...), it doesn't support the good --skipped=only option . If folks don't want to use it, just not use, but disabling for everyone by fiat isn't a friendly thing. 1. These are real weaknesses of nrwt, we should fix them. If gardening is better with orwt (i doubt that is the case, but I don't do gardening regularly), we should improve nrwt, i.e. reimplement features from orwt. I applaud your enthusiasm. 2. I believe basically everybody agrees that we should drop orwt, except you Ossy. Maybe I'm wrong. So, is there anybody still want to have support for orwt? If so, why? I'm with Ossy on this. Getting rid of ORWT would be a show stopper for me. This talk of fixing bugs in NRWT is really great but until such time as those bugs are fixed, let's keep ORWT. Understandable. Let me make it clear: I don't prefer one over the other. I believe it's contra-productive that some people/bots use this, and others use that. It adds overhead to bot maintainance, it's bad for developer experience, and it blocks the evolution of the one and only tool - because some people still make efforts on fixing/improving the other instead. I think one issue that ought to be fixed with NRWT is the level of infrastructural complexity that it has, and the extent to which its algorithmic design (say, load balancing technique) is ossified through the use of entirely unnecessary object oriented pattern overhead. In short, NRWT is overengineered. As such, there will be those of us, which prefer solutions that are not overengineered, and who thus end up hacking ORWT when we need to get work done. Do you really do quick local hacks to ORWT? Or are you talking about adding actual features to ORWT? While NRWT is more code than ORWT, it's also thoroughly unittested. It's a little harder to dive into, but it's much easier to maintain IMO. The complexity in the codebase is partially because it was written before webkitpy existed and then ported to work on top of webkitpy. So there is still cleanup that could happen to make it less complex. But the specific complexity you're talking about for the parallelism was necessary to make NRWT use multiple processes, which itself was necessary because of Python multithreading bugs. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] can we stop using Skipped files?
On Fri, Jun 8, 2012 at 10:14 AM, Zoltan Herczeg zherc...@webkit.org wrote: Hi, I don't see why it would make sense to keep two parallel tools for this once all the workflow bugs people have are addressed. The reason is easy. In the past when people tried to add new features to NRWT, they were not allowed to because the feature is not useful for NRWT devs. Eventually people got tired of NRWT and use ORWT instead since its development is not as restricted. Is this changed? Can you point to a case of this? There's no policy restricting adding features to NRWT, so I'm not really sure what you're referring to. A specific example might help. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Defaulting --exit-after-n-failures to 0
Speaking of differences between NRWT and ORWT, ORWT doesn't limit to n failures by default. NRWT limits to 500 by default. It looks like all the bots pass in --exit-after-n-failures=500. Any objection to making NRWT match ORWT here? I don't think having this limit is useful/necessary for local development. We'll still default --exit-after-n-crashes-or-timeouts to 20. That seems more useful to me since you'd rarely hit this case locally and want to continue running tests. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Defaulting --exit-after-n-failures to 0
On Fri, Jun 8, 2012 at 12:08 PM, Ryosuke Niwa rn...@webkit.org wrote: On Fri, Jun 8, 2012 at 12:05 PM, Ojan Vafai o...@chromium.org wrote: We'll still default --exit-after-n-crashes-or-timeouts to 20. That seems more useful to me since you'd rarely hit this case locally and want to continue running tests. We should also increase this number. Non-chromium bots hit this limit all the time, and it's one of most frequent complaints about new-run-webkit-tests. I have no objection to upping this number. We picked 20 fairly arbitrarily. I think we picked it to match the number that the build.webkit.org bots were using at the time. That said, every non-chromium bot is passing in 20 via the commandline as best I can tell, so changing the default won't change anything there. Is it really true that non-chromium bots are hitting this frequently? Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] We should rename layoutTestController to testController
testController seems fine to me. I agree it's an improvement. On the other hand there are other tasks that have more benefit in terms of code maintenance for people wanting to spend time working in this area. A couple ideas: - Move more APIs that only depend WebCore code to internals. Reduces code duplication and complexity. - Work on exposing things like eventSender through an NPAPI plugin. That way it can be shared across browser vendors and could be used by the W3C test harness as well. This would be for APIs that we want for testing but don't make sense to expose to the web. Ojan On Thu, May 31, 2012 at 11:56 AM, Darin Adler da...@apple.com wrote: I am thinking we should rename layoutTestController to testController. Or if you don’t like that name, maybe testHarness or some even better name. The old name is too long and the word “layout” is so strange. We could expose the object under the new name and the old one, and then over time convert all the tests to the new name, then get rid of the old one. What do you all think? -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] We should rename layoutTestController to testController
Darin, sorry for derailing this thread. I suppose I should have changed the subject. :) On Thu, May 31, 2012 at 1:51 PM, Jesus Sanchez-Palencia je...@webkit.orgwrote: Just a heads-up: there is a meta bug tracking this at https://bugs.webkit.org/show_bug.cgi?id=87284 . A few folks have been going through the list created during the hackathon (https://trac.webkit.org/wiki/Internals_Hackathon) and picking a method to port every now and then. Awesome! We were following the if-it-depends-only-on-WebCore-code-it-should-be-moved idea quite strictly, but now it seems we are facing a trade-off about internals x private WebKit APIs/SPIs from a few ports being tested. In other words, a few methods from layoutTestController are also testing code (private?) from WebKit and moving them to internals can leave these untested... For Qt I'm removing private methods that were only used by DRT or WTR, but I can't make this decision for other ports. If any of these involve the Chromium port, I'm sure we'd be happy to accommodate whatever is needed to move the API to internals. Feel free to CC me on bugs where that's the case. Anyway, if you know something that can be moved for sure, just open a bug blocking b87284 and I'm quite sure there will be people happy to work on it. Cheers, jesus Work on exposing things like eventSender through an NPAPI plugin. That way it can be shared across browser vendors and could be used by the W3C test harness as well. This would be for APIs that we want for testing but don't make sense to expose to the web. Ojan On Thu, May 31, 2012 at 11:56 AM, Darin Adler da...@apple.com wrote: I am thinking we should rename layoutTestController to testController. Or if you don’t like that name, maybe testHarness or some even better name. The old name is too long and the word “layout” is so strange. We could expose the object under the new name and the old one, and then over time convert all the tests to the new name, then get rid of the old one. What do you all think? -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using ref tests for repaint bugs
Do you just need to force a layout at the end of repaintTest, e.g. document.body.offsetHeight;? On Thu, May 24, 2012 at 6:34 AM, Andrei Bucur andrei.bu...@gmail.comwrote: Hello WebKittens, I'm trying to simplify the patch for a certain repaint bug ( https://bugs.webkit.org/show_bug.cgi?id=59863 ) by using ref tests instead of pixel tests. The test HTML file should first render the document in state 1 and then, by modifying the DOM, render it again state 2. The bug appears when repainting from state 1 to state 2. The expected result HTML for the test is just a document directly written in state 2 (so there is no transition, no bug). This should work on all the platforms but there's problem with the time control between paints for state 1 and state 2 in the test HTML. I've tried three options: 1. Use setTimeout() to update the DOM - I really wouldn't like to use this because it slows down the test and can be flaky. 2. Use layoutTestController.display() before modifying the DOM to trigger a paint - calling this seems to make the test fail because display() starts tracking the paint rects which doesn't happen in the reference document. 3. Make use of requestAnimationFrame to time the paintings for state 1 and state 2 - doesn't work directly out-of-the-box because requestAnimationFrame schedules a full repaint after going into state 2 and the bug doesn't reproduce anymore. Is there a preferred solution to this problem or another one that I'm missing? Thanks, Andrei. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using ref tests for repaint bugs
On Thu, May 24, 2012 at 3:59 PM, Andrei Bucur andrei.bu...@gmail.comwrote: No, I need a way to force a paint operation, similar to what layoutTestController.display() achieves, but without tracking the paint rectangles. Does anyone you find value in adding an optional parameter to display (or create another method on LTC) that disables paint rectangle tracking. I don't see any problems with this. Someone more familiar with the paint code might see issues. The main advantage for such a change would be in extending the ref tests power to also cover repaint tests (at least a part of them), thus making pixel tests less necessary. Repaint bugs appear usually when the layout changes but the old pixels are incorrectly invalidated. By making the reference HTML directly identical to the test document after the bug triggering changes have been applied, it is pretty certain that the output pixels for the reference file will not contain the repaint artifacts (there's no repaint operation involved). When running such a test with a client that has the bug, it will fail because the test file output will have artifacts and the reference output will not. On the other hand, using a client with the repaint bug fixed, the pixels output for the test and reference files should be identical. Thoughts? I'm open to experimenting with this, but different ports have very different repaint schemes (e.g. Chromium merges all the repaint rects into a large rect). I'm curious how likely a repaint reftest is to work across ports. If you're willing to investigate converting a handful of repaint tests over to reftests, that'd be very helpful. FWIW, you can easily test the reftests on the chromium port by sending a patch to the EWS via bugzilla since the Chromium-linux EWS runs the layout tests. Obviously, if we can convert repaint tests reftests (or, at least have new repaints tests be reftests), that would be a huge improvement in maintainability since repaint tests are one of the few categories of tests where we can't currently use reftests. Andrei. On Thu, May 24, 2012 at 11:57 PM, Ojan Vafai o...@chromium.org wrote: Do you just need to force a layout at the end of repaintTest, e.g. document.body.offsetHeight;? On Thu, May 24, 2012 at 6:34 AM, Andrei Bucur andrei.bu...@gmail.comwrote: Hello WebKittens, I'm trying to simplify the patch for a certain repaint bug ( https://bugs.webkit.org/show_bug.cgi?id=59863 ) by using ref tests instead of pixel tests. The test HTML file should first render the document in state 1 and then, by modifying the DOM, render it again state 2. The bug appears when repainting from state 1 to state 2. The expected result HTML for the test is just a document directly written in state 2 (so there is no transition, no bug). This should work on all the platforms but there's problem with the time control between paints for state 1 and state 2 in the test HTML. I've tried three options: 1. Use setTimeout() to update the DOM - I really wouldn't like to use this because it slows down the test and can be flaky. 2. Use layoutTestController.display() before modifying the DOM to trigger a paint - calling this seems to make the test fail because display() starts tracking the paint rects which doesn't happen in the reference document. 3. Make use of requestAnimationFrame to time the paintings for state 1 and state 2 - doesn't work directly out-of-the-box because requestAnimationFrame schedules a full repaint after going into state 2 and the bug doesn't reproduce anymore. Is there a preferred solution to this problem or another one that I'm missing? Thanks, Andrei. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 10:37 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 7:27 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 1:42 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 1:37 PM, Peter Kasting pkast...@chromium.orgwrote: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. I don't think we should do this. It seems very subtle. I'd rather be explicit. I'm OK with the rest of your numbered proposals. I disagree, but I'm fine with punting this to the list of controversial changes that we should discuss separately. FWIW, my main motivation here is that it allows us to unify the Skipped file format with the test_expectations.txt format. But again, we can discuss that separately. Adding SKIP (or whatever) to every line of skipped files is not a big hurdle, I think we could live with that is a transitions tep. I think the bigger hurdle is supporting chaining across multiple directories. That's great. I don't think anyone is opposed to adding chaining and I think that's on Dirk short-list of todos. The only potentially tricky thing here is figuring out what the platform modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar modifiers to Chromium (mac, linux, win, debug, release, etc). But I think the difficulty here is more in getting the python code right than agreeing on what the correct behavior is. I think it would be good if platform modifiers in the expectations file matched the platform names we use under the platform/ directory, either literally or as a suffix. So for example mac in the chromium expectations file could mean chromium-mac, in the qt expectations file it could mean qt-mac, in the mac expectations file it should not be used, but snowleopard would mean mac-snowleopard. That seems like a good way to organize it to me. Dirk, that make sense to you? Implementation detail: this might be nice for automating the generation of the macros used for determining what the generic ones map to (e.g. the mac maps to snowleopard, leopard, mt lion for chromium). It would be a nice secondary benefit to get rid of the current hard-coded lists. Only thing we'd need to hard code is what platform the generic directory maps to (e.g. that chromium-mac is actually the chromium mt lion directory). Also, currently the test_expectations.txt format requires either a bug number or a bug(ojan) entry. Would that be OK with you too? It has proven really good historically for keeping track of why a test was added to the file and for keeping track of getting the tests fixed (or, more importantly, having someone responsible for following up on it), but we could easily restrict this requirement to the Chromium expectations file if other ports dislike it. Requiring a bug seems good. I don't personally see the need for any exception to having a filed and tracked bug but perhaps folks closer to the problem know of a reason. Great. That's simpler. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
I have a proposal that hopefully addresses everyone's concerns, is minimally different from the current format *and* unifies the format with Skipped lists (i.e. Skipped lists as they exist today are valid test_expectations.txt format). The changes from the current format are as follows: -Leaving out any outcomes means the test is skipped, unless the test has a SLOW modifier, in which case the implied outcome is PASS. -Remove the SKIP modifier entirely. -Make everything but the test name case-insensitive. -Have the test name be the last item on the line -Separate modifiers/outcomes/testname with a common delimiter (i.e. :) -Any line starting with // or # is a comment -Including a bug entry is optional (maybe only if the test is skipped or wontfix?) -Bugs are listed as URLs, except for the bug_ojan format. Examples: foo/bar.html # Skipped wontfix : foo/bar.html # Skipped and we never intend to fix it. For things like dashboard compatibility tests that only Apple will ever want to make pass wontfix : text : foo/bar.html # We never intend to fix this, but we expect it to run and fail text diff. Will still fail if the test times out or crashes. webkit.org/b/12345 : text image : foo/bar.html # Flaky. Sometimes only fails text diff, sometimes only fails pixel diff. slow mac debug : foo/bar.html # Given extra time to run on mac debug, but is expected to pass. image+text : foo/bar.html # Fails both text and pixel diffs bug_ojan : fail : foo/bar.html # Fails and ojan is taking responsibility to address the failure. bug_ojan : foo/bar.html # Skipped and ojan is taking responsibility to address it. # The following would give lint errors image : text : foo/bar.html # two outcomes listed in separate groupings slow text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 text : foo/bar.html # outcome listed with non-outcome modifier crbug.com/12345 : wontfix : foo/bar.html # two non-outcomes modifiers listed in separate groupings On Thu, May 17, 2012 at 1:12 AM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote: There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). I think it makes sense to syntactically separate at least the platform/configuration keywords from the outcome keywords. It might be nice to separate the other things somehow as well, but I don't have any great ideas for things that are clearly better than the existing left/right convention. SKIP and WONTFIX seem parallel to PASS to me. I assume TEXT means a failure of text output and IMAGE means a pixel test failure? Yes. What does the build configuration info do? Does it apply the line to only those configurations? If that is the case, it does seem potentially different in kind, though maybe also better expressed by being able to combine multiple test_expectations files fro different platform/ directories. Yes. e.g. if you have: BUGWK12345 WIN DEBUG : test.html = TEXT then we expect test.html to have a failure of text output (i.e. -expected.txt doesn't match) on Windows debug builds and expect it to pass on all other platforms. I actually don't like the fact we're sort of duplicating platform fallback structure here (e.g. we should be able to have a separate text_expectations.txt in platform/chromium-win, platform/chromium-mac, etc... to do the same but we don't currently support that) but there was a strong resistance to do so among Chromium contributors the last time I checked because that'll increase the number of files to maintain. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 1:37 PM, Peter Kasting pkast...@chromium.orgwrote: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. I don't think we should do this. It seems very subtle. I'd rather be explicit. I'm OK with the rest of your numbered proposals. I disagree, but I'm fine with punting this to the list of controversial changes that we should discuss separately. FWIW, my main motivation here is that it allows us to unify the Skipped file format with the test_expectations.txt format. But again, we can discuss that separately. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:06 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:05 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 2:02 PM, Ryosuke Niwa rn...@webkit.org wrote: Seems reasonable as the first step except: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 6. Use the same character as a separator before and after the test (:). I counter-propose to drop : or any other delimiter altogether. Delimiters are the biggest pain points in the current syntax as suggested by Benjanmin, Darin, and I. If we're going to have expectations after test name, then there's no need for delimiters other than imposing arbitrary maintenance cost on everyone who edits the file. There is clearly not consensus on this. Getting rid of = as a delimiter seemed uncontroversial to me. Note that I explicitly put getting rid of delimiters in the controversial list. Who opposed this? And why is he or she not replying on this thread? Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. webkit.org/b/12345 : foo/bar.html MAC DEBUG : foo/bar.html : SLOW crbug.com/1234 : foo/bar.html : SLOW TEXT IMAGE webkit.org/b/12345 foo/bar.html MAC DEBUG foo/bar.html SLOW crbug.com/1234 foo/bar.html SLOW TEXT IMAGE ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. I don't think anyone else has opposed, and this has been complaints from many people who have complained about the current syntax (See Darin Ben's responses earlier in the thread). Given that, I don't see how we can rationalize to keep the delimiters if our goal is to accommodate their needs. I would like for us to move forward on the subset of things we can all agree on so that we can focus the rest of the bikeshedding on a smaller number of issues. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 2:29 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:19 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai o...@chromium.org wrote: Oh, I supposed I misread Peter's earlier email as being opposed to this. But, I for one am opposed to it. I find the jumble of modifiers in the second set of lines here much harder to quickly parse than the groupings in the first set of lines. I don't think anyone else has opposed, and this has been complaints from many people who have complained about the current syntax (See Darin Ben's responses earlier in the thread). Given that, I don't see how we can rationalize to keep the delimiters if our goal is to accommodate their needs. I would like for us to move forward on the subset of things we can all agree on so that we can focus the rest of the bikeshedding on a smaller number of issues. But then we won't be solving the problem that some people find the current syntax confusing. Replacing BUGCR12345/BUGWK12345 by URLs sound like a good idea. Moving SKIP/WONTFIX after test names also sound like a good idea. However, they aren't things people have complained about. On the other hand, people have, and repeatedly have, complained about delimiters on mailing lists and at the contributors' meeting. I'm just trying to make forward progress on the things we agree on. If someone makes a proposal that most people are happy with, then we can do that. But if we can't find such a proposal, then changing syntax to appease some people and make other people upset seems like unnecessary churn (e.g. now every developer who is already accustomed to the current format needs to make sense of the new one). So far, there has yet to be a proposal that doesn't have significant objections that addresses the problems you care about. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 3:37 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote: Perhaps we can address these two problems using some tools. e.g. I don't care about the format of test_expectations.txt at all if there was a GUI tool that lets me add/edit entries easily without ever having to look at raw files. Similarly, people who are advocating to keep the current syntax may not mind changing the format in the repository if there were a way to view the file using the current format for easier readability. Sure. Perhaps it would be more useful to spend time working on adding suppression support to garden-o-matic and getting other ports to use that instead of worrying about hand-editing files. FWIW, adding support for this to garden-o-matic is almost entirely a matter of coming up with the right UI. When you rebaseline expected failures, we already automatically update the test_expectations.txt entries for that test. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)
On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak m...@apple.com wrote: On May 17, 2012, at 1:42 PM, Ojan Vafai o...@chromium.org wrote: On Thu, May 17, 2012 at 1:37 PM, Peter Kasting pkast...@chromium.orgwrote: On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. I don't think we should do this. It seems very subtle. I'd rather be explicit. I'm OK with the rest of your numbered proposals. I disagree, but I'm fine with punting this to the list of controversial changes that we should discuss separately. FWIW, my main motivation here is that it allows us to unify the Skipped file format with the test_expectations.txt format. But again, we can discuss that separately. Adding SKIP (or whatever) to every line of skipped files is not a big hurdle, I think we could live with that is a transitions tep. I think the bigger hurdle is supporting chaining across multiple directories. That's great. I don't think anyone is opposed to adding chaining and I think that's on Dirk short-list of todos. The only potentially tricky thing here is figuring out what the platform modifiers mean for non-Chromium ports, e.g. I imagine Qt will want similar modifiers to Chromium (mac, linux, win, debug, release, etc). But I think the difficulty here is more in getting the python code right than agreeing on what the correct behavior is. Also, currently the test_expectations.txt format requires either a bug number or a bug(ojan) entry. Would that be OK with you too? It has proven really good historically for keeping track of why a test was added to the file and for keeping track of getting the tests fixed (or, more importantly, having someone responsible for following up on it), but we could easily restrict this requirement to the Chromium expectations file if other ports dislike it. I think with those three things and https://bugs.webkit.org/show_bug.cgi?id=86796 addressed, then the formats will be unified and the only thing to bikeshed over is the filename. :) Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Closing the loop...bug filed https://bugs.webkit.org/show_bug.cgi?id=86796 minus item 2 since that turned out to be controversial. On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote: We are arguing about too many orthogonal things at once. It seems like there are a few things we can agree on. Lets make incremental progress on those and after have a more targeted discussion on the controversial issues that are left. We don't need to make this change in one big commit (and shouldn't really). 1. Make SLOW and WONTFIX outcomes instead of modifiers. 2. Make outcomes optional. If they are left out, then the test is skipped (unless the test is marked SLOW, in which case it's expected to pass). There is no SKIP modifier. 3. Change bug modifiers to be URLs. The format for person bugs is bug(ojan). 4. The only things that come before the test are bug modifiers and platform/configuration modifiers (e.g. MAC DEBUG). 5. WONTFIX tests are either skipped or we ignore their output as long as they don't crash (I don't care which one we do). 6. Use the same character as a separator before and after the test (:). 7. Standardize both Skipped and test_expectations.txt on a common comment delimiter (i.e. #). Examples: webkit.org/b/12345 : foo/bar.html # test is skipped on all platforms/configurations this test_expectations.txt file applies to MAC DEBUG : foo/bar.html : SLOW # test is slow on mac debug, but is expected to pass crbug.com/1234 : foo/bar.html : SLOW TEXT IMAGE # test is slow and flaky. sometimes fails text diff, other times fails image diff Talking to people off-thread, there are strong differing opinions on the below controversial issues, so we should discuss separately since getting consensus will be difficult (maybe even on dedicated email threads): -Lowercasing the modifiers/outcomes -Moving all modifiers/outcomes before/after the test name -Making bug modifiers optional for non-wontfix tests -Getting rid of delimiters entirely On Thu, May 17, 2012 at 12:53 PM, Dirk Pranke dpra...@chromium.orgwrote: On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, May 17, 2012 at 12:36 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai o...@chromium.org wrote: -Make everything but the test name case-insensitive. I don't think I like this; it could lead to a lot of arbitrarily different formatting in the file, making things harder to read. Modifiers and expectations are already case-insensitive as far as I read the code yesterday. It may be that it's legal to mix the case, but no one does it. I think we'd probably find ourselves (re-)converging on some convention pretty quickly. I personally find all uppercase fairly easy to read in this case since it distinguishes the modifiers from the test name. I think this problem will disappear once we place modifiers and expectations on the same side because then there is exactly one place those tokens could appear. There is no need to scan through a line then. It's possible. As I said, having some other clear delimiter would help. If we have some other clear delimiter this would probably be less important, in which case all lower case would be fine as well. Initial caps seems less good to me. I find either all-lowercase or all-caps to be much harder to read than capitalized words. They look like a blob of letters to me. We might have to agree to disagree here, then, but that's fine. If there was a clear consensus that one style or another is better, we should go with that. Also, I don't think we use all-caps name anywhere else in WebKit so it's inconsistent with the convention we use elsewhere. I don't think this particularly matters. We should design a format based on what is most useful in this context. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] spamming the developer console
The amount of spam we throw in the developer console has grown quite a bit. spam == things logged to the console that web developers have no control over Unlike uncaught javascript exceptions (which can easily just be caught), there is no way to prevent the following from cluttering your console: -clientX/clientY deprecation warning -setting the fragment on a frame URL [1] -loading a resource disallowed by CSP -attempting to load a resource (e.g. in an image or iframe) that doesn't exist These warnings are not developer friendly. The equivalent would be to have compiler warnings that you are unable to turn off. It clutters the console and makes many console use-cases harder (e.g. console.log style debugging). We need a better solution. In many cases, these warnings don't actually represent bugs in the program (e.g. the are legit reasons to try to load a non-existent resource in an image element). Some potential solutions: 1. Create a new loggling level (browserwarnings?) in addition to the current errors/warnings/logs. This kind of half-solves the problem since you can't just side these logs. 2. Have preventDefault in an onError handler prevent logging these to the console (works for the navigation warnings). 3. Have some other inspector panel where these get logged. 4. Have some window-level state that disables all these sorts of warnings (or something more fine-grained like being able to disable deprecation warnings and navigation warnings separately). 5. ??? My preference would be to do both options 1 and 2 (they're not mutually exclusive), unless someone has a better suggestion. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] spamming the developer console
On Fri, May 11, 2012 at 2:25 PM, Adam Barth aba...@webkit.org wrote: On Fri, May 11, 2012 at 2:21 PM, Ojan Vafai o...@chromium.org wrote: The amount of spam we throw in the developer console has grown quite a bit. spam == things logged to the console that web developers have no control over Unlike uncaught javascript exceptions (which can easily just be caught), there is no way to prevent the following from cluttering your console: -clientX/clientY deprecation warning -setting the fragment on a frame URL [1] -loading a resource disallowed by CSP ^^^ Why isn't this a bug that developers would want to fix? For example, in some code I just wrote, I append an iframe to the DOM, then at some later point set its src. When I append the iframe to the DOM, it tries to load about:blank, which CSP disallows. I could restructure the code to not append the iframe until I know what it's source will be, but it makes the code more complicated than it needs to be. The only downside to appending it first is this warning. -attempting to load a resource (e.g. in an image or iframe) that doesn't exist These warnings are not developer friendly. The equivalent would be to have compiler warnings that you are unable to turn off. It clutters the console and makes many console use-cases harder (e.g. console.log style debugging). We need a better solution. In many cases, these warnings don't actually represent bugs in the program (e.g. the are legit reasons to try to load a non-existent resource in an image element). Some potential solutions: 1. Create a new loggling level (browserwarnings?) in addition to the current errors/warnings/logs. This kind of half-solves the problem since you can't just side these logs. 2. Have preventDefault in an onError handler prevent logging these to the console (works for the navigation warnings). 3. Have some other inspector panel where these get logged. 4. Have some window-level state that disables all these sorts of warnings (or something more fine-grained like being able to disable deprecation warnings and navigation warnings separately). 5. ??? My preference would be to do both options 1 and 2 (they're not mutually exclusive), unless someone has a better suggestion. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] spamming the developer console
On Fri, May 11, 2012 at 2:39 PM, Maciej Stachowiak m...@apple.com wrote: On May 11, 2012, at 2:21 PM, Ojan Vafai o...@chromium.org wrote: The amount of spam we throw in the developer console has grown quite a bit. spam == things logged to the console that web developers have no control over Unlike uncaught javascript exceptions (which can easily just be caught), there is no way to prevent the following from cluttering your console: -clientX/clientY deprecation warning -setting the fragment on a frame URL [1] -loading a resource disallowed by CSP -attempting to load a resource (e.g. in an image or iframe) that doesn't exist These warnings are not developer friendly. The equivalent would be to have compiler warnings that you are unable to turn off. It clutters the console and makes many console use-cases harder (e.g. console.log style debugging). We need a better solution. In many cases, these warnings don't actually represent bugs in the program (e.g. the are legit reasons to try to load a non-existent resource in an image element). Some potential solutions: 1. Create a new loggling level (browserwarnings?) in addition to the current errors/warnings/logs. This kind of half-solves the problem since you can't just side these logs. 2. Have preventDefault in an onError handler prevent logging these to the console (works for the navigation warnings). 3. Have some other inspector panel where these get logged. 4. Have some window-level state that disables all these sorts of warnings (or something more fine-grained like being able to disable deprecation warnings and navigation warnings separately). 5. ??? My preference would be to do both options 1 and 2 (they're not mutually exclusive), unless someone has a better suggestion. It seems like items related to networking (which is most of the above, other than deprecation warnings) could be represented in resource-oriented views instead of in the console. That way you can still see if something failed to load, but at the time of looking at loading rather than looking for JS-related information and errors. As for deprecation warnings, I am not a huge fan of them, but if there is an easy way to turn them off or ignore them, then they are even less likely to have any benefit. Just addressing the networking items is enough to satisfy me. The deprecation warnings are a much more complicated discussion and IMO less severe of an annoyance. Or, I suppose, the point of them is to be annoying. Showing these errors in the network panel instead of in the console sounds good to me, especially since we don't associate a line-number/stacktrace/file with them anyways. The only downside here is that you need to have the inspector open when the load occurs in order to get the error, but that seems fine with me. In fact, the non-existant resource warning is already redundant with information in the network panel. So, really we should just kill those and find a home for CSP and fragment on frame cases. Speaking of the fragment on frame cases, why do we ever disallow setting the fragment on a frame? That seems unnecessarily restrictive to me. I thought the security issue was addressed by just avoiding scrolling when the fragment is set. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] spamming the developer console
On Fri, May 11, 2012 at 2:48 PM, Ojan Vafai o...@chromium.org wrote: On Fri, May 11, 2012 at 2:39 PM, Maciej Stachowiak m...@apple.com wrote: On May 11, 2012, at 2:21 PM, Ojan Vafai o...@chromium.org wrote: The amount of spam we throw in the developer console has grown quite a bit. spam == things logged to the console that web developers have no control over Unlike uncaught javascript exceptions (which can easily just be caught), there is no way to prevent the following from cluttering your console: -clientX/clientY deprecation warning -setting the fragment on a frame URL [1] -loading a resource disallowed by CSP -attempting to load a resource (e.g. in an image or iframe) that doesn't exist These warnings are not developer friendly. The equivalent would be to have compiler warnings that you are unable to turn off. It clutters the console and makes many console use-cases harder (e.g. console.log style debugging). We need a better solution. In many cases, these warnings don't actually represent bugs in the program (e.g. the are legit reasons to try to load a non-existent resource in an image element). Some potential solutions: 1. Create a new loggling level (browserwarnings?) in addition to the current errors/warnings/logs. This kind of half-solves the problem since you can't just side these logs. 2. Have preventDefault in an onError handler prevent logging these to the console (works for the navigation warnings). 3. Have some other inspector panel where these get logged. 4. Have some window-level state that disables all these sorts of warnings (or something more fine-grained like being able to disable deprecation warnings and navigation warnings separately). 5. ??? My preference would be to do both options 1 and 2 (they're not mutually exclusive), unless someone has a better suggestion. It seems like items related to networking (which is most of the above, other than deprecation warnings) could be represented in resource-oriented views instead of in the console. That way you can still see if something failed to load, but at the time of looking at loading rather than looking for JS-related information and errors. As for deprecation warnings, I am not a huge fan of them, but if there is an easy way to turn them off or ignore them, then they are even less likely to have any benefit. Just addressing the networking items is enough to satisfy me. The deprecation warnings are a much more complicated discussion and IMO less severe of an annoyance. Or, I suppose, the point of them is to be annoying. Showing these errors in the network panel instead of in the console sounds good to me, especially since we don't associate a line-number/stacktrace/file with them anyways. The only downside here is that you need to have the inspector open when the load occurs in order to get the error, but that seems fine with me. Ugh. I was a little wrong here. The 404 errors do show you a stacktrace. Still, it seems like we should find a way to fit that into the network panel. While we're at it, it would be nice for the CSP and fragment on frame cases to show you a stacktrace as well. In fact, the non-existant resource warning is already redundant with information in the network panel. So, really we should just kill those and find a home for CSP and fragment on frame cases. Speaking of the fragment on frame cases, why do we ever disallow setting the fragment on a frame? That seems unnecessarily restrictive to me. I thought the security issue was addressed by just avoiding scrolling when the fragment is set. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] spamming the developer console
Bugs filed. Navigation warnings: https://bugs.webkit.org/show_bug.cgi?id=86263. clientX/clientY: https://bugs.webkit.org/show_bug.cgi?id=86264. Would be good for someone informed on that issue to chime in. On Fri, May 11, 2012 at 3:08 PM, Ojan Vafai o...@chromium.org wrote: On Fri, May 11, 2012 at 2:48 PM, Ojan Vafai o...@chromium.org wrote: On Fri, May 11, 2012 at 2:39 PM, Maciej Stachowiak m...@apple.com wrote: On May 11, 2012, at 2:21 PM, Ojan Vafai o...@chromium.org wrote: The amount of spam we throw in the developer console has grown quite a bit. spam == things logged to the console that web developers have no control over Unlike uncaught javascript exceptions (which can easily just be caught), there is no way to prevent the following from cluttering your console: -clientX/clientY deprecation warning -setting the fragment on a frame URL [1] -loading a resource disallowed by CSP -attempting to load a resource (e.g. in an image or iframe) that doesn't exist These warnings are not developer friendly. The equivalent would be to have compiler warnings that you are unable to turn off. It clutters the console and makes many console use-cases harder (e.g. console.log style debugging). We need a better solution. In many cases, these warnings don't actually represent bugs in the program (e.g. the are legit reasons to try to load a non-existent resource in an image element). Some potential solutions: 1. Create a new loggling level (browserwarnings?) in addition to the current errors/warnings/logs. This kind of half-solves the problem since you can't just side these logs. 2. Have preventDefault in an onError handler prevent logging these to the console (works for the navigation warnings). 3. Have some other inspector panel where these get logged. 4. Have some window-level state that disables all these sorts of warnings (or something more fine-grained like being able to disable deprecation warnings and navigation warnings separately). 5. ??? My preference would be to do both options 1 and 2 (they're not mutually exclusive), unless someone has a better suggestion. It seems like items related to networking (which is most of the above, other than deprecation warnings) could be represented in resource-oriented views instead of in the console. That way you can still see if something failed to load, but at the time of looking at loading rather than looking for JS-related information and errors. As for deprecation warnings, I am not a huge fan of them, but if there is an easy way to turn them off or ignore them, then they are even less likely to have any benefit. Just addressing the networking items is enough to satisfy me. The deprecation warnings are a much more complicated discussion and IMO less severe of an annoyance. Or, I suppose, the point of them is to be annoying. Showing these errors in the network panel instead of in the console sounds good to me, especially since we don't associate a line-number/stacktrace/file with them anyways. The only downside here is that you need to have the inspector open when the load occurs in order to get the error, but that seems fine with me. Ugh. I was a little wrong here. The 404 errors do show you a stacktrace. Still, it seems like we should find a way to fit that into the network panel. While we're at it, it would be nice for the CSP and fragment on frame cases to show you a stacktrace as well. In fact, the non-existant resource warning is already redundant with information in the network panel. So, really we should just kill those and find a home for CSP and fragment on frame cases. Speaking of the fragment on frame cases, why do we ever disallow setting the fragment on a frame? That seems unnecessarily restrictive to me. I thought the security issue was addressed by just avoiding scrolling when the fragment is set. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] EFL Debug Buildbot Green
That's a great milestone. Congratulations! On Thu, May 10, 2012 at 8:43 AM, Dominik Röttsches dominik.rottsc...@intel.com wrote: Hi all, We're happy to share with you that yesterday the EFL Linux Debug Buildbot has turned green for the first time. http://build.webkit.org/builders/EFL%20Linux%20Debug This has been an example of a great team effort [1]. We'd like to thank the friendly reviewers and committers who helped us for their support! We'll keep a close eye on our buildbot lamp http://goo.gl/ei1gL from now and set up a gardening schedule to keep it green and tidy in the future. Dominik [1] http://wkb.ug/85601 -- Dominik Röttsches dominik.rottsc...@intel.com dominik.rottsc...@intel.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Default viewport value on WAP DTDs
On Tue, May 8, 2012 at 11:32 AM, Hugo Parente Lima hugo.l...@openbossa.orgwrote: On Tuesday, May 08, 2012 07:22:17 PM John Mellor wrote: Both Chrome for Android and the legacy Android Browser do what Hugo described, i.e. we default to a widthÞvice-width viewport* when an XHTML-MP doctype is provided. And as suggested in wkbug.com/55874, we similarly default to a widthÞvice-width viewport if a legacy HandheldFriendly meta tag is present, e.g.: meta nameHandheldFriendly contenttrue and if a legacy MobileOptimized meta tag is present then we will use a viewport with the width provided, e.g. if the following tag is present: meta nameMobileOptimized content240 we will treat that the same as a width$0 viewport tag (though IIRC the legacy Android Browser ignored the provided width and just treated it as device-width). However note that we give these implicit viewport declarations lower priority than actual viewport meta tags, so irrespective of the order they appear in the document, a viewport meta tag will override any behaviour we infer from the doctype or those legacy meta tags. We do all this because some number of legacy sites depend on this behaviour. I'm afraid we don't know how common these sites still are; but these heuristics have never seemed to cause us any problems. It makes sense to propose standardizing the XHTML-MP behaviour on www-style since I agree with Kenneth that this is something that should be covered by the http://dev.w3.org/csswg/css-device-adapt/ spec. That would also be a good place to try and standardize how we deal with legacy mobile meta tags. And we'd be happy to see any of these heuristics incorporated into WebKit (either before or after they get standardized) so we don't need to fork them. Kenneth pointed that the XHTML-MP behavior is already on the spec, but on a non normative section, the introduction: Certain DOCTYPEs (for instance XHTML Mobile Profile) are used to recognize mobile documents which are assumed to be designed for handheld devices, hence using the viewport size as the initial containing block size. on http://www.w3.org/TR/css-device-adapt/ I see. Given this and the fact that Android already does this, I think this change is fine. Do any Apple/iOS folk object? The Android behavior seems a little more conservative to me, so I'd prefer if we did that (e.g. don't mess with zooming). We should make the most minimal change we can to optimize compatibility. Before making this change, I'd still like to see discussion on www-style for this. Specifically, it would be good for this to move to a normative section and to be more concretely specified. This open-ended language is not useful for actually achieving interoperability across mobile browser vendors. It should say specifically which doctypes and how it relates to the viewport meta tag. Again, I prefer the Android behavior here of always having the viewport meta tag win. Speaking of which, given the note in the spec, has there already been discussion on www-style about this? Ojan Cheers, John *: (unlike Hugo's current patch, we don't add initial-scale1.0, user-scalableno, just widthÞvice-width, since there doesn't seem to be any reason to assume that XHTML-MP sites want to disable zooming, and it may actually lead to a worse user experience). You are right, setting just the width should be enough. On Sat, May 5, 2012 at 12:58 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: Hi there, On Sat, May 5, 2012 at 7:12 AM, Ojan Vafai o...@chromium.org wrote: I see. That's unfortunate. I don't really know the best path forward here. I'm inclined to agree with Alexey that we should at least try to st ndardize this before committing code. It's not clear to me where this should be specced. Easiest path forward is to make this proposal to both wha...@whatwg.org for the HTML spec and www-st...@w3.org for the CSS Device Adaptation spec. I will pass it by the CSS Device Adaptation spec first as I really think it fits there. We'll see what the response there is and can decide what to do next based off that response. Does that sound OK? I think we will add a feature flag for now, together with layout tests for a document with XHTML-MP doctype using and not using fixed layouting. I'm reluctant to make a change like this, but it sounds like there might not be a better choice. One concern I have is how many sites would break due to this behavior? For example, will this fix sites on N9, but break them on Android/iOS or are these wapfor m doctypes never sent to Android/iOS because of UA-sniffing? It can only break browsers respecting the viewport meta and using fixed layouting in some way, those currently mobile browsers. As far as I heard
Re: [webkit-dev] Default viewport value on WAP DTDs
Instead of UA faking, is it possible for you to pick an actual UA string that is more compatible with the web at large? For Chromium we experimented with making the most minimal UA string possible without a big loss in web compatibility. To our disappointment, we found we had to match the Safari UA string almost exactly. Our current UA string is Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/536.6 (KHTML, like Gecko) Chrome/20.0.1096.1 Safari/536.6. The parts that we found to be safe to change are the platform names in the first sent of parenthesis. The version number after AppleWebKit. And the Chrome/versionNumber section. Even getting rid of the Safari/versionNumber caused us significant web compatibility problems. That said, we did all this testing in 2008. The web may have changed considerably since then. In either case, if your UA string diverges too much, I expect this problem will just be the tip of the iceberg of compatibility problems you'll encounter. So it might be worth considering changing your UA string before trying to add new DocType switching behavior. Ojan On Fri, May 4, 2012 at 10:53 AM, Hugo Parente Lima hugo.l...@openbossa.orgwrote: On Friday, May 04, 2012 10:11:07 AM Ryosuke Niwa wrote: On Fri, May 4, 2012 at 9:39 AM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: This is not supporting XHTML-MP, as we are not implementing anything special to support it. We are basically showing the content as it was HTML5 and that solves most real use-cases. Injecting a proper viewport configuration makes it also layout properly. Okay. Is this change observable by the page? Or more specifically, can a web page currently feature-detect whether a given browser support XHTML-MP by checking the size of the viewport? The page knows nothing, just as it knows nothing about the ~980 pixels used for the canvas width, it's a matter of change a magic value to the device- width to get websites better looking. I attached screenshots of MiniBrowser runnin with and without the patch using: MiniBrowser --window-size 480x720 http://m.yahoo.com Without patch (viewport of 980 pixels): https://bug-85425-attachments.webkit.org/attachment.cgi?id=140272 Patched (viewport of 880 pixels) https://bug-85425-attachments.webkit.org/attachment.cgi?id=140273 If the answer is yes, then we'll be breaking the feature detection. Unfortunately most unknown mobile browsers tend to get lots of XHTML-MP. Heck, we even get that for google.com on the Nokia N9 :-( as well as other high profile sites. Yeah, it's very unfortunate. This makes the sites render acceptable, until we can advocate the sites to accept our user agent, something which we haven't always had luck with. Google for one didn't want to provide us the Tier 1 site of google.com on the N9, even though it works a lot better than the XHTML-MP version we are being served. I don't see this situation change any time soon. Can we work-around this issue by faking the user agent string? If you are working on your own browser you wont be telling every website that you are a iPhone forever, at least you will not be happy doing that. Regards. Hugo Parente Lima ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Default viewport value on WAP DTDs
I see. That's unfortunate. I don't really know the best path forward here. I'm inclined to agree with Alexey that we should at least try to standardize this before committing code. It's not clear to me where this should be specced. Easiest path forward is to make this proposal to both wha...@whatwg.org for the HTML spec and www-st...@w3.org for the CSS Device Adaptation spec. We'll see what the response there is and can decide what to do next based off that response. Does that sound OK? I'm reluctant to make a change like this, but it sounds like there might not be a better choice. One concern I have is how many sites would break due to this behavior? For example, will this fix sites on N9, but break them on Android/iOS or are these wapforum doctypes never sent to Android/iOS because of UA-sniffing? On Fri, May 4, 2012 at 3:39 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: Nokia actually looked into this about a year ago and we homogenized our UA strings across our different devices, so that we could start to tell contents providers to give us the best content supported by our browsers. Part of this work was actually simplifying our UA string so much as possible and it is actually quite similar to what you are using today. The user agent for the N9 browser, for instance, is: Mozilla/5.0 (MeeGo; NokiaN9) AppleWebKit/534.13 (KHTML, like Gecko) NokiaBrowser/8.5.0 Mobile Safari/534.13 The problem is not just the user agent. For instance the user agent is known by your Google, and we did pass validation for Tier 1 YouTube content, but the Google search team, as far as I heard, decided that we didn't have enough market penetration for them to turn on Tier 1 content for us, and serves us the XHTML-MP (Tier 3?) content instead. As far as I understand, the decision comes from that team not wanting to dedicate resources to make sure the Tier 1 content keeps working on our device. I totally understand their reasoning and decision, but it is a saddens me given the promise of the open web and HTML5. It is even more sad that this is not a unique case and it will only be solved the day content providers stop looking at the user agent strings. Kenneth On Fri, May 4, 2012 at 11:32 PM, Ojan Vafai o...@chromium.org wrote: Instead of UA faking, is it possible for you to pick an actual UA string that is more compatible with the web at large? For Chromium we experimented with making the most minimal UA string possible without a big loss in web compatibility. To our disappointment, we found we had to match the Safari UA string almost exactly. Our current UA string is Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/536.6 (KHTML, like Gecko) Chrome/20.0.1096.1 Safari/536.6. The parts that we found to be safe to change are the platform names in the first sent of parenthesis. The version number after AppleWebKit. And the Chrome/versionNumber section. Even getting rid of the Safari/versionNumber caused us significant web compatibility problems. That said, we did all this testing in 2008. The web may have changed considerably since then. In either case, if your UA string diverges too much, I expect this problem will just be the tip of the iceberg of compatibility problems you'll encounter. So it might be worth considering changing your UA string before trying to add new DocType switching behavior. Ojan On Fri, May 4, 2012 at 10:53 AM, Hugo Parente Lima hugo.l...@openbossa.org wrote: On Friday, May 04, 2012 10:11:07 AM Ryosuke Niwa wrote: On Fri, May 4, 2012 at 9:39 AM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: This is not supporting XHTML-MP, as we are not implementing anything special to support it. We are basically showing the content as it was HTML5 and that solves most real use-cases. Injecting a proper viewport configuration makes it also layout properly. Okay. Is this change observable by the page? Or more specifically, can a web page currently feature-detect whether a given browser support XHTML-MP by checking the size of the viewport? The page knows nothing, just as it knows nothing about the ~980 pixels used for the canvas width, it's a matter of change a magic value to the device- width to get websites better looking. I attached screenshots of MiniBrowser runnin with and without the patch using: MiniBrowser --window-size 480x720 http://m.yahoo.com Without patch (viewport of 980 pixels): https://bug-85425-attachments.webkit.org/attachment.cgi?id=140272 Patched (viewport of 880 pixels) https://bug-85425-attachments.webkit.org/attachment.cgi?id=140273 If the answer is yes, then we'll be breaking the feature detection. Unfortunately most unknown mobile browsers tend to get lots of XHTML-MP. Heck, we even get that for google.com on the Nokia N9 :-( as well as other high profile sites. Yeah, it's
Re: [webkit-dev] Bot watching and Apple bots
Good to know. I had stopped paying attention to many of the Apple bots for the reason you mention. It would be really helpful if someone could make the webkit-patch tooling works correctly for the non-Chromium bots. Specifically, webkit-patch rebaseline-test and webkit-patch garden-o-matic. They are 99% of the way towards working for non-Chromium bots and just need a handful of small fixes. I've outlined what the necessary work at https://bugs.webkit.org/show_bug.cgi?id=82719. I'd be happy to walk someone through any of this. Ojan On Wed, Apr 25, 2012 at 1:58 PM, Maciej Stachowiak m...@apple.com wrote: Hi everyone, A while back, Apple's WebKit teams instituted a bot watching rotation to try to get the bots for our ports to get and stay green. We've managed to consistently stay around low single digits of failures, but the green doesn't seem to stick. We think some folks in the community may have stopped paying attention to bots for Apple ports since they were poorly maintained for some time. We'd appreciate it if folks would pay attention to our core test and build bots when landing. Currently, our focus is on getting the following bots green. Once they are working well, we may expand to more: * Lion Intel Debug Build - http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Build%29 * Lion Intel Release Build - http://build.webkit.org/builders/Lion%20Intel%20Release%20%28Build%29 * Lion Intel Debug Tests - http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28Tests%29 * Lion Intel Debug (WebKit2) Tests - http://build.webkit.org/builders/Lion%20Intel%20Debug%20%28WebKit2%20Tests%29 * Lion Intel Release Tests - http://build.webkit.org/builders/Lion%20Intel%20Release%20%28Tests%29 * Lion Intel Release (WebKit2) - Tests http://build.webkit.org/builders/Lion%20Intel%20Release%20%28WebKit2%20Tests%29 We'll also be posting the bot watchers on duty at any given time here: http://trac.webkit.org/wiki/Apple_Bot_Watchers You should feel free to contact them if you're experiencing problems or have caused a regression. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Handling failing reftests
Ossy, I think the right thing to do, in general, with failing reftests is to skip them. Dave, you can see the chromium win/linux failures here (it passes on chromium mac): http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=trueshowLargeExpectations=truerevision=113914tests=fast%2Fmulticol%2Fcell-shrinkback.html. Search the page for DIFF: to see the image diffs. I agree that it's hard sometimes to construct reftests that work, but once you've done so, the cost on the project of maintaining the test is usually considerably lower than pixel tests. On Thu, Apr 12, 2012 at 11:23 AM, David Hyatt hy...@apple.com wrote: That reftest is mine. It was supposed to work cross-platform. I am happy to fix, but I'd need to understand what about it didn't work on other platforms. Can someone send me a screenshot of what it looks like? (This is actually why I hate reftests for pagination. It's very hard to construct an unpaginated version that matches the paginated layout without tripping over stuff like this.) Thanks Dave (hy...@apple.com) On Apr 12, 2012, at 1:01 PM, Ryosuke Niwa wrote: On Thu, Apr 12, 2012 at 10:57 AM, Dirk Pranke dpra...@chromium.orgwrote: This is an interesting situation; does it make sense to require our reftests to be generic, or is that unrealistic? I also wonder if we should allow platform-specific pixel tests to coexist with platform-specific reference tests (since you could obviously write a platform-specific ref test that just displayed a single IMG, but the tools don't make that particularly easy). Allowing platform-specific pixel results for a ref test is a bad idea IMO. It makes things even more complicated and harder to understand than it already is today. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is converting pixel tests to reftests kosher for imported libraries?
On Thu, Apr 12, 2012 at 12:50 PM, Robert Hogan li...@roberthogan.netwrote: On Thursday 12 April 2012 00:58:47 Ryosuke Niwa wrote: On Wed, Apr 11, 2012 at 4:45 PM, Ojan Vafai o...@chromium.org wrote: I agree with the sentiment that we should be upstreaming these to the W3C, but I don't see why we would require upstreaming them first instead of committing them locally and then upstreaming them. How do we know whether a reference file came from W3C repository or not. (Maybe by the fact it's named *-expected.html?) Yes, that is it exactly. Presumably that's by design - as NRWT currently won't use anything like *-ref.htm unless it's in a manifest. Also, there are directories with reftest.list but without reference files for some tests. The last time I checked, you were opposed to having bot reftest.list and *-expected.html / *-expected-mismatch.html files. Have you changed your opinion on this? It would be good if this was allowed. It would allow us to import the reftest.list from the CSS test suite while keeping our own reference results in there for tests in the suite that don't have them yet. I'm not opposed to changing this if it gives us a path forward. I still firmly believe we should not use manifest files for non-imported test suites, but we don't need to enforce that with the tooling if it makes things more complicated. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Handling failing reftests
At that point, we should probably just make it a pixel test? At some level we just don't have enough experience with reftests to understand what the common problems with them are and make broad policy recommendations. On Thu, Apr 12, 2012 at 1:30 PM, Jacob Goldstein jac...@adobe.com wrote: Isn't the goal of writing a ref test that it is not platform specific? From: Ryosuke Niwa rn...@webkit.org Date: Thu, 12 Apr 2012 12:29:42 -0700 To: Ojan Vafai o...@chromium.org Cc: Dirk Pranke dpra...@chromium.org, WebKit Development webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] Handling failing reftests On Thu, Apr 12, 2012 at 11:53 AM, Ojan Vafai o...@chromium.org wrote: I agree that it's hard sometimes to construct reftests that work, but once you've done so, the cost on the project of maintaining the test is usually considerably lower than pixel tests. Not so sure. There are cases where we have these platform specific failures for ref tests, and they're much harder to fix than rebaselining pixel results. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Handling failing reftests
On Thu, Apr 12, 2012 at 2:18 PM, Dirk Pranke dpra...@chromium.org wrote: On Apr 12, 2012 1:53 PM, Ryosuke Niwa rn...@webkit.org wrote: My suggestion is to try to make it platform-independent but it's significantly harder than rebaselining the results for pixel tests. As you point out, it defeats one of the benefits of reftests if we end up introducing platform-specific reference files. - Ryosuke It defeats one of the benefits but not all, I think (hope?) Maintaining a reftest with multiple platform-specific baselines seems like it would be strictly more work than maintaining a pixel test with multiple platform-specific baselines in most cases. -- Dirk On Thu, Apr 12, 2012 at 1:46 PM, Jacob Goldstein jac...@adobe.com wrote: Right, but when those differences come to light, should the goal be to adjust the ref test to make it platform-independent, or is having platform-specific ref tests acceptable? Doesn't that put us in the same situation as having platform-specific pixel tests? From: Ryosuke Niwa rn...@webkit.org Date: Thu, 12 Apr 2012 13:43:26 -0700 To: Jacob Goldstein jac...@adobe.com Cc: Ojan Vafai o...@chromium.org, Dirk Pranke dpra...@chromium.org, WebKit Development webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] Handling failing reftests Right but you wouldn't know platform-specific issues until you land them. e.g. rounding errors, subtle font differences in edge cases, etc... On Thu, Apr 12, 2012 at 1:30 PM, Jacob Goldstein jac...@adobe.com wrote: Isn't the goal of writing a ref test that it is not platform specific? From: Ryosuke Niwa rn...@webkit.org Date: Thu, 12 Apr 2012 12:29:42 -0700 To: Ojan Vafai o...@chromium.org Cc: Dirk Pranke dpra...@chromium.org, WebKit Development webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] Handling failing reftests On Thu, Apr 12, 2012 at 11:53 AM, Ojan Vafai o...@chromium.org wrote: I agree that it's hard sometimes to construct reftests that work, but once you've done so, the cost on the project of maintaining the test is usually considerably lower than pixel tests. Not so sure. There are cases where we have these platform specific failures for ref tests, and they're much harder to fix than rebaselining pixel results. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Process for making changes that affect layout test results
Typically, if you're working on Chromium Linux or Win, you'd include the new expected results for that platform in your initial commit/code-review as well. On Wed, Apr 11, 2012 at 2:09 PM, Tony Payne tpa...@chromium.org wrote: All code I'm changing is inside of #if PLATFORM(CHROMIUM) blocks. Thanks for the quick answer. Tony On Wed, Apr 11, 2012 at 2:03 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Apr 11, 2012 at 1:57 PM, Tony Payne tpa...@chromium.org wrote: Given the recent discussion on test_expectations.txt, perhaps the answer to my question is still up in the air. I'm working on a change that I expect to require changing the expectations for about 75 tests on chromium win and linux. https://trac.webkit.org/wiki/Rebaseline seems to only cover the gardening work to rebaseline after the commit. I cannot find any wiki pages that describe what the original author is expected to do when making visual changes. Should I attempt to rebaseline manually? Should I mark the tests as failing? Should I just check in and let the bots go red? Just land the patch and rebaseline the tests. Please also coordinate with Chromium port's WebKit gardener when landing this patch. Also, does this patch only affect Chromium Windows and Linux, and not GTK, Qt, Windows, etc...? If the answer is no, and will affect other non-Chromium ports, then you're also responsible for rebaselining or coordinating with other ports to make sure you don't break tests on their ports as well. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is converting pixel tests to reftests kosher for imported libraries?
On Wed, Apr 11, 2012 at 3:48 PM, Dirk Pranke dpra...@chromium.org wrote: On Wed, Apr 11, 2012 at 3:46 PM, Dirk Pranke dpra...@chromium.org wrote: On Fri, Mar 9, 2012 at 2:49 PM, Sam Weinig wei...@apple.com wrote: On Mar 7, 2012, at 4:41 PM, Ojan Vafai wrote: I just did a first pass a greening the Chromium Lion bot: http://trac.webkit.org/changeset/110096. Of these hundreds of tests, ~99% of them are perfect candidates for being reftests (e.g. they contain one line of text and a solid box or two under the text), but most of them are in the CSS imported test suites. Is it kosher to convert them to reftests or should we leave pixel tests from imported test suites alone? If we want to make these ref tests, it probably makes more sense to do that work with the CSS WG, so that they can be part of the standard test suite. Until then, I think we should keep them regular pixel tests. Note that this thread (to resurrect it just-after-easter because it's timely) is directly relevant to the note I just sent out about importing test suites. I, at least, would like some clarification ... if we are importing tests that have no accompanying expected result (and are expected to be inspected manually for correctness), is it acceptable to write reference html for the tests, or do they have to be imported as pixel tests? Put differently, we either need to add an -expected html or an -expected png. Does it have to be the latter? I strongly prefer adding -expected.html files. While there is a chance the reference may not cover all the code paths the original test intended, I believe our overall test coverage will be better with more reftests and fewer pixel tests, not to mention our project-wide sanity from spending less time doing expectations file management. Pixel tests accidentally let bugs through all the time because not all ports run them and because it's often hard to tell if a new result is correct. I agree with the sentiment that we should be upstreaming these to the W3C, but I don't see why we would require upstreaming them first instead of committing them locally and then upstreaming them. If there are people willing to do the work, lets allow them do it either way. I personally think it's acceptable, but I understand that there might be a difference of opinion. https://bugs.webkit.org/show_bug.cgi?id=72987 is an example of this. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] handling failing tests (test_expectations, Skipped files, etc.)
I don't think we can come up with a hard and fast rule given current tooling. In a theoretical future world in which it's easy to get expected results off the EWS bots (or some other infrastructure), it would be reasonable to expect people to incorporate the correct expected results for any EWS-having ports before committing the patch. I expect we'd all agree that would be better than turning the bots red or adding to test_expectations.txt/Skipped files. In the current world, it's a judgement call. If I expect a patch to need a lot of platform-specific baselines, I'll make sure to commit it at a time when I have hours to spare to cleanup any failures or, if I can't stick around for the bots to cycle, I'll add it to test_expectations.txt appropriately. Both approaches have nasty tradeoffs. It is probably worth writing up a wiki page outlining these two options and explaining why you might do one or the other for people new to the project, but I don't see benefit in trying to pick a hard rule that everyone must follow. Ojan On Tue, Apr 10, 2012 at 11:58 AM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Apr 10, 2012 at 11:42 AM, Stephen Chenney schen...@chromium.orgwrote: On Tue, Apr 10, 2012 at 1:00 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Apr 10, 2012 at 6:10 AM, Stephen Chenney schen...@chromium.orgwrote: There is a significant practical problem to turn the tree red and work with someone to rebaseline the tests. It takes multiple hours for some bots to build and test a given patch. That means, at any moment, you will have maybe tens and in some cases hundreds of failing tests associated with some changelist that you need to track on the bots. You might have more failing tests associated with a different changelist, and so on. But you have to do this for non-Chromium ports anyway because they don't use test_expectations.txt and skipping the tests won't help you generate new baseline. In my opinion, we should not further diverge from the way things are done in other ports. How long on average does it take a builder to get through a change on another port? Right now the Chromium Mac 10.5 and 10.6 dbg builds are processing a patch from about 3 hours ago. About 20 patches have gone in since then. For the Mac 10.5 tree to ever be green would require there being no changes at all requiring new baselines for a 3 hour window. Just because other teams do it some way does not mean that Chromium, with it's greater number of bots and platforms, should do it the same way. Yes, it does mean that we should do it the same way. What if non-Chromium ports started imposing arbitrary processes like this on the rest of us? It'll be a total chaos, and nobody would understand the right thing to do for all ports. We are discussing a process here, not code, and in my mind the goal is to have the tree be as green as possible with all failures tracked with a minimal expectations file and as little engineer time as possible. That's not our project goal. We have continuous builds and regression tests to prevent regressions to improve the stability, not to keep bots green. Please review http://www.webkit.org/projects/goals.html Just look at how often the non-chromium mac and win builds are red. In particular, changes submitted via the commit queue take an indeterminate amount of time to go in, anything from an hour to several hours. Patch authors do not necessarily even have control over when the CQ+ is given. That's why I don't use commit queue when I know my patch requires platform-dependent rebaselines. Even when manually committing, if it takes 3 hours to create baselines then no patches go in in the afternoon. What if the bots are down or misbehaving? We need to promptly fix those bots. I would also point out the waste of resources when every contributor needs to track every failure around commit time in order to know when their own changes cause failures, and then track the bots to know when they are free to go home. But that's clearly stated in the contribution guide line. Why not simply attach an owner and a resolution date to each expectation? The real problem right now is accountability and a way to remind people that they have left expectations hanging. That's what WebKit bugs are for. Ossy frequently files a bug and cc'es the patch author when a new test is added or a test starts failing and he doesn't know whether new result is correct or not. He also either skips the test or rebaseline the test as needed. He also reverts patches when the patch clearly introduced serious regressions (e.g. crashes on hundreds of tests). Yes, Ossy does an excellent job of gardening. Unfortunately, on Chrome we have tens if not hundreds of gardeners and, as this thread has revealed, no clear agreement on the best way to garden. That IS the problem. We have too many in-experiented gardeners that don't understand the WebKit culture or the
Re: [webkit-dev] Why does PositionIterator iterates until the end of the document when selecting?
One case where this matters: div
style=-webkit-user-select:nonefoo/divbar
If you mousedown on foo and drag right, you want to start selecting bar. In
the common case, we don't do any walking. The next position we grab from
the iterator is valid and we use it.
On Thu, Mar 29, 2012 at 7:27 AM, Sergiy Byelozyorov
byelozyo...@cs.uni-saarland.de wrote:
Hi,
When searching for the character to be selected (on mouse click), we run
an interator over the characters to determine the one under the cursor. I
am trying to understand why does PositionInterator that is used in this
case iterates over all characters starting from the element that was
clicked and until the end of the document(!). The following method is
called to verify whether PositionIterator has finished traversing the
characters (see comments inline):
bool PositionIterator::atEnd() const
{
if (!m_anchorNode) // This is clear - if we don't have an an anchor -
then we are done.
return true;
if (m_nodeAfterPositionInAnchor) // This is also understandable - if
there is a next position then we are not at the end.
return false;
// This is what puzzles me. First check will be true until we will
reach the root of the Document.
return !m_anchorNode-parentNode() (m_anchorNode-hasChildNodes()
|| m_offsetInAnchor = lastOffsetForEditing(m_anchorNode));
}
Is this the intended behaviour? Why wouldn't we just stay within the
element that was clicked on? This would save us a lot of CPU cycles because
we won't be checking text that in all other elements until the end of the
document, wouldn't it? This check has been around since at least 2004, so I
doub't that it's wrong, but I don't get the logic here. Please explain this
to me. Thanks.
Sergiy Byelozyorov
Computer Graphics Chair
Universitat des Saarlandes
Tel.: +49 (681) 302-3837
Web: http://graphics.cs.uni-saarland.de/sbyelozyorov/
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] PSA: Chromium layout test fallbacks AKA down with graphs, long live trees
Sometime this week or next, we plan to change the Chromium layout test fallback order. We'll be changing it so all chromium ports fallback to platform/chromium, then to platform/mac. They will never fallback to platform/win or platform/mac-* as they do today. The benefits of this change are that it makes the fallback order much easier to reason about for both human beings and code. I believe it makes the fallback order for all WebKit ports a tree instead of a complex graph. This will have minimal impact on the number of expected result files in the tree since only a few hundred chromium tests currently fallback to platform/win and/or platform/mac-*. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] CSS3 Selectors3 test suite
I've recently been greening Chromium's expectations for css3/selectors3. ~10% of these test need interaction (e.g. hovering over a link or selecting text). Given that this is an imported test suite does it make sense to add the appropriate layoutTestController hooks? As it is, the tests aren't really verifying correctness. Also, this test suite is a great example of one that I think it would make more sense for us to check in reftest expected results instead of png+rendertreedump. These tests are explicitly testing selector matching only, so it would be easy to write reftests that have a high confidence of accurately verifying correctness. They'd only fail if we had some egregious bug such that we painted everything green as white. Presumably we'd notice such a bug through other means. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Please set the svn:mime-type property on binary files before committing
Whoops. A lot of these were from me yesterday. Sorry, I didn't realize I didn't have my subversion config set correctly. I went to fix up my commits from yesterday and realized that a very large percentage of the pngs in the LayoutTests tree have the wrong svn:mime-type. Is anyone opposed to me doing a bulk fix for all the pngs? find LayoutTests | grep \.png$ | grep -v \.svn | xargs svn ps svn:mime-type image/png On Thu, Mar 8, 2012 at 1:52 AM, Ashod Nakashian ashodnakash...@yahoo.comwrote: Please let's also enforce svn:eol-style to LF for all scripts as this is breaking (at least) the bash scripts that are checked-out with native style on Windows. Some bash versions don't like CR. Please see a (failed) attempt at fixing this manually[1]. I also believe we should mark all (Bash/Perl/Python) scripts as executable. It's best to at least automate it, if not also check for violations via check-webkit-style. (And for the loving of all that's good, would someone please help with this bug[1]?) [1] https://bugs.webkit.org/show_bug.cgi?id=79509 -Ash -- *From:* Simon Fraser simon.fra...@apple.com *To:* Eric Seidel e...@webkit.org *Cc:* WebKit Development webkit-dev@lists.webkit.org *Sent:* Thursday, March 8, 2012 3:37 AM *Subject:* Re: [webkit-dev] Please set the svn:mime-type property on binary files before committing The best way to enforce it would be with a pre-commit hook: https://bugs.webkit.org/show_bug.cgi?id=80548 Simon On Mar 7, 2012, at 3:33 PM, Eric Seidel wrote: Unless this is enforced by a tool, it's very likely to be forgotten. https://bugs.webkit.org/show_bug.cgi?id=75824 https://bugs.webkit.org/show_bug.cgi?id=75825 On Wed, Mar 7, 2012 at 3:21 PM, Dan Bernstein m...@apple.com wrote: Please set the svn:mime-type property on binary files that you add to the tree, such as *-expected.png, before committing. Otherwise the resulting webkit-changes message will include those files as text, which is inconvenient. Thanks. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Is converting pixel tests to reftests kosher for imported libraries?
I just did a first pass a greening the Chromium Lion bot: http://trac.webkit.org/changeset/110096. Of these hundreds of tests, ~99% of them are perfect candidates for being reftests (e.g. they contain one line of text and a solid box or two under the text), but most of them are in the CSS imported test suites. Is it kosher to convert them to reftests or should we leave pixel tests from imported test suites alone? ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is converting pixel tests to reftests kosher for imported libraries?
That hadn't occurred to me. You're right, we wouldn't actually modify the test itself. We would just replace the -expected.txt/png with a -expected.html file. Maciej, does that change your opinion on this? On Wed, Mar 7, 2012 at 4:44 PM, Darin Fisher da...@chromium.org wrote: Hrm, if the test expectations are customized already for different ports of WebKit, then why not support replacing a PNG file with a HTML file that is intended to generate exactly the same result? How does this impair our ability to update the tests? (I realize that our current reftest system may not work like this. I'm not familiar with the details of how it works in fact, but it seems like it could be as simple as having an expected result that is a HTML file instead of a PNG file.) -Darin On Wed, Mar 7, 2012 at 4:10 PM, Maciej Stachowiak m...@apple.com wrote: I'd prefer we not modify imported test suites. That will just make it more confusing to update. Perhaps future CSS test suites will be changed to a reftest model. Regards, Maciej On Mar 7, 2012, at 1:41 PM, Ojan Vafai wrote: I just did a first pass a greening the Chromium Lion bot: http://trac.webkit.org/changeset/110096. Of these hundreds of tests, ~99% of them are perfect candidates for being reftests (e.g. they contain one line of text and a solid box or two under the text), but most of them are in the CSS imported test suites. Is it kosher to convert them to reftests or should we leave pixel tests from imported test suites alone? ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Is converting pixel tests to reftests kosher for imported libraries?
On Wed, Mar 7, 2012 at 4:50 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Mar 7, 2012 at 4:44 PM, Darin Fisher da...@chromium.org wrote: Hrm, if the test expectations are customized already for different ports of WebKit, then why not support replacing a PNG file with a HTML file that is intended to generate exactly the same result? How does this impair our ability to update the tests? (I realize that our current reftest system may not work like this. I'm not familiar with the details of how it works in fact, but it seems like it could be as simple as having an expected result that is a HTML file instead of a PNG file.) How do we know that we are testing what the test is intending to test after the conversion? e.g. it's possible to create a reference file that fails to catch certain bugs. It's not obvious to me how one would figure out how many reference files are needed for a given test to make sure we're not making the test more permissible than the author intended it to be. I'm not suggesting that we convert 100% of these tests to reftests, but a very very large percentage of them can easily be verified to be testing the correct thing and would only need a single reference file (it's just a line of text with a colored box in it after all). ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] scoping Node destruction during DOM modifications
We have a lot of code (e.g. in ContainerNode.cpp or any of the editing code) that needs to RefPtr nodes to make sure they're not destroyed due to synchronous events like blur, mutation events, etc. For example, ContainerNode::removeChild needs to RefPtr the parent to make it's not destroyed during a sync event. Currently, we need to write careful code that knows whether any methods it's calling can fire sync events and RefPtrs all the appropriate nodes. I have a proposal to automate this in https://bugs.webkit.org/show_bug.cgi?id=80041. It certainly makes the code cleaner and safer, but it comes at a performance cost in some cases. Basically, any scope that needs to be careful of sync events adds a DOMRemovalScope at the top which keeps nodes from getting destroyed until the scope is destroyed (DOMRemovalScope adds them to a VectorRefPtrNode). In cases where we don't delete nodes, there's no performance impact. In cases where we delete nodes, this is always slower. I uploaded two performance tests to that bug: 1. Set innerHTML = on a node that has half a million children. This test goes from ~166ms to ~172ms on my machine. A 3.6% regression. 2. Destroy half a million nodes *during* a synchronous event (uses range.deleteContents). Goes from ~284ms to ~342ms. A 20% regression. So destroying Nodes during synchronous events fired during a DOM mutation is significantly slower. This case is rare though. I had to think hard to come up with a case where we would hit this. For the most part, nodes don't actually get destroyed due to JS until a garbage collection occurs, which is usually after the event has completed and the DOMRemovalScope has been destroyed. The advantage of this is that it gives a simple way to address a common class of crash and potentially security bugs. The disadvantage of course is the possible performance hit. What do you think? Is this worth trying? Are there better ideas? Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] scoping Node destruction during DOM modifications
I think my earlier testing was faulty. Now when I test case 2, I get something comparable with and without the patch. If there is a regression, it's below the noise. Running it through a profiler shows a negligible amount of time in the new code. I had tried running it through Dromaeo first, but any performance impact (if there is any) was well below the variance. I can take a stab at running Peacekeeper and Acid3 tomorrow, but I don't have high hopes of getting useful information out of them. My hope with the microbenchmarks was to show worst-case behavior. I expect that in practice this will have no performance impact in the real world. That's a hard statement to prove though. I'll see if I can reduce the variance in my microbenchmarks. On Thu, Mar 1, 2012 at 5:27 PM, Maciej Stachowiak m...@apple.com wrote: I agree with Adam's remarks. The safety benefit seems great, but we should investigate ways to get it at less performance cost (ideally no measurable cost). I'm also curious what impact this change has on less micro- but still DOM-oriented benchmarks, such as Dromaeo's DOM tests, Peacekeeper, or the Acid3 secret hidden perf test. I think all of those entail some DOM node destruction although perhaps they do not hit this pattern at all. Regards, Maciej On Mar 1, 2012, at 5:06 PM, Adam Barth wrote: Do we understand what's causing the performance regression? For example, there are other implementation approaches where we try to transfer the last ref rather than churning it or where we could use a free list rather than a vector. I just wonder if there's a way to get the benefits with a lower cost. Adam On Thu, Mar 1, 2012 at 4:50 PM, Ojan Vafai o...@chromium.org wrote: We have a lot of code (e.g. in ContainerNode.cpp or any of the editing code) that needs to RefPtr nodes to make sure they're not destroyed due to synchronous events like blur, mutation events, etc. For example, ContainerNode::removeChild needs to RefPtr the parent to make it's not destroyed during a sync event. Currently, we need to write careful code that knows whether any methods it's calling can fire sync events and RefPtrs all the appropriate nodes. I have a proposal to automate this in https://bugs.webkit.org/show_bug.cgi?id=80041. It certainly makes the code cleaner and safer, but it comes at a performance cost in some cases. Basically, any scope that needs to be careful of sync events adds a DOMRemovalScope at the top which keeps nodes from getting destroyed until the scope is destroyed (DOMRemovalScope adds them to a VectorRefPtrNode). In cases where we don't delete nodes, there's no performance impact. In cases where we delete nodes, this is always slower. I uploaded two performance tests to that bug: 1. Set innerHTML = on a node that has half a million children. This test goes from ~166ms to ~172ms on my machine. A 3.6% regression. 2. Destroy half a million nodes *during* a synchronous event (uses range.deleteContents). Goes from ~284ms to ~342ms. A 20% regression. So destroying Nodes during synchronous events fired during a DOM mutation is significantly slower. This case is rare though. I had to think hard to come up with a case where we would hit this. For the most part, nodes don't actually get destroyed due to JS until a garbage collection occurs, which is usually after the event has completed and the DOMRemovalScope has been destroyed. The advantage of this is that it gives a simple way to address a common class of crash and potentially security bugs. The disadvantage of course is the possible performance hit. What do you think? Is this worth trying? Are there better ideas? Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] (no subject)
On Wed, Feb 29, 2012 at 12:43 AM, Sergio Villar Senin svil...@igalia.comwrote: En 29/02/12 09:33, Konstantin Tokarev escribiu: Although I normally use it for cherry-picking a commit to upload, I have always missed the option to upload a bunch of commits as a single patch. Basically, as you said, forcing people to merge several commits in a single one to upload a patch to bz totally breaks the typical git workflow (micro-commits and so). Do you know how to use git rebase -i? Konstantin, that's why I meant with merge several commits in a single one. You do not normally want to do that while you're developing a patch as having multiple commits gives you a lot of flexibility while developing. I normally have to create a new branch to rebase the commits I want in a single patch to upload it to the bz. That is annoying, that's why I said that having something like webkit-patch upload range_of_commits will be nice to have, as you wouldn't have to create a new branch and rebase several commits, just to upload a new patch to the bz. You can do this with the current -g option by adding a commit range, e.g. -g=commit1..commit2. AFAIK, the only thing you can't do currently with -g is pass a commit range *and* include the staged/working copy changes. Under the hood it basically does what you described (create a new branch, copy the commits over as a single commit, upload from the branch, etc). ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] (no subject)
On Mon, Feb 27, 2012 at 5:56 PM, Dirk Pranke dpra...@chromium.org wrote: Hi all, If you don't use webkit-patch and Git, you can stop reading now. Otherwise ... Currently, webkit-patch -g has some special logic for figuring out what to diff against for Git checkouts. Specifically, webkit-patch -g commitish gets translated to the git equivalent of 'git diff commitish^..commitish'. Then, we have an additional tweak that rewrites '-g HEAD' to '-g HEAD..' in order to pick up any uncommitted changes, and if nothing is specified, we will attempt to diff against the remote master/trunk version. This is very useful if you typically want to just upload a single commit issue, but is a bit un-git-like, and actually thwarts some other use cases. My questions are: 1) Do you use -g foo to upload a single change? If so, would you be annoyed if I changed that syntax to a different argument, or eliminated it completely (so that you would have to type foo^..foo)? 2) Do you object to changing the default to match what 'git diff' does? This would change the defaults so that: a) instead of no arguments meaning diff against remote master, it would mean diff against what is staged for commit I'd rather this not change. It used to work like this and there was much happiness when it changed to the current scheme. I think a lot of people's workflows depend on the no argument case uploading all the changes in their branch. b) 'git diff commitish would show the diff between commitish and your current working directory Now that I think about it, I realize that this doesn't really work without also changing (a). Also, there seem to be at least a few people who expect -g to work like cherry-pick. I'm beginning to think it probably makes sense to add a different commandline argument that works exactly like diff (e.g. takes an optional second value). --git-diff perhaps? If there is a consensus that we shouldn't change the defaults, I will likely implement a different command line argument that does mirror what git does. As an aside, I will probably be adding a patch that will figure out what the 'tracking' branch (as defined by branch.branchname.merge) is for your current checkout, and give webkit-patch a way to use that instead of the remote head (this would make using stacked local branches much easier). I haven't decided if this should be the default or if you should have to request this via something like 'webkit-patch diff -g UPSTREAM' instead; if you have an opinion, feel free to comment. There is a bug tracking this work at https://bugs.webkit.org/show_bug.cgi?id=76958 if you want to comment there instead of here or follow along with whatever ends up happening. Thanks, -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Removing HTML notifications from WebKit (Was: Web Notifications API)
This seems fine to me. The most important thing here is standardizing what we ship to the web platform. On Thu, Feb 16, 2012 at 4:44 PM, Aaron Boodman a...@chromium.org wrote: Hello again, I'd like to revise my proposal on deprecating HTML notifications in Chrome extensions: I would like to make the feature runtime-enabled, and leave it enabled for Chrome extensions until there is a suitable replacement. Once we have a replacement, we can follow the deprecation plan I sketched earlier. It sounds like the Chrome web platform team is more OK with the idea of removing it from web content. Having the runtime switch would make it easy to implement both policies. I understand that having this code is a tax on the WebKit project, but it seems pretty small compared to other things that have been left in for other clients. - a On Mon, Feb 13, 2012 at 11:23 AM, Maciej Stachowiak m...@apple.com wrote: This plan sounds reasonable to me. No disruption of Chrome extensions in the short term, but we would better align with each other and with standards in the longer term. Jon? Regards, Maciej On Feb 9, 2012, at 2:48 PM, Aaron Boodman wrote: On Wed, Feb 8, 2012 at 7:50 PM, Maciej Stachowiak m...@apple.com wrote: On Feb 8, 2012, at 6:15 PM, Aaron Boodman wrote: On Wed, Feb 8, 2012 at 4:58 PM, Jon Lee jon...@apple.com wrote: 2. Remove HTML notifications. It has been removed from the spec, and we don't intend on ever supporting HTML notifications. I brought this issue up before; is there an update on this front from any other platforms? HTML notifications are a pretty popular feature in Chrome's extension system. As of a few months ago: * 614 extensions in our web store use createHTMLNotification * 72 have more than 10k users * 14 have more than 100k users * 3 have more than 500k users * 6.7M total actextension installs We can move developers off of this API, but not overnight. Is it possible to come up with a slower deprecation plan than immediately? Since HTML notifications are already under a separate feature flag, it's probably practical to keep them around for a while, and just not include them in the proposed updated API. Do you have a suggestion for what might be a reasonable deprecation timeline? Awesome. Here is a rough plan of how deprecation could work: 0 months: Add a new extension format version and remove HTML notifications support with that version. 2 months: Support for the new format version is in Chrome's stable channel. The documentation advises the new format version, and new features require it. 4 months: We start requiring the new manifest version for new extensions uploaded to the store. 8 months: We start requiring the new manifest version for updates to existing extensions in the store. 10 months: Remove support for the old manifest version from trunk of Chrome. I believe at this point, we can remove the code from WebKit. We've never actually done this before though, so there may be some hiccups. I'd plan on about a year. We have already started a new manifest format version for Chrome 18, hopefully I can squeeze this change into that release. On Wed, Feb 8, 2012 at 9:24 PM, Adam Barth aba...@webkit.org wrote: Unrelated to timeline, it might be worthwhile to make createHTMLNotification a runtime-enabled feature so that we can avoid offering it to the web at large and possibly restrict it to only a whitelisted set of extensions. This is a very good idea. - a ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] test_expectations.txt for non-chromium ports
On Mon, Feb 13, 2012 at 1:06 PM, Maciej Stachowiak m...@apple.com wrote: On Feb 10, 2012, at 4:20 PM, Dirk Pranke wrote: I think at one point Adam indicated he wanted to use them for the Apple Win port, but he is still using the Skipped files since the Win port is still using ORWT on the bots. That said, I understand why you're asking this (I think), but I would prefer that we not have to support both options indefinitely into the future. I would like to merge whatever features we need from both approaches into one solution if possible (of course, we can do that after forcing ports to use only one or the other for now). I agree that combining the features of both approach is the best long-term solution. I haven't checked with others who have an interest in the ports maintained by Apple, but I at least like different aspects of the two systems and would love to settle on a unified solution. Sounds good to me if there is someone willing to find consensus on a final design and actually implement it. The last time this was discussed at any length, people seemed to want expectations to cascade, but I never got any clear feedback on what the semantics of the cascade would be, and how that might interact with different modifiers in the test_expectations file. Since that time, I've come to believe that even the way Chromium uses expectations files is just making things harder for developers and maintainers, and so I would like to change how Chromium does things as well ... this is a long-winded way of saying most things are on the table for discussion. For example, we might want to use only Skipped files for tests that are always planned to be skipped, and test_expectations for things that are supposed to be temporary workarounds to keep the tree green until bugs can be filed and new baselines generated. Or, we might want to do something else ... I'd much prefer a single file, but perhaps with different status and uses of states than the current test_expectations.txt format. I think agreeing on the list of states and whether to have a single file or cascading files are the two controversial points. While (if?) we figure this out, it seems better to me if we do the simple work of only allowing either of a test_expectations.txt file or Skipped lists. Until someone does the work of unifying the two systems, it will continue to be a source of confusion to allow both for a given port. I do agree that distinguishing test not applicable to this port from this test is temporarily failing for unknown reasons is a good thing to do. It is unfortunate that we don't make the distinction very well right now. test_expectations.txt has a WONTFIX modifier for this purpose. Chromium used to have two files test_expectations.txt and test_expectations_wontfix.txt instead of having the modifier. I would kind of like us to move back to that model because then test_expectations.txt is a file that you hope to keep completely empty and test_expectations_wontfix.txt is a file that your rarely touch. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] test_expectations.txt for non-chromium ports
On Mon, Feb 13, 2012 at 3:09 PM, Maciej Stachowiak m...@apple.com wrote: On Feb 13, 2012, at 1:40 PM, Ojan Vafai wrote: On Mon, Feb 13, 2012 at 1:06 PM, Maciej Stachowiak m...@apple.com wrote: On Feb 10, 2012, at 4:20 PM, Dirk Pranke wrote: For example, we might want to use only Skipped files for tests that are always planned to be skipped, and test_expectations for things that are supposed to be temporary workarounds to keep the tree green until bugs can be filed and new baselines generated. Or, we might want to do something else ... I'd much prefer a single file, but perhaps with different status and uses of states than the current test_expectations.txt format. I think agreeing on the list of states and whether to have a single file or cascading files are the two controversial points. While (if?) we figure this out, it seems better to me if we do the simple work of only allowing either of a test_expectations.txt file or Skipped lists. Until someone does the work of unifying the two systems, it will continue to be a source of confusion to allow both for a given port. This seems like mainly an issue for ports that have both files present. If only one is actually used, then this makes it a low-stakes change. If there's any port actually applying both, then this may be a nontrivial change, and may not be worth doing until we figure out our longer-term plan. I do agree that distinguishing test not applicable to this port from this test is temporarily failing for unknown reasons is a good thing to do. It is unfortunate that we don't make the distinction very well right now. test_expectations.txt has a WONTFIX modifier for this purpose. Chromium used to have two files test_expectations.txt and test_expectations_wontfix.txt instead of having the modifier. I would kind of like us to move back to that model because then test_expectations.txt is a file that you hope to keep completely empty and test_expectations_wontfix.txt is a file that your rarely touch. It's good that there is a state for this, but WONTFIX doesn't seem like a great name to me, at least for tests that are inapplicable due to missing features. It implies both that the missing feature is by definition a bug, and also that the decision will not be reconsidered. Granted, this may be bikeshedding, but if I were, say, disabling tests for Apple's port that relate to the new WebSockets protocol because we don't have it on yet, I would be very reluctant to mark them WONTFIX. For tests that are inapplicable for reasons other than missing features, it may be simply that there is more than one acceptable behavior, in which case WONTFIX seems wrong as well. The intention of WONTFIX is exactly that the decision likely won't need to be reconsidered (e.g. because the test is platform specific). For the other purposes (e.g. websockets), we use SKIP. I actually believe we should rename WONTFIX to NEVERFIX to make it even more clear. Ojan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Subpixel Layout Update
On Fri, Feb 10, 2012 at 6:14 PM, Adam Barth aba...@webkit.org wrote: On Fri, Feb 10, 2012 at 6:09 PM, Levi Weintraub le...@google.com wrote: On Fri, Feb 10, 2012 at 6:03 PM, Adam Barth aba...@webkit.org wrote: On Fri, Feb 10, 2012 at 5:49 PM, Levi Weintraub le...@google.com wrote: WebKittens, We're planning to wrap up our conversion of the RenderTree to subpixel units next week. We've created a the following wiki page to help explain the changes we are making: https://trac.webkit.org/wiki/LayoutUnit If you work on the rendering code, please take a look and talk to Emil and me if you have any questions. This will effect a large number of layout test expectations as well as some platform interfaces. If you are working on or maintaining a port other than Apple, Qt, and Chromium, please touch bases with us. We've been talking about turning on mock scroll bars for Chromium. Would it make sense to do that at the same time to minimize the number of baseline updates? It would allow us to make only one WebKit sheriff's life miserable instead of two, but the changes don't really relate in any other meaningful way. I meant it would let us update the PNGs once instead of twice. I don't think coupling these two is worth the benefits. It makes getting both changes checked in harder and makes it harder to identify regressions due to either patch. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
