Re: [webkit-dev] Thank You

[Ojan Vafai]

On Wed, Apr 3, 2013 at 2:02 PM, Eric Seidel  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)

[Ojan Vafai]

On Sat, Feb 9, 2013 at 7:41 PM, Maciej Stachowiak  wrote:

>
> Hi Peter,
>
> On Feb 9, 2013, at 3:48 PM, Peter Kasting  wrote:
>
> On Sat, Feb 9, 2013 at 11:52 AM, Maciej Stachowiak  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

Re: [webkit-dev] PSA: review tool dropping comments

[Ojan Vafai]

Should be fixed. Let me know if you see problems.


On Wed, Jan 2, 2013 at 11:41 AM, Ojan Vafai  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

[webkit-dev] PSA: review tool dropping comments

[Ojan Vafai]

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

[Ojan Vafai]

On Tue, Dec 11, 2012 at 1:11 PM, Emil A Eklund  wrote:

> On Tue, Dec 11, 2012 at 12:19 PM, Ojan Vafai  wrote:
> > On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund 
> 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] Please avoid rolling out patches speculatively and reland them ASAP if you had to

[Ojan Vafai]

On Tue, Dec 11, 2012 at 12:17 PM, Emil A Eklund  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] Adding element to WebCore

[Ojan Vafai]

On Tue, Nov 27, 2012 at 1:47 PM, Ojan Vafai  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  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 
> wrote:
>
>> Hi all,
>>
>> this is my first post to the list.
>>
>> I have submitted a patch [1] to add   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 element to WebCore

[Ojan Vafai]

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 wrote:

> Hi all,
>
> this is my first post to the list.
>
> I have submitted a patch [1] to add   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
>
>  
>
> ___
> 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] RenderArena: Teaching an old dog new tricks

[Ojan Vafai]

On Wed, Nov 14, 2012 at 2:48 PM, Geoffrey Garen  wrote:

> Hi Eric.
>
> Here are some problems in RenderArena that I know of:
>
> - Grows memory without bound
> - Duplicates the functionality of FastMalloc
> - Makes allocation error-prone (allocation in the wrong arena is sometimes
> a leak, sometimes a use-after-free, and sometimes a heizenbug of the two)
> - Makes allocation verbose (you have to thread an arena pointer everywhere)
> - Makes object lifetime complicated (all objects are implicitly tied to a
> single owner they may outlive)
> - Uses C-style macros and manual initialization and destruction, instead
> of modern WebKit C++ style
>
> You didn't mention any of these problems in your email, so I'll assume you
> weren't aware of them.
>
> Considering these problems now, please don't use RenderArena in more
> places.
>
> > Slab-allocators (i.e. RenderArena) hand out memory all from a single
> > region, guaranteeing (among other things) that free'd objects can only
> > be ever overwritten by other objects from the same pool.  This makes
> > it much harder, for example to find a Use-After-Free of a RenderBlock
> > and then fill that RenderBlock's memory (and vtable pointer) with
> > arbitrary memory (like the contents of a javascript array).
> > http://en.wikipedia.org/wiki/Slab_allocation
>
> This is magical thinking. RenderArena is no different from FastMalloc.
>
> (1) RenderArena recycles by object size, just like FastMalloc.
>
> (2) FastMalloc is a slab allocator, just like RenderArena.
>
> (3) RenderArena grows by calling FastMalloc.
>
> Isolating object types from each other -- and specifically isolating
> objects of arbitrary size and contents like arrays -- is an interesting
> idea. RenderArena is neither necessary nor sufficient for implementing this
> feature.
>
> The only reason RenderArena seems isolated from other object types is
> social, not technical: we actively discourage using RenderArena, so few
> object types currently use it.
>
> > Since RenderArena is generic, the current plan to move it to WTF (as
> > by Chris Marrin suggested back in
> > http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg12672.html),
> > clean up the code further, and investigate wider deployment (like to
> > the DOM tree) for the security benefit and possible perf win.
> > https://bugs.webkit.org/show_bug.cgi?id=101087
>
> Having dealt with the specific technical question of RenderArena, I'd like
> to briefly discuss the meta-level of how the WebKit project works.
>
> Sam Weinig and I both provided review feedback saying that using
> RenderArena more was a bad idea 
> (https://bugs.webkit.org/show_bug.cgi?id

*This seems completely unfair *

*
> *

*Geoff*

*
> *

*___*

*webkit-dev mailing list*

*webkit-dev@lists.webkit.org*

*http://lists.webkit.org/mailman/listinfo/webkit-dev*

=101087#c9 ,
> https://bugs.webkit.org/show_bug.cgi?id=101087#c18). Nonetheless, you
> r+ed a patch to move in that direction, and you describe it here as the
> "current plan" for WebKit.
>
> I'm a little disappointed that, as individual contributors, Chris Neklar
> and Chris Evans didn't realize or understand the problems listed above, and
> didn't tackle them. However, the mistake is understandable: Chris and Chris
> are new to WebKit. The WebKit project has a mechanism for resolving
> mistakes like this: patch review.
>
> Your job as a reviewer is to understand the zeitgeist of the project, to
> use good judgement, and to r- patches that make mistakes like this. A bad
> patch is only a small nuisance. But the small nuisance turns into a major
> problem when you, as a reviewer, take a bad patch, mark it r+, and declare
> it the current direction of the project, despite the objections of two
> other reviewers who are senior members of the project.
>

As someone outside all these discussions, this seems like a completely
unfair characterization of what happened. Sam voiced an objection, then
there was a bunch of discussion in which Chris made an argument that Eric
found compelling. Many days passed, then Eric r+'ed. Unless there was other
discussion not on the bug that I missed, your objection came after Eric's
r+. I don't see the process problem here.
___
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?

[Ojan Vafai]

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  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

[Ojan Vafai]

On Thu, Nov 8, 2012 at 2:00 AM, Žan Doberšek  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 wrote:
>
>>  On 11/08/2012 02:51 AM, Ojan Vafai wrote:
>>
>>  On Wed, Nov 7, 2012 at 12:41 PM, Dirk Pranke wrote:
>>
>>>  On Tue, Nov 6, 2012 at 11:58 PM, Žan Doberšek 
>>> 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 ha

Re: [webkit-dev] The --test-list option behavior in NRWT

[Ojan Vafai]

On Wed, Nov 7, 2012 at 12:41 PM, Dirk Pranke  wrote:

> On Tue, Nov 6, 2012 at 11:58 PM, Žan Doberšek 
> 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.


> I would also be fine changing the code so that order is preserved for
> tests passed in on the command line as well. (e.g. so
> "run-webkit-tests foo.html bar.html" runs them in that order).
>
> I think it would be good to keep the behavior consistent across the
> two methods of specifying names.
>
> -- 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] Does anyone still use the TestFailures app?

[Ojan Vafai]

I agree. The question is what uses does TestFailures support that
garden-o-matic doesn't? I haven't used it much, so I don't have a sense of
what features garden-o-matic is missing.

The flakiness dashboard isn't really meant for figuring out what tests are
currently failing. But, if you have a specific test you want to quickly
lookup, you can append the test name to
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=.
It's been on my TODO list for a while to make it by default load a simpler
page that lets you just put in a test name instead of loading the data for
a random bot.


On Mon, Nov 5, 2012 at 12:52 PM, Simon Fraser wrote:

> I use it from time to time, but I think we really need merge it with
> garden-o-matic; they do much of the same stuff.
>
> Simon
>
> On Nov 5, 2012, at 12:21 PM, Dirk Pranke  wrote:
>
> > http://build.webkit.org/TestFailures/
> >
> > I think Adam Roben was working on this a year or so ago. It appears to
> > be broken at the moment (it's likely that I broke it, in fact), but
> > before I spend much time fixing it I thought I'd check.
> >
> > I've never actually used it myself, so I'm not sure what all it was
> > supposed to do; it looks like it overlaps in functionality some with
> > the flakiness dashboard, but was probably written before the flakiness
> > dashboard worked with the build.webkit.org bots and everyone was
> > converted to using NRWT.
> >
> > If anyone is still using it (or would if it was actually working) in
> > preference to the flakiness dashboard, can you let me know why?
> > Ideally I'd like to get rid of it and roll any good features it had
> > into the flakiness dashboard, but I'm happy to fix it and/or keep it
> > around if it does other things I'm not aware of or if people are still
> > using it.
>
> ___
> 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

[Ojan Vafai]

On Thu, Nov 1, 2012 at 12:32 PM, Peter Kasting wrote:

> 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/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

Re: [webkit-dev] moving focus when clicking on scrollbars

[Ojan Vafai]

On Thu, Nov 1, 2012 at 10:13 AM, Elliott Sprehn wrote:

> On Thu, Nov 1, 2012 at 4:42 AM, Maciej Stachowiak  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

[Ojan Vafai]

On Wed, Oct 31, 2012 at 3:14 PM, Peter Kasting wrote:

> On Wed, Oct 31, 2012 at 2:40 PM, Ojan Vafai  wrote:
>
>>  On Wed, Oct 31, 2012 at 2:29 PM, Peter Kasting wrote:
>>
>>> 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] moving focus when clicking on scrollbars

[Ojan Vafai]

On Wed, Oct 31, 2012 at 2:29 PM, Peter Kasting wrote:

> On Wed, Oct 31, 2012 at 1:32 PM, Ojan Vafai  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

[webkit-dev] moving focus when clicking on scrollbars

[Ojan Vafai]

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] WebKit & DOM 3 Events

[Ojan Vafai]

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  wrote:

> On Thu, Oct 18, 2012 at 7:00 PM, Wez  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

[Ojan Vafai]

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  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  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 wrote:
>>
>>> 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

Re: [webkit-dev] On returning mutable pointers from const methods

[Ojan Vafai]

On Thu, Oct 25, 2012 at 5:04 AM, Stephen Chenney wrote:

> I second the principle. I ran into this kind of code pattern recently in
> fonts and was unnerved by the thought of a mutable pointer emerging from a
> const method.
>
> I am not at all certain that we can simply make the change without some
> heavy refactoring. On the one hand, there are some long const-ness chains
> in some places that removing const from a method will break. On the other
> hand, making the returned pointer const in some places will force
> additional const-ness or require changes to code that uses the pointer.
>
> Do we try to apply the new style to new code only?
>

This is typically how we handle all style rules. They apply to new code and
to code you are modifying, but there's no expectation to clean up old code
just for the sake of meeting the style rule. Although, I think in this
case, if someone felt moved, it might be acceptable since this isn't just a
readability issue and will likely uncover real bugs.


> Stephen.
>
> On Thu, Oct 25, 2012 at 6:48 AM, Andreas Kling  wrote:
>
>> Yo WebKittens!
>>
>> After some mild morning discussion in #webkit, I'm wondering if we should
>> amend our style guide to disallow returning mutable pointers (Foo*) from
>> const methods, like so:
>>
>> - Foo* foo() const;
>>
>> While this is useful when you want to be able to take a strong reference
>> to the returned object, i.e by assigning it to a RefPtr, it's
>> counter-intuitive as it encourages "read-only" methods to hand out
>> "read/write" references to internal data.
>>
>> I've been using const-ness to prevent certain classes of bugs at compile
>> time for the immutable/mutable implementations of WebCore's
>> ElementAttributeData and StylePropertySet. It's especially important that
>> the immutable versions of these objects don't have their internal data
>> modified by unwitting external clients, since they may be shared between
>> multiple owners. While using assertions helps us catch incorrect use of
>> these objects, it's obviously optimal to catch the bugs already at compile
>> time.
>>
>> I just now discovered that you can grab at a mutable CSSValue* pointer in
>> an immutable "const StylePropertySet*" by going through
>> propertyAt(index).value(), since we have "CSSValue* CSSProperty::value()
>> const". This is bad, because it means that we have a bunch of call sites
>> operating on mutable pointers into what's supposed to be an immutable
>> object!
>>
>> While I haven't identified any real bugs caused by this (yet), it would
>> be easy to introduce one by mistake.
>>
>> So, I propose that we allow only these two signature formats for raw
>> pointers:
>>
>> - const Foo* foo() const;
>> - Foo* foo();
>>
>> Moreover, for methods that return references to objects where call sites
>> are expected to take a strong reference, we should really be using
>> PassRefPtr as the return type.
>>
>> Thoughts? Objections? Am I missing something?
>>
>> -Kling
>>
>> ___
>> 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] WebKit & DOM 3 Events

[Ojan Vafai]

On Thu, Oct 18, 2012 at 7:00 PM, Wez  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

[webkit-dev] FYI: change to fake mouse events and hover updating during scroll

[Ojan Vafai]

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?

[Ojan Vafai]

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

Re: [webkit-dev] Tips on why run-webkit-tests is not working for me?

[Ojan Vafai]

On Wed, Oct 3, 2012 at 9:05 AM, Darin Adler  wrote:

> I’m trying to test some changes on my Mountain Lion Mac, using the Mac
> port. When I call run-webkit-tests it fails with file not found.
>
> Using port 'mac-mountainlion'
> Test configuration: 
> Placing test results in /Users/darin/Build/Debug/layout-test-results
> Baseline search path: mac -> generic
> Using Debug build
> Pixel tests disabled
> Regular timeout: 35000, slow test timeout: 175000
> Command line: /Users/darin/Build/Debug/DumpRenderTree -
>
> Found 31448 tests; running 29263, skipping 2185.
> Checking build ...
> OSError raised: [Errno 2] No such file or directory
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py",
> line 110, in run
>   unexpected_result_count = manager.run(args)
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py",
> line 404, in run
>   if not self._set_up_run():
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py",
> line 355, in _set_up_run
>   if not self._port.check_build(self.needs_servers()):
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/port/base.py",
> line 240, in check_build
>   if not self._check_port_build():
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/port/mac.py",
> line 162, in _check_port_build
>   return self._build_java_test_support()
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/layout_tests/port/mac.py",
> line 136, in _build_java_test_support
>   if self._executive.run_command(build_java, return_exit_code=True):
>  # Paths are absolute, so we don't need to set a cwd.
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/common/system/executive.py",
> line 402, in run_command
>   close_fds=self._should_close_fds())
> File
> "/Users/darin/Safari/OpenSource/Tools/Scripts/webkitpy/common/system/executive.py",
> line 458, in popen
>   return subprocess.Popen(*args, **kwargs)
> File
> "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
> line 679, in __init__
>   errread, errwrite)
> File
> "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
> line 1228, in _execute_child
>   raise child_exception
> Failed to execute Tools/Scripts/new-run-webkit-tests at
> /Users/darin/Safari/Internal/Tools/Scripts/../../../OpenSource/Tools/Scripts/run-webkit-tests
> line 126.
>
> Anyone know why?
>

Haven't seen this before. I'm guessing there's something about your
checkout that is confusing the code that finds the path to
run-webkit-tests. Does
/Users/darin/Safari/Internal/Tools/Scripts/../../../OpenSource/Tools/Scripts/run-webkit-tests
actually exist or is that path incorrect?


> -- Darin
>
> ___
> 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: Chromium Lion layout test bots are now 10.7.5

[Ojan Vafai]

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.

[Ojan Vafai]

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  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  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  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=166300&action=prettypatch
>>> [3]: https://bugs.webkit.org/show_bug.cgi?id=97984
>>>
>>> On Wed, Sep 26, 2012 at 5:12 PM, Mike West  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<--
>>> > 
>>> > 
>>> >   
>>> > Bug 152212
>>> > 
>>> >   
>>> >   
>>> > 
>>> >   var xhr = ne

Re: [webkit-dev] Bug numbers in TestExpectations

[Ojan Vafai]

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  wrote:

> I support making bug URL or Bug(~) optional.
>
> On Thu, Sep 20, 2012 at 10:34 AM, Alexey Proskuryakov wrote:
>
>>
>> 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] Should we close the tree? (was: Re: the new TestExpectations syntax is landing soon)

[Ojan Vafai]

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  wrote:

> I'd prefer to see the patch rolled out.
>
> Geoff
>
> On Sep 20, 2012, at 11:07 AM, Alexey Proskuryakov  wrote:
>
> >
> > Now tracked as .
> >
> > 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  написал(а):
> >
> >> 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 
> 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

Re: [webkit-dev] Removing the prefix from webkitPostMessage

[Ojan Vafai]

On Thu, Sep 13, 2012 at 12:06 PM, Dirk Pranke  wrote:

> On Wed, Sep 12, 2012 at 11:00 PM, Maciej Stachowiak  wrote:
> >
> > On Sep 12, 2012, at 10:36 PM, Ojan Vafai  wrote:
> >
> > On Wed, Sep 12, 2012 at 6:40 PM, Maciej Stachowiak 
> 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

[Ojan Vafai]

On Wed, Sep 12, 2012 at 6:40 PM, Maciej Stachowiak  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

[Ojan Vafai]

On Fri, Aug 24, 2012 at 11:19 PM, Benjamin Poulain wrote:

> On Fri, Aug 24, 2012 at 10:18 PM, Adam Barth  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

[Ojan Vafai]

On Fri, Aug 24, 2012 at 4:42 AM, Žan Doberšek  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-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

[Ojan Vafai]

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-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

[Ojan Vafai]

On Mon, Aug 20, 2012 at 6:03 PM, Maciej Stachowiak  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
> -.txt, and check in the new
> result as -.
>

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=chromium&q=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

[Ojan Vafai]

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  wrote:

> Do we know what ports have CSS3_FLEXBOX enabled?
>
> On Fri, Aug 17, 2012 at 6:47 PM, Ojan Vafai  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

Re: [webkit-dev] A simpler proposal for handling failing tests WAS: A proposal for handling "failing" layout tests and TestExpectations

[Ojan Vafai]

That matches my understanding. You proposed modification sounds fine to me.

On Fri, Aug 17, 2012 at 6:40 PM, Maciej Stachowiak  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  wrote:
>
> +1
>
>
> On Fri, Aug 17, 2012 at 5:36 PM, Ryosuke Niwa  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  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  wrote:
>>>
>>> On Fri, Aug 17, 2012 at 4:55 PM, Ojan Vafai  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

[webkit-dev] PSA: removing CSS3_FLEXBOX define

[Ojan Vafai]

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

[Ojan Vafai]

+1


On Fri, Aug 17, 2012 at 5:36 PM, Ryosuke Niwa  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  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  wrote:
>>
>> On Fri, Aug 17, 2012 at 4:55 PM, Ojan Vafai  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] A simpler proposal for handling failing tests WAS: A proposal for handling "failing" layout tests and TestExpectations

[Ojan Vafai]

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  wrote:

> On Fri, Aug 17, 2012 at 11:29 AM, Ryosuke Niwa  wrote:
> > On Fri, Aug 17, 2012 at 11:06 AM, Dirk Pranke 
> 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
>

Re: [webkit-dev] A proposal for handling "failing" layout tests and TestExpectations

[Ojan Vafai]

On Thu, Aug 16, 2012 at 2:32 PM, Filip Pizlo  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

[Ojan Vafai]

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  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  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 
> 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

[webkit-dev] DRT/WTR should clear the cache at the beginning of each test?

[Ojan Vafai]

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?

[Ojan Vafai]

On Tue, Jul 31, 2012 at 10:05 PM, Kihong Kwon  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

Re: [webkit-dev] PSA: rebaseline tooling

[Ojan Vafai]

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  wrote:

> On Thu, Jul 12, 2012 at 4:16 PM, Dirk Pranke  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

[webkit-dev] PSA: rebaseline tooling

[Ojan Vafai]

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] Comments in the code (Was Please include function-level comments in change log entries)

[Ojan Vafai]

On Wed, Jul 11, 2012 at 8:43 AM, John Mellor  wrote:

> On Wed, Jul 11, 2012 at 4:21 PM, Ryosuke Niwa  wrote:
>
>> On Wed, Jul 11, 2012 at 6:54 AM, John Mellor  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 .
>>> // 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

Re: [webkit-dev] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...

[Ojan Vafai]

On Thu, Jun 14, 2012 at 9:20 PM, Maciej Stachowiak  wrote:

>
> On Jun 14, 2012, at 9:06 PM, Adam Barth  wrote:
>
> > On Thu, Jun 14, 2012 at 9:02 PM, Ojan Vafai  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
bikeshed 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] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...

[Ojan Vafai]

On Thu, Jun 14, 2012 at 9:00 PM, Adam Barth  wrote:

> On Thu, Jun 14, 2012 at 8:56 PM, Ojan Vafai  wrote:
> > On Thu, Jun 14, 2012 at 4:34 PM, Dirk Pranke 
> wrote:
> >> On Thu, Jun 14, 2012 at 4:22 PM, Maciej Stachowiak 
> wrote:
> >> > On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa  wrote:
> >> > On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting  >
> >> > wrote:
> >> >>
> >> >> On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger 
> >> >> 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

[webkit-dev] IMAGE+TEXT WAS: TestExpectations syntax changes, last call (for a while, at least) ...

[Ojan Vafai]

On Thu, Jun 14, 2012 at 4:34 PM, Dirk Pranke  wrote:

> On Thu, Jun 14, 2012 at 4:22 PM, Maciej Stachowiak  wrote:
> >
> > On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa  wrote:
> >
> >
> > On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting 
> > wrote:
> >>
> >> On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger 
> 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

[webkit-dev] are fuzzer tests appropriate layout tests?

[Ojan Vafai]

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

[webkit-dev] removing js-test-post.js WAS: Can we distinguish imported tests in LayoutTests/css3 ?

[Ojan Vafai]

On Tue, Jun 12, 2012 at 10:18 AM, Alexey Proskuryakov  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

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Ojan Vafai]

On Tue, Jun 12, 2012 at 9:50 AM, Darin Adler  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

Re: [webkit-dev] Can we distinguish imported tests in LayoutTests/css3 ?

[Ojan Vafai]

I don't think there is any consistency across the project here except
everyone agrees that imported test suites shouldn't go in fast, e.g. all
our new flexbox tests have gone in css3/flexbox.

On Mon, Jun 11, 2012 at 10:28 PM, James Robinson  wrote:

> There are plenty of non-text tests in fast/, even entire directories of
> them (fast/repaint). My understanding of the meaning of fast/ is that it is
> where new tests that are not imported should go. This meaning is not
> universally applied.
>
> - James
> On Jun 11, 2012 7:33 PM, "Mike Lawther"  wrote:
>
>> I'm the guy that added css3/calc tests.
>>
>> I did so because I not all my tests are 'text only' tests, but I still
>> wanted them all together. My understanding was that the 'fast' directory
>> was intended for 'text only' tests only.
>>
>> Does having the 'fast' directory still serve a useful purpose? I reckon
>> the original intent could be pushed into the tools, eg have a
>> 'new-run-webkit-tests --fast', which will only run text-only tests. Then
>> the developer adding new tests doesn't have to worry about where to put
>> them.
>>
>> mike
>>
>> On 11 June 2012 18:57, Ryosuke Niwa  wrote:
>>
>>> Hi,
>>>
>>> I realized that there are a whole bunch of tests in LayoutTests/css3
>>> that use layoutTestController (e.g. css3/calc, css3/filters, etc...), which
>>> appears to mean that they're our own tests. However, css3/selectors3 is an
>>> imported W3C test suite. It's very confusing to mix imported tests and
>>> WebKit's own tests. Can we put the imported W3C tests in imported-w3c
>>> directory as proposed earlier?
>>>
>>> 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
>>
>>
> ___
> 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] DOM tree traversal on detached nodes

[Ojan Vafai]

On Mon, Jun 11, 2012 at 9:32 PM, Filip Pizlo  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  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 
> 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 stop using Skipped files?

[Ojan Vafai]

On Sun, Jun 10, 2012 at 4:54 AM, Balazs Kelemen  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

[Ojan Vafai]

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  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?

[Ojan Vafai]

On Fri, Jun 8, 2012 at 12:50 PM, Filip Pizlo  wrote:

> On Jun 8, 2012, at 12:31 PM, Dirk Pranke wrote:
>
> > On Fri, Jun 8, 2012 at 10:56 AM, Filip Pizlo  wrote:
> >>
> >> It's a lot harder to dive into, a lot more cumbersome to improve, and
> not
> >> any easier to maintain.
> >>
> >
> > I definitely agree that NRWT is more complicated than it seems like it
> > should be; it got contorted as we added all the features we needed to
> > add, and I have been in a "simplify" mode over the past few months. I
> > would welcome any feedback where you think things are overly complex.
>
> This is a difficult question - it's unfortunately easier to observe that
> something is complex, than it is to pinpoint why it is complex.  But I will
> try.
>
> 1) Code locality.  I can open Tools/Scripts/old-run-webkit-tests and
> pretty rapidly discover (a) how options are parsed, (b) how platform
> differences are handled, (c) how tests are found, and (d) how tests are
> run.  I can hack all of this code because it's all in one place.  I don't
> have to be a domain expert to do it.  Hell, I don't even have to be good a
> Perl to find my way around.
>
> 2) Code size:
>
> [pizlo@wartooth OpenSource] wc Tools/Scripts/old-run-webkit-tests
>2796   10316   98733 Tools/Scripts/old-run-webkit-tests
>
> [pizlo@wartooth OpenSource] wc Tools/Scripts/new-run-webkit-tests
> Tools/Scripts/webkitpy/layout_tests/*.py
> Tools/Scripts/webkitpy/layout_tests/*/*.py
>  bunch of stuff
>   23197 91897 1049914 total
>
> That's a *HUGE* difference.  Consider that NRWT just adds only one thing
> that most people care about: parallelism.  Is an 8x increase in code size
> justified?
>
> I know that LoC metrics are evil in most cases.  But this is not most
> cases.  This is an order-of-magnitude difference.  That's 8x more code I
> have to look at to find what I want.  That's 8x more code that I
> potentially have to edit to add a feature.  That's 8x more code that could
> have a bug.  And so on.  Badness!
>
> >
> >> The fact that it is unittested is part of the problem.  It restricts
> what
> >> you can do to the interfaces used internally in the code, which makes
> larger
> >> changes much harder to pull off.  It also steepens the learning curve.
> >
> > This is true. Of course, like all tests, it's a tradeoff. If it's not
> > tested, it's very easy for one to change things and break some other
> > feature that people need.
>
> But the act of running layout tests is in itself a regression test of the
> layout test harness just as much as it is a regression test of WebKit.
>
> 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.


>
> >
> >> Look, the test harness should be a thing that helps us get work done,
> not an
> >> end unto itself.  The harness should err on the side of
> understandability,
> >> hackability, and, in short, simplicity.
> >
> > I definitely agree that the harness should be a means and not an end.
> > I of course like those ilities, too, but we also  include
> > functionality and performance, and both of those usually bring in
> > complexity :(.
>
> Right, and we are all for functionality and performance.  That's good
> stuff.
>
> But to me, one of the primary goals of tooling infrastructure should be
> that it is as simple as possible.  This is of course a huge challenge -
> doing more with less is always harder than allowing the code to bloat ad
> infinitum.  But the need for simplicity is particularly acute in something
> that as central to our understanding of WebKit's conformance and
> correctness as the test harness.  This is the part of the system that must
> not be wrong or flaky, because if it is, then it diminishes the value of
> having tests at all, which then leads to horrible bugs not getting caught.
>
> >
> >> Python feels like the wrong technology to use for multithreading.  I'm
> >> curious if, in the future, next time we rewrite RWT (and I do believe
> that
> >> there will be such a "next time"), we could pick a language that allows
> this
> >> more naturally.
> >
> > It is true that Python's multithreading support is not great (it works
> > as long as you don't need to interact with subprocesses, and as long
> > as you don't need truly conc

Re: [webkit-dev] can we stop using Skipped files?

[Ojan Vafai]

On Fri, Jun 8, 2012 at 1:37 PM, Ryosuke Niwa  wrote:

> On Fri, Jun 8, 2012 at 1:27 PM, Dirk Pranke  wrote:
>>
>> Most of these abstractions were either added to make testing easier
>> (and faster since we didn't have to write to a real filesystem)
>>
>
> That sounds like a bad idea.
>

FWIW, I thought so at first as well, but as I've worked with the FileSystem
abstraction, I've found it to be very helpful and provide almost no extra
abstraction burden since it tries to closely match the Python APIs.

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

[Ojan Vafai]

On Fri, Jun 8, 2012 at 12:46 PM, Dirk Pranke  wrote:

> On Fri, Jun 8, 2012 at 12:44 PM, Ryosuke Niwa  wrote:
> > On Fri, Jun 8, 2012 at 12:37 PM, Dirk Pranke 
> wrote:
> >>
> >> I have no objection either to increasing the defaults for either of
> >> these numbers or making it possible to have different defaults per
> >> port.
> >>
> >> Do you want to suggest different defaults? Should we use ORWT's
> >> (infinite failures and infinite crashes by default)?
> >
> >
> > Unbounded failures & crashes sound good to me although I can definitely
> see
> > a value in passing some fixed value on bots so as to reduce the cycle
> time
> > when 10,000+ tests start to crash. Something like 100, 300, or 500 would
> > probably be acceptable.
> >
>
> NRWT defaults to 500 failures, so you're suggesting we just up the
> default # of crashes to 500? Anyone have any other numbers jumping out
> at them?
>

I'm OK with the ORWT defaults of infinite for both. Then we never have to
have this discussion again. :) It can be on the buildbot maintainers to
pass in the right flags. All of the non-chromium ports already do anyways
as best I can tell.
___
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

[Ojan Vafai]

On Fri, Jun 8, 2012 at 12:08 PM, Ryosuke Niwa  wrote:

> On Fri, Jun 8, 2012 at 12:05 PM, Ojan Vafai  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

[webkit-dev] Defaulting --exit-after-n-failures to 0

[Ojan Vafai]

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] can we stop using Skipped files?

[Ojan Vafai]

On Fri, Jun 8, 2012 at 10:14 AM, Zoltan Herczeg  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

Re: [webkit-dev] can we stop using Skipped files?

[Ojan Vafai]

On Fri, Jun 8, 2012 at 9:25 AM, Filip Pizlo  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 Kelemen  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?

[Ojan Vafai]

On Fri, Jun 8, 2012 at 12: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.


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-**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

[Ojan Vafai]

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 wrote:

> 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  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] We should rename layoutTestController to testController

[Ojan Vafai]

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  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] Using ref tests for repaint bugs

[Ojan Vafai]

On Thu, May 24, 2012 at 3:59 PM, Andrei Bucur wrote:

> 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  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 wrote:
>>
>>> 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

[Ojan Vafai]

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 wrote:

> 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)

[Ojan Vafai]

On Thu, May 17, 2012 at 10:37 PM, Maciej Stachowiak  wrote:

>
> On May 17, 2012, at 7:27 PM, Ojan Vafai  wrote:
>
> On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:
>
>>
>> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>>
>> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting wrote:
>>
>>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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)

[Ojan Vafai]

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  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 wrote:
>
>> On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa  wrote:
>> > On Thu, May 17, 2012 at 12:36 PM, Dirk Pranke 
>> wrote:
>> >>
>> >> On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai  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] Merging Skipped and test_expectations.txt formats WAS: Simplifying syntax in test_expectations.txt (bug 86691)

[Ojan Vafai]

On Thu, May 17, 2012 at 4:29 PM, Maciej Stachowiak  wrote:

>
> On May 17, 2012, at 1:42 PM, Ojan Vafai  wrote:
>
> On Thu, May 17, 2012 at 1:37 PM, Peter Kasting wrote:
>
>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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)

[Ojan Vafai]

On Thu, May 17, 2012 at 4:50 PM, Maciej Stachowiak  wrote:

>
> On May 17, 2012, at 4:40 PM, Dirk Pranke  wrote:
>
> > On Thu, May 17, 2012 at 4:16 PM, Maciej Stachowiak 
> wrote:
> >>
> >> Let's take an example. "TEXT" next to a test name apparently means that
> the text fails. There is no way in the world I would guess that just from
> reading an expectations file. This is only conceivably understandable to
> someone who is an expert on the format. If someone used TEXT in code to
> mean "fail", I would r- their patch for failure to use meaningful
> identifiers.
> >>
> >
> > I hardly think you have to be an expert on the format. I think you
> > probably need it explained to you once, or you could just read
> > http://trac.webkit.org/wiki/TestExpectations (which is linked to from
> > (I think) all of the expectations files).
>
> If someone gave that kind of explanation for a variable in a patch to C++
> code, I would still r- their patch. The token's meaning should be apparent
> without having to read out-of-line docs first.
>
> >
> > At the risk of overly repeating myself, I am not wedded to any one
> > format here, but I'm also not inclined to change things just because a
> > couple of people have vocally objected. If there was a clear consensus
> > that any change was preferred, that's fine by me.
>
> I feel like the people objecting to change, you included, are objecting
> because mostly because they are already familiar and comfortable with the
> current format. And not because it is genuinely better than another format
> that people might be similarly experienced with in the future. It is
> totally possible to be both newbie-friendly and experienced-friendly if you
> do not limit "experienced" to only the people who already have experience
> today.
>

All the proposals that are not just bikeshedding we seem to agree on and
will happen (the half-dozen things I listed above). Sure TEXT, IMAGE, etc
are not very clear, but noone has actually proposed something better.

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)

[Ojan Vafai]

On Thu, May 17, 2012 at 3:37 PM, Dirk Pranke  wrote:

> On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa  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

Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

[Ojan Vafai]

On Thu, May 17, 2012 at 2:29 PM, Ryosuke Niwa  wrote:

> On Thu, May 17, 2012 at 2:19 PM, Ojan Vafai  wrote:
>
>> On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa  wrote:
>>
>>> On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai  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)

[Ojan Vafai]

On Thu, May 17, 2012 at 2:16 PM, Ryosuke Niwa  wrote:

> On Thu, May 17, 2012 at 2:11 PM, Ojan Vafai  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)

[Ojan Vafai]

On Thu, May 17, 2012 at 2:06 PM, Ryosuke Niwa  wrote:

> On Thu, May 17, 2012 at 2:05 PM, Ojan Vafai  wrote:
>
>> On Thu, May 17, 2012 at 2:02 PM, Ryosuke Niwa  wrote:
>>
>>> Seems reasonable as the first step except:
>>>
>>> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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)

[Ojan Vafai]

On Thu, May 17, 2012 at 2:02 PM, Ryosuke Niwa  wrote:

> Seems reasonable as the first step except:
>
> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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.
___
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)

[Ojan Vafai]

On Thu, May 17, 2012 at 1:37 PM, Peter Kasting wrote:

> On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai  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)

[Ojan Vafai]

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  wrote:

> On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa  wrote:
> > On Thu, May 17, 2012 at 12:36 PM, Dirk Pranke 
> wrote:
> >>
> >> On Thu, May 17, 2012 at 4:30 AM, Ojan Vafai  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

Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)

[Ojan Vafai]

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  wrote:

> On Thu, May 17, 2012 at 1:06 AM, Maciej Stachowiak  wrote:
>>
>>  On May 16, 2012, at 10:39 PM, Dirk Pranke  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] spamming the developer console

[Ojan Vafai]

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  wrote:

> On Fri, May 11, 2012 at 2:48 PM, Ojan Vafai  wrote:
>
>> On Fri, May 11, 2012 at 2:39 PM, Maciej Stachowiak  wrote:
>>
>>>
>>> On May 11, 2012, at 2:21 PM, Ojan Vafai  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

[Ojan Vafai]

On Fri, May 11, 2012 at 2:48 PM, Ojan Vafai  wrote:

> On Fri, May 11, 2012 at 2:39 PM, Maciej Stachowiak  wrote:
>
>>
>> On May 11, 2012, at 2:21 PM, Ojan Vafai  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

[Ojan Vafai]

On Fri, May 11, 2012 at 2:39 PM, Maciej Stachowiak  wrote:

>
> On May 11, 2012, at 2:21 PM, Ojan Vafai  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

[Ojan Vafai]

On Fri, May 11, 2012 at 2:25 PM, Adam Barth  wrote:

> On Fri, May 11, 2012 at 2:21 PM, Ojan Vafai  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

[webkit-dev] spamming the developer console

[Ojan Vafai]

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] EFL Debug Buildbot Green

[Ojan Vafai]

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  
>
>
> ___
> 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

[Ojan Vafai]

On Tue, May 8, 2012 at 11:32 AM, Hugo Parente Lima
wrote:

> 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.:
> >
> > 
> >
> > 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:
> >
> > 
> >
> > 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  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 resp

Re: [webkit-dev] Default viewport value on WAP DTDs

[Ojan Vafai]

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  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

Re: [webkit-dev] Default viewport value on WAP DTDs

[Ojan Vafai]

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
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 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] Bot watching and Apple bots

[Ojan Vafai]

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  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

[Ojan Vafai]

On Thu, Apr 12, 2012 at 2:18 PM, Dirk Pranke  wrote:

> On Apr 12, 2012 1:53 PM, "Ryosuke Niwa"  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 
> 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 
> >> Date: Thu, 12 Apr 2012 13:43:26 -0700
> >> To: Jacob Goldstein 
> >> Cc: Ojan Vafai , Dirk Pranke ,
> WebKit Development 
> >>
> >> 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 
> wrote:
> >>>
> >>> Isn't the goal of writing a ref test that it is not platform specific?
>
> >>>
> >>> From: Ryosuke Niwa 
> >>> Date: Thu, 12 Apr 2012 12:29:42 -0700
> >>> To: Ojan Vafai 
> >>> Cc: Dirk Pranke , 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 
> 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

[Ojan Vafai]

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  wrote:

> Isn't the goal of writing a ref test that it is not platform specific?
>
> From: Ryosuke Niwa 
> Date: Thu, 12 Apr 2012 12:29:42 -0700
> To: Ojan Vafai 
> Cc: Dirk Pranke , 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  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] Is converting pixel tests to reftests kosher for imported libraries?

[Ojan Vafai]

On Thu, Apr 12, 2012 at 12:50 PM, Robert Hogan wrote:

> On Thursday 12 April 2012 00:58:47 Ryosuke Niwa wrote:
> > On Wed, Apr 11, 2012 at 4:45 PM, Ojan Vafai  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

[Ojan Vafai]

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=true&showLargeExpectations=true&revision=113914&tests=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  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 wrote:
>>
>> 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?

[Ojan Vafai]

On Wed, Apr 11, 2012 at 3:48 PM, Dirk Pranke  wrote:

> On Wed, Apr 11, 2012 at 3:46 PM, Dirk Pranke  wrote:
> > On Fri, Mar 9, 2012 at 2:49 PM, Sam Weinig  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] Process for making changes that affect layout test results

[Ojan Vafai]

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  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  wrote:
>
>> On Wed, Apr 11, 2012 at 1:57 PM, Tony Payne  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] handling failing tests (test_expectations, Skipped files, etc.)

[Ojan Vafai]

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  wrote:

> On Tue, Apr 10, 2012 at 11:42 AM, Stephen Chenney 
> wrote:
>
>> On Tue, Apr 10, 2012 at 1:00 PM, Ryosuke Niwa  wrote:
>>
>>> On Tue, Apr 10, 2012 at 6:10 AM, Stephen Chenney 
>>> wrote:
>>>
 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.
>>

Re: [webkit-dev] Why does PositionIterator iterates until the end of the document when selecting?

[Ojan Vafai]

One case where this matters: foobar

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

[Ojan Vafai]

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