from:"Maciej Stachowiak"


On Aug 27, 2012, at 2:45 PM, Eric Seidel e...@webkit.org wrote:

 Checking back in:
 
 Curious if this effort is still underway.  Adam and I would like to
 delete the New XML Parser if it's not needed in order to simplify the
 HTML 5 Parser again. :)

We do tentatively plan to get back to it (the original implementor is currently 
working full-time at Apple on the WebKit team).

As far as simplifying the HTML5 parser - isn't most of the foundational work 
that touches the HTML5 parser also required for WebVTT, as mentioned by me in 
the email you quote below? Is there a big simplicity win to be had without 
breaking WebVTT? If so, we can think about whether removing the scaffolding and 
reconstructing it when needed is worthwhile.

 - Maciej

 
 
 On Thu, Mar 15, 2012 at 1:58 PM, Maciej Stachowiak m...@apple.com wrote:
 
 On Mar 15, 2012, at 1:29 PM, Eric Seidel wrote:
 
 It seems the New XML Parser hasn't been touched in about 8 months:
 
 http://trac.webkit.org/browser/trunk/Source/WebCore/xml/parser
 
 Are there any plans to continue work on such, or can it be removed?
 The refactoring which was done to support it seems to mostly just
 confuse the HTML 5 Parser code... :(
 
 (I went looking for how something was implemented in the HTML5
 parser... and it took me a long time to find it this afternoon).
 
 Yes, we plan to get more active on this effort again within a few months.
 
 Note that some of the refactoring was able to benefit the WebVTT parser, so 
 we need some generic parser abstractions in any case.
 
 Regards,
 Maciej
 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Is the New XMLParser dead?


On Aug 27, 2012, at 3:48 PM, Adam Barth aba...@webkit.org wrote:

 On Mon, Aug 27, 2012 at 3:06 PM, Maciej Stachowiak m...@apple.com wrote:
 On Aug 27, 2012, at 2:45 PM, Eric Seidel e...@webkit.org wrote:
 Checking back in:
 
 Curious if this effort is still underway.  Adam and I would like to
 delete the New XML Parser if it's not needed in order to simplify the
 HTML 5 Parser again. :)
 
 We do tentatively plan to get back to it (the original implementor is 
 currently working full-time at Apple on the WebKit team).
 
 As far as I can tell, no one has worked on the NEWXML code in over a
 year, the implementation doesn't work, and the code is disabled by all
 ports.  It seems like we should remove it from trunk.  We can retore
 it if/when someone is interested in working on it again.

What you describe as the current status is (afaik) correct. The data point I 
provided (since Eric asked) is that we do in fact plan to get back to it.

 
 As far as simplifying the HTML5 parser - isn't most of the foundational work 
 that touches the HTML5 parser also required for WebVTT, as mentioned by me 
 in the email you quote below? Is there a big simplicity win to be had 
 without breaking WebVTT? If so, we can think about whether removing the 
 scaffolding and reconstructing it when needed is worthwhile.
 
 This is a separate issue.

If there's a reason to remove it other than simplify the HTML5 parser again, 
then certainly we can consider it. But that was the only reason Eric cited, so 
I wanted to check if it's actually the case, in light of WebVTT. I am still 
curious about the answer. But I'd be happy to discuss other reasons instead.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Is the New XMLParser dead?


On Aug 27, 2012, at 4:28 PM, Adam Barth aba...@webkit.org wrote:

 On Mon, Aug 27, 2012 at 4:02 PM, Maciej Stachowiak m...@apple.com wrote:
 On Aug 27, 2012, at 3:48 PM, Adam Barth aba...@webkit.org wrote:
 On Mon, Aug 27, 2012 at 3:06 PM, Maciej Stachowiak m...@apple.com wrote:
 On Aug 27, 2012, at 2:45 PM, Eric Seidel e...@webkit.org wrote:
 Checking back in:
 
 Curious if this effort is still underway.  Adam and I would like to
 delete the New XML Parser if it's not needed in order to simplify the
 HTML 5 Parser again. :)
 
 We do tentatively plan to get back to it (the original implementor is 
 currently working full-time at Apple on the WebKit team).
 
 As far as I can tell, no one has worked on the NEWXML code in over a
 year, the implementation doesn't work, and the code is disabled by all
 ports.  It seems like we should remove it from trunk.  We can retore
 it if/when someone is interested in working on it again.
 
 What you describe as the current status is (afaik) correct. The data point I 
 provided (since Eric asked) is that we do in fact plan to get back to it.
 
 As far as simplifying the HTML5 parser - isn't most of the foundational 
 work that touches the HTML5 parser also required for WebVTT, as mentioned 
 by me in the email you quote below? Is there a big simplicity win to be 
 had without breaking WebVTT? If so, we can think about whether removing 
 the scaffolding and reconstructing it when needed is worthwhile.
 
 This is a separate issue.
 
 If there's a reason to remove it other than simplify the HTML5 parser 
 again, then certainly we can consider it. But that was the only reason Eric 
 cited, so I wanted to check if it's actually the case, in light of WebVTT. I 
 am still curious about the answer. But I'd be happy to discuss other reasons 
 instead.
 
 My understanding is that we don't typically leave broken, unused code
 in trunk unless someone is actively working on it.  Having this code
 around has costs and little benefit:
 
 1) The code needs to be maintained.
 2) The code confuses contributors who don't know that it's dead.
 
 By contrast, if someone wants to work on this code again, they can
 just revert the patch that removed it.  They might need to do some
 maintenance work at that point, but that's work that otherwise would
 have to have been done by someone else.

Do you have examples of actual confusion or undue additional maintenance? I 
think if those things were really happening, then that would make a decent 
argument for removing the code for the time being. From svn history, it doesn't 
look like the new xml parser code has been touched in quite some time, so I'm 
not seeing evidence for maintenance cost. But I could be overlooking other 
costs.

 
 As for VTT, I suspect that the VTT parser doesn't need all the
 complexity that the HTML parser needs.  It's doing a much simpler task
 and likely can be made much simpler by not try to share code with the
 HTML parser at all.

I think the main sharing in all three cases is the MarkupTokenBase and 
MarkupTokenizerBase base classes. So XML tokenizer causes (as far as I know) no 
additional abstraction or complexity beyond WebVTT.

I am skeptical that either duplicating that particular code instead of sharing 
it, or rewriting the WebVTT tokenizer in some dramatically different way would 
be beneficial. Let's set the simplification argument aside unless there is some 
compelling concrete evidence for it (such as WebVTT hackers expressing interest 
in rewriting the WebVTT parser to not share code with HTML.)


Regards,
Maciej



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Is the New XMLParser dead?


On Aug 27, 2012, at 5:02 PM, Adam Barth aba...@webkit.org wrote:

 On Mon, Aug 27, 2012 at 4:46 PM, Dirk Schulze dschu...@adobe.com wrote:
 
 Ha! So the reason for removing the code is simplifying the HTML5 parser, 
 just to undo the simplification once the original writer has the time to 
 come back to it. Seems like well spend time :)
 
 That depends on how likely it is that and when the original authors
 gets back to the work, hence Eric's question.  Tentatively plan to
 get back to it makes it sound like it will be a while.

It's not anyone's immediate current task, and I can't really make firm promises 
about future timelines. But we actually are seriously interested in finishing 
it.

Cheers,
Maciej

___
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

2012-08-24 Thread Maciej Stachowiak


What's the difference between the ASCIILiteral and ConstructFromLiteral 
versions?

 - Maciej

On Aug 24, 2012, at 8:00 PM, Benjamin Poulain benja...@webkit.org wrote:

 Dear webkit-dev,
 
 
 Some recent changes improved the way we can use string classes with literals.
 
 There are 3 new constructors for initializing a string from a literal:
 -String(ASCIILIteral): 
 http://trac.webkit.org/browser/trunk/Source/WTF/wtf/text/WTFString.h#L139
 -String(const char[], ConstructFromLiteralTag)
 -AtomicString(const char[], ConstructFromLiteralTag)
 
 The differences with the regular char* constructor are:
 -no memory is allocated for the bytes of the string.
 -the characters are not copied to the heap
 -String(const char[], ConstructFromLiteralTag) does not even access the bytes 
 of the string.
 
 Those constructors are faster and use less memory in some cases.
 I converted some of the generated code to use the new constructors. In the 
 future, please consider using ASCIILiteral() whenever constructing a String, 
 and ConstructFromLiteral for a AtomicString.
 
 cheers,
 Benjamin
 
 PS: I started a Wiki page about the efficient use of the string classes: 
 http://trac.webkit.org/wiki/EfficientStrings
 ___
 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

2012-08-21 Thread Maciej Stachowiak

On Aug 21, 2012, at 3:23 PM, Ojan Vafai o...@chromium.org wrote:

On Mon, Aug 20, 2012 at 6:03 PM, Maciej Stachowiak m...@apple.com wrote:
Here's how I imagine the workflow when a sheriff or just innocent bystander
notices a deterministically failing test. Follow this two-step algorithm:

1) Are you confident that the new result is an improvement or no worse? If
so, then simply update -expected.txt.
2) Otherwise, copy the old result to
-whatever-we-call-the-unexpected-pass-result.txt, and check in the new
result as -whatever-we-call-the-expected-failure-result.txt.

I think we should do this. I don't care much about the naming.

This replaces all other approaches to marking expected failures, including
the Skipped list, overwriting -expected even you know the result is a
regression, marking the test in TestExpectations as Skip, Wontfix, Image,
Text, or Text+Image, or any of the other legacy techniques for marking an
expected failure reult.

Don't forget suffixing the test with -disabled! We have 109 such tests at
the moment according to
http://code.google.com/searchframe#search/exact_package=chromiumq=file:third_party/WebKit/LayoutTests/.*%5C-disabled$type=cs.
I think we should also get rid of this. If we need a way to disable a test
across ports (e.g. because it crashes in cross-platform code), we should make
a Skipped/TestExpectations file in LayoutTest/platform instead of renaming
the test file.

I agree that renaming to -disabled should be phased out as well. I specifically
did not cover failure modes that produce no result, such as crashes or hangs.
Those should still be tracked via TestExpectations IMO. Likewise for
nondeterministic expectations failures.

Regards,
Maciej

___
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

2012-08-18 Thread Maciej Stachowiak


On Aug 18, 2012, at 1:08 AM, Filip Pizlo fpi...@apple.com wrote:

 I like your idea of having both the result-we-currently-expect and the 
 result-we-think-may-be-more-correct to be checked in.  I still prefer Dirk's 
 naming scheme though.

I think if we had both checked in, the result-we-think-may-be-more-correct 
should be named something other than -expected, since it is not, in fact, 
expected. That was the basis of my naming scheme. 

I think I would be happy with any scheme that had both checked in, and matched 
the criteria that you never have a file named -expected that is unexpected. For 
example, there could be schemes with no file named expected. If you let it be 
verbose, you could have:

Single result:
foo-expected.txt

Possibly-worse current result, possibly-better older result:
foo-expected-failure.txt
foo-unexpected-pass.txt

 
 I get the notion that expected always means literally what it seems to mean 
 from the standpoint of whether the tooling is silent for the test (actual == 
 expected) or has something to say.
 
 But I think that if the tooling is behaving right, your concern that a test 
 would fail if it did *not* match the failing result would be addressed: 
 the tooling could be silent for actual == failing (if a failing file exists) 
 but notify you of an unexpected pass if actual == expected.

But if you match neither, you get a failure for not matching the failing 
result. That still strikes me as a little goofy. Not failing is failing, and 
getting the expected result is unexpected. I think my extra-verbose naming 
scheme above would better match what you suggest the tool UI would do. Maybe 
there is a more concise way to get the same point across.

Regards,
Maciej

___
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

2012-08-18 Thread Maciej Stachowiak


On Aug 18, 2012, at 5:11 PM, Filip Pizlo fpi...@apple.com wrote:

 Maybe at this point we can agree to let Dirk land some variant of this with 
 whatever half-way sensible name (any of the options on the table are decent) 
 and see how it works?
 
 It seems that the only thing anyone is disagreeing over is naming and which 
 files to keep around, which is a much smaller set of differences than 
 status-quo versus any variant of this proposal. 

I agree that we should adopt some variant over the status quo. As you rightly 
noted, there are too many different ways to handle tests that deviate from the 
original expectation, and we have the opportunity to obsolete most of those 
ways with an approach that combines advantages of multiple current approaches.

However, I fear that whatever names we pick for the first round will then be 
unchangeable due to status quo bias (which we see a lot of in test 
infrastructure discussions, indeed, even this one). And anyone arguing against 
change at that point will have a valid argument that a huge global rename of 
tests is a bad idea. So I think it's worth expending a little effort to find 
names that are good.

Would you object to -expected-failure/-unexpected-pass as a naming scheme, 
along with the approach of keeping both around when they are used?

Regards,
Maciej

 
 -Filip
 
 On Aug 18, 2012, at 2:01 PM, Maciej Stachowiak m...@apple.com wrote:
 
 
 On Aug 18, 2012, at 1:08 AM, Filip Pizlo fpi...@apple.com wrote:
 
 I like your idea of having both the result-we-currently-expect and the 
 result-we-think-may-be-more-correct to be checked in.  I still prefer 
 Dirk's naming scheme though.
 
 I think if we had both checked in, the result-we-think-may-be-more-correct 
 should be named something other than -expected, since it is not, in fact, 
 expected. That was the basis of my naming scheme. 
 
 I think I would be happy with any scheme that had both checked in, and 
 matched the criteria that you never have a file named -expected that is 
 unexpected. For example, there could be schemes with no file named expected. 
 If you let it be verbose, you could have:
 
 Single result:
   foo-expected.txt
 
 Possibly-worse current result, possibly-better older result:
   foo-expected-failure.txt
   foo-unexpected-pass.txt
 
 
 I get the notion that expected always means literally what it seems to 
 mean from the standpoint of whether the tooling is silent for the test 
 (actual == expected) or has something to say.
 
 But I think that if the tooling is behaving right, your concern that a 
 test would fail if it did *not* match the failing result would be 
 addressed: the tooling could be silent for actual == failing (if a failing 
 file exists) but notify you of an unexpected pass if actual == expected.
 
 But if you match neither, you get a failure for not matching the failing 
 result. That still strikes me as a little goofy. Not failing is failing, and 
 getting the expected result is unexpected. I think my extra-verbose naming 
 scheme above would better match what you suggest the tool UI would do. Maybe 
 there is a more concise way to get the same point across.
 
 Regards,
 Maciej
 

___
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

2012-08-17 Thread Maciej Stachowiak


My understanding of the current proposal is this:

1) This applies to tests that fail deterministically, for reasons other than a 
crash or hang.
2) If the test has a new result that you're confident is a progression (or 
neither better or worse), you simply update the -expected.txt file.
3) If the test has a new result that you're confident is a regression, you 
delete the -expected.txt file and check in the new results as -failing.txt.
4) Ditto points 2 and 3 with respect to -expected.png, for image diffs.
5) We would stop using all other ways of marking tests that fail 
deterministically, including Skipped and the many things you could enter in 
TestExpectations.

Is that correct?

If so, I'd like to suggest a minor modification. In place of point 3 above, I 
propose the following:

3) If the test has a new result that you're confident is a regression, you 
rename the -expected.txt file to -previous.txt (or maybe -correct.txt or 
-pre-expected.txt or something) and check in the new results as -expected.txt 
(unless there is already a -previous.txt, in which case just update -expected 
and leave -previous).

I propose this for the following reasons:

- It maintains the longstanding approach that -expected.txt reflects what is 
currently *expected* to happen, not whether it is right or wrong in some 
abstract sense. It is an expectation, not a reference.
- It still leaves a clear indication of tests that somebody needs to look at 
further, to determine if a regression occurred.
- It leaves both old and new result in place for easy comparison by an expert.

Regards,
Maciej


On Aug 17, 2012, at 6:06 PM, Ojan Vafai o...@chromium.org wrote:

 +1
 
 
 On Fri, Aug 17, 2012 at 5:36 PM, Ryosuke Niwa rn...@webkit.org wrote:
 That's my expectation although we probably can't do that for flaky tests :(
 
 e.g. sometimes fails with image diff.
 
 On Fri, Aug 17, 2012 at 5:35 PM, Filip Pizlo fpi...@apple.com wrote:
 +1, contingent upon the following: are we agreeing that all current uses of 
 TEXT, IMAGE, and so forth in TestExpectations should be in the *very near 
 term* following Dirk's change be turned into -failing files?
 
 -Filip
 
 
 On Aug 17, 2012, at 5:01 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 On Fri, Aug 17, 2012 at 4:55 PM, Ojan Vafai o...@chromium.org wrote:
 Asserting a test case is 100% correct is nearly impossible for a large 
 percentage of tests. The main advantage it gives us is the ability to have 
 -expected mean unsure.
 
 Lets instead only add -failing (i.e. no -passing). Leaving -expected to mean 
 roughly what it does today to Chromium folk (roughly, as best we can tell 
 this test is passing). -failing means it's *probably* an incorrect result 
 but needs an expert to look at it to either mark it correct (i.e. rename it 
 to -expected) or figure out how the root cause of the bug.
 
 This actually matches exactly what Chromium gardeners do today, except 
 instead of putting a line in TestExpectations/Skipped to look at later, they 
 checkin the -failing file to look at later, which has all the advantages 
 Dirk listed in the other thread.
 
 I'm much more comfortable with this proposal.
 
 - Ryosuke
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev
 
 
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

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

2012-08-15 Thread Maciej Stachowiak


On Aug 15, 2012, at 12:27 PM, Filip Pizlo fpi...@apple.com wrote:

 This sounds like it's adding even more complication to an already complicated 
 system.
 
 Given how many tests we currently have, I also don't buy that continuing to 
 run a test that is already known to fail provides much benefit.  If the test 
 covers two things and one fails while the other succeeds (example #1: it 
 prints correct results but doesn't crash; example #2: it literally has tests 
 for two distinct unrelated features, and one feature is broken) then the 
 correct thing to do is to split up the test.

The case where this happens isn't so much testing unrelated features. It's more 
that a single test can face failures of multiple incidental aspects. Let me 
give an example. Let's say you have a test that's intended to verify JavaScript 
multiplication. Let's say it starts failing because of a regression in function 
call behavior. There may not be a clean way to split these two aspects. You may 
have to rewrite the multiplication test to work around this hypothetical 
function call bug. It may be nontrivial to figure out how to do that.

So I agree in principle that splitting orthogonal tests is good, but it's not 
always a good solution to avoiding chained regressions.

Note also that having more test files makes the test rights slower.

 
 So, I'd rather not continue to institutionalize this notion that we should 
 have loads of incomprehensible machinery to reason about tests that have 
 already given us all the information they were meant to give (i.e. they 
 failed, end of story).

I think another aspect of your reasoning doesn't entirely fit with the history 
of WebKit regression testing. Our tests were not originally designed to test 
against some abstract notion of correct behavior. They were designed to 
detect behavior changes. It's definitely not the case that, once behavior has 
changed once, there's no value to detecting when it changes again. Some of 
these behavior changes might be good and should form a new baseline. Others 
might be bad but still should not inhibit detecting other testing while they 
are investigated.


(I should note that I don't yet have an opinion on the new proposal; I have not 
read it. But I am skeptical that splitting tests is a practical solution.)


Regards,
Maciej


 
 -F
 
 
 On Aug 15, 2012, at 12:22 PM, Dirk Pranke dpra...@chromium.org wrote:
 
 Hi all,
 
 As many of you know, we normally treat the -expected files as
 regression tests rather than correctness tests; they are intended
 to capture the current behavior of the tree. As such, they
 historically have not distinguished between a correct failure and an
 incorrect failure.
 
 The chromium port, however, has historically often not checked in
 expectations for tests that are currently failing (or even have been
 failing for a long time), and instead listed them in the expectations
 file. This was primarily motivated by us wanting to know easily all of
 the tests that were failing. However, this approach has its own
 downsides.
 
 I would like to move the project to a point where all of the ports
 were basically using the same workflow/model, and combine the best
 features of each approach [1].
 
 To that end, I propose that we allow tests to have expectations that
 end in '-passing' and '-failing' as well as '-expected'.
 
 The meanings for '-passing' and '-failing' should be obvious, and
 '-expected' can continue the current meaning of either or both of
 what we expect to happen and I don't know if this is correct or
 not :).
 
 A given test will be allowed to only have one of the three potential
 results at any one time/revision in a checkout. [2]
 
 Because '-expected' will still be supported, this means that ports can
 continue to work as they do today and we can try -passing/-failing on
 a piecemeal basis to see if it's useful or not.
 
 Ideally we will have some way (via a presubmit hook, or lint checks,
 or something) to be able to generate a (checked-in) list (or perhaps
 just a dashboard or web page) of all of the currently failing tests
 and corresponding bugs from the -failing expectations, so that we
 can keep one of the advantages that chromium has gotten out of their
 TestExpectations files [3].
 
 I will update all of the tools (run-webkit-tests, garden-o-matic,
 flakiness dashboard, etc.) as needed to make managing these things as
 easy as possible. [4]
 
 Thoughts? I'm definitely open to suggestions/variants/other ideas/etc.
 
 -- Dirk
 
 Notes:
 
 [1] Both the check in the failures and the suppress the failures
 approaches have advantages and disadvantages:
 
 Both approaches have their advantages and disadvantages:
 
 Advantages for checking in failures:
 
 * you can tell when a test starts failing differently
 * the need to distinguish between different types of failures (text
 vs. image vs. image+text) in the expectations file drops; the baseline
 tells you what to expect
 * the TestExpectations file

Re: [webkit-dev] Status of multithreaded image decoding

2012-08-13 Thread Maciej Stachowiak


On Aug 13, 2012, at 11:08 AM, Alpha Lam hc...@chromium.org wrote:

 That's a good point. I'm not sure but a safe bet would be after RenderView is 
 layout'ed then iterate through images to start decoding.
 
 The same thing would be needed for scrolling too, as page scrolls need to 
 iterate images in the viewport.

This approach is probably safe (as far as I know) but would have the downside 
of an extra pass over the whole render tree, or else overhead of maintaining an 
up-to-date list of rendered images; and it would happen very close to painting.

I think to some extent, benefit from parallelism is in direct tension with 
benefit from lazy on-demand decoding.

Note by the way that in general many kinds of render objects can result in 
rendering images, not just RenderImage. Consider the various forms of CSS 
images (CSS backgrounds, CSS content property pointing to an image, etc).

Regards,
Maciej

 
 2012/8/13 Alpha (Hin-Chung) Lam hc...@google.com
 That's a good point. I'm not sure but a safe bet would be after RenderView is 
 layout'ed then iterate through images to start decoding.
 
 The same thing would be needed for scrolling too, as page scrolls need to 
 iterate images in the viewport.
 
 Alpha
 
 2012/8/13 Maciej Stachowiak m...@apple.com
 
 The thing I'm not confident of is whether an image's position in absolute 
 coordinates can be changed by an ancestor after RenderImage::layout 
 completes. It would be helpful if a layout expert would weigh in.
 
  - Maciej
 
 On Aug 13, 2012, at 3:20 AM, Dong Seong Hwang luxte...@company100.net wrote:
 
  https://bugs.webkit.org/show_bug.cgi?id=90375#c80
 
  In the above link, Hin-Chung shows how to determine whether an image
  is actually painted.
 
  2012/8/13 Maciej Stachowiak m...@apple.com:
  I that case, starting async decoding at layout time makes sense if and only
  if at layout to e you can predict what you will paint. I don't know enough
  about our rendering to know of that is the case.
 
  - Maciej
 
 
 
  On Aug 12, 2012, at 5:34 PM, Peter Kasting pkast...@chromium.org wrote:
 
  On Sun, Aug 12, 2012 at 1:24 PM, Maciej Stachowiak m...@apple.com wrote:
 
  Why not start asynchronous decoding immediately as the image is loading,
  and synchronize at paint time? What is the benefit of waiting until layout
  time to start decoding the image data?
 
 
  Uninformed guess (since I haven't touched the decoders in a while), but
  currently we don't decode unless the image is actually painted, which helps
  a ton on pages that are an enormous long string of images (common cases:
  Boston Big Picture blog, various porn sites), since most of the images can
  be decoded after initial layout, or not at all (if the user never scrolls
  down enough).  If we started decoding as images loaded I'd imagine we'd do
  (possibly a lot of) extra work compared to today.
 
  PK
 
 
  ___
  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] Status of multithreaded image decoding

2012-08-12 Thread Maciej Stachowiak


On Aug 10, 2012, at 3:47 PM, Alpha Lam hc...@chromium.org wrote:

 This is a very valid concern. The question you raised is one topic I want to 
 discuss more broadly.
 
 Chromium has a separate rasterization stage so there is some time for 
 decoders to work, synchronization can happen in rasterization stage. This 
 doesn't mean Chromium won't benefit if decoding starts before draw time. 
 
 For other ports without rasterization stage this approach alone would have 
 the problem you mentioned. Just like you suggested decoding have to start 
 before paint time and then synchronize at paint time. One approach is to 
 start asynchronous decoding during layout time. When an image is downloaded 
 metadata is decoded synchronously and triggers a layout when intrinsic 
 dimension is available. This could be the time to check with current viewport 
 and start asynchronous decoding. For image used as background of a RenderBox 
 we can also start decoding if the rectangle of the RenderBox intersect with 
 viewport.

Why not start asynchronous decoding immediately as the image is loading, and 
synchronize at paint time? What is the benefit of waiting until layout time to 
start decoding the image data?

Regards,
Maciej


 Alpha
 
 2012/8/10 James Robinson jam...@google.com
 
 
 On Thu, Aug 9, 2012 at 5:10 PM, Alpha Lam hc...@chromium.org wrote:
 Hi everyone!
 
 A few weeks ago some folks from company100.net have started a thread of 
 multithreaded (parallel) image decoding in WebKit. We have worked together 
 since then to get a better idea how to complete this feature. I would like to 
 report on the progress and our plan.
 
 (The goal for Chromium is slightly different but it will reuse most of the 
 architecture discussed here.)
 
 Master bug: https://bugs.webkit.org/show_bug.cgi?id=90375
 
 In WebKit today image rendering is progressive, meaning that image is 
 rendered as bytes are received from the network. This is done through 
 ImageObserver and CachedImageClient interfaces. Multithreaded image decoding 
 uses the same notification architecture, clients of BitmapImage are notified 
 when a new region is decoded and available for painting.
 
 Today image decoding happens synchronously when a BitmapImage is drawn into a 
 GraphicsContext. We plan to use the draw operation as a trigger to start 
 image decoding asynchronously. Which means the first draw of BitmapImage will 
 get a transparent image, subsequent draws will have the most recently decoded 
 bitmap.
 
 
 I don't think this approach will work terribly well (at least not by itself). 
  It seems to require that we flash at least one frame without the image data 
 even when we really do have it available.  For instance, imagine a page with 
 a number of small image resources (small enough that they all load completely 
 in roughly the same instant), or cached image resources, that impact the 
 layout of the page.  Today, when the images load we will redo the layout to 
 accomodate the resources size, then at paint time decode the images and 
 render them.  The user never sees an intermediate state unless the resources 
 isn't fully loaded at paint time.  With this proposal, the user would always 
 see a flash where the images occupy layout space but are not actually 
 rendered.  I think that will be an unacceptably bad user experience.
 
 To do this, I think you either need deeper integration with the raster system 
 to make sure that it actually renders the images on the first paint (perhaps 
 by deferring the actual raster ops to give the decoder some time), or kicking 
 off the decode steps sooner and adding synchronization at paint time to make 
 sure we actually see the pixels.
 
 I'm really happy someone is looking into these difficult issues.
 
 - James
 
 This architecture will take several steps:
 
 https://bugs.webkit.org/show_bug.cgi?id=93467
 
 This modifies ImageSource to be asynchronous. ImageSource is used as the 
 public interface for decoding images. By making this interface asynchronous 
 individual port can implement parallel image decoding or a similar 
 architecture. This change is ready for review. I would like to get more 
 feedback on the interface since this touches all ports.
 
 https://bugs.webkit.org/show_bug.cgi?id=93590
 
 Progressive painting of an image may not be possible everywhere. For example 
 canvas and accelerated-composited img requires synchronous decoding. We 
 plan to keep synchronous decoding the default case. This change identifies 
 code location where asynchronous decoding is possible and tell BitmapImage 
 asynchronous image decoding is requested.
 
 After these two changes we will be able to implement multithreaded image 
 decoder inside BitmapImage and ImageSource. I will report on the progress 
 once we get to this point.
 
 Alpha
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Status of multithreaded image decoding

2012-08-12 Thread Maciej Stachowiak

I that case, starting async decoding at layout time makes sense if and only if 
at layout to e you can predict what you will paint. I don't know enough about 
our rendering to know of that is the case.

 - Maciej



On Aug 12, 2012, at 5:34 PM, Peter Kasting pkast...@chromium.org wrote:

 On Sun, Aug 12, 2012 at 1:24 PM, Maciej Stachowiak m...@apple.com wrote:
 Why not start asynchronous decoding immediately as the image is loading, and 
 synchronize at paint time? What is the benefit of waiting until layout time 
 to start decoding the image data?
 
 Uninformed guess (since I haven't touched the decoders in a while), but 
 currently we don't decode unless the image is actually painted, which helps a 
 ton on pages that are an enormous long string of images (common cases: Boston 
 Big Picture blog, various porn sites), since most of the images can be 
 decoded after initial layout, or not at all (if the user never scrolls down 
 enough).  If we started decoding as images loaded I'd imagine we'd do 
 (possibly a lot of) extra work compared to today.
 
 PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] We should encourage the use of httpS://svn.webkit.org

2012-08-09 Thread Maciej Stachowiak


On Aug 8, 2012, at 5:41 PM, Ryosuke Niwa rn...@webkit.org wrote:

 Hi,
 
 It appears that we recommend the use of non-secure HTTP connection on many 
 webkit.org documents:
 e.g. https://www.webkit.org/building/checkout.html
 
 Can we move away from this and recommend the use of HTTPS instead? It's very 
 easy to eavesdrop the svn credentials if we're using HTTP.

For anything that has an https equivalent, I think it would be fine to 
recommend https instead.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Add support for CSS3 text-decoration* properties

2012-08-01 Thread Maciej Stachowiak


On Aug 1, 2012, at 4:36 PM, Elliott Sprehn espr...@chromium.org wrote:

 
 On Tue, Jul 31, 2012 at 7:35 PM, Bruno Abinader brunoabina...@gmail.com 
 wrote:
 Hi all :)
 
 As suggested by Ojan, I am writing a mail to you about my intention to
 implement all updated and missing text-decoration* properties from
 CSS3 spec (currently in development), named below:
 
 -webkit-text-decoration ( https://bugs.webkit.org/show_bug.cgi?id=92000 )
 CSS3 dev spec: http://dev.w3.org/csswg/css3-text/#text-decoration
 Mozilla ref: https://developer.mozilla.org/en/CSS/text-decoration
 Status: Proposed patch / pending review
 
 It seems weird to have a prefixed version of text-decoration instead of just 
 making text-decoration allow the new prefixed keywords.
 
 What's the reason for having a whole new prefixed property?

Making regular text-decoration accept prefixed keywords for the new/unstable 
tuff sounds like a good approach to me. I too am curious why that is not 
proposed.

 - Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Enable MICRODATA feature by default

2012-07-31 Thread Maciej Stachowiak


Has anything been done to verify security and stability of the feature, for 
example, fuzz testing? I'd like to request that before enabling for Apple's 
ports.

 - Maciej

On Jul 30, 2012, at 11:35 PM, Arko Saha ngh...@motorola.com wrote:

 I am planning to enable MICRODATA feature by default in WebKit.
 
 Microdata master bug: https://bugs.webkit.org/show_bug.cgi?id=68609
 Microdata spec: 
 http://www.whatwg.org/specs/web-apps/current-work/multipage/microdata.html
 
 Currently we have support for Microdata DOM API's to interacting with 
 microdata using microdata attributes like itemscope, itemtype, itemprop, 
 itemref, itemid through JavaScript.
 Also we have added support for HTMLPropertiesCollection, PropertyNodeList 
 interfaces to intract with microdata item-properties.
 
 As of now, WebKit EFL and Blackberry port has already enabled MICRODATA by 
 default.
 Mozilla and Opera has also enabled the support for Microdata DOM API.
 
 Please let me know your thoughts on the same.
 
 Thanks and Regards,
 Arko
 ___
 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 memory instrumentation

2012-07-25 Thread Maciej Stachowiak


On Jul 25, 2012, at 2:08 AM, Yury Semikhatsky yu...@chromium.org wrote:

 
 
 On Tue, Jul 24, 2012 at 10:34 PM, Maciej Stachowiak m...@apple.com wrote:
 
 On Jul 24, 2012, at 12:39 AM, Yury Semikhatsky yu...@chromium.org wrote:
 
 
 
 On Tue, Jul 24, 2012 at 12:47 AM, Maciej Stachowiak m...@apple.com wrote:
 
 On Jul 23, 2012, at 8:09 AM, Yury Semikhatsky yu...@chromium.org wrote:
 
 
 
 First option we consider is to define a class with the same set of fields 
 as the instrumented one, then have a compile time assert that size of the 
 reference class equals to the size of the instrumented one. See 
 https://bugs.webkit.org/attachment.cgi?id=153479action=review for more 
 details.
 
 Pros: compile time error whenever size of an instrumented class changes 
 with the appropriate modifications to the instrumentation function.
 Cons: it will require each committer to adjust the reference class and the 
 instrumentation on any modification that affects size of the instrumented 
 class. Changes that don't affect size of the class will go unnoticed.
 
 What is the advantage of this approach compared to just using the sizeof 
 operator on the relevant classes?
 
 Regards,
 Maciej
 
 
 Summing up sizeof type of each field and parent class and comparing it to 
 sizeof the class would do the same thing except that we would need sometimes 
 to manually handle alignment discrepancies on different platforms.
 
 I'm not sure I understand the problem you are trying to solve. Isn't sizeof 
 on the class the correct answer? Why would you need to manually add sizeof 
 for the fields? You need sizes of objects referenced by pointer, but that 
 won't be affected by alignment issues.
 We use sizeof to report *this size, after that we need to go through the 
 fields and report referenced objects. The problem is that the set of fields 
 may change eventually and break the instrumentation. The compile check should 
 help developers not to forget to update the instrumentation and add traversal 
 code for the new fields.

It seems like the compile-time check would give false positives (if you add a 
non-pointer field) and false negatives (if you change the size of a buffer 
pointed to). The most obvious way to fix it would also be to update the size of 
the reference class and not do anything special about new pointers, which would 
be the wrong thing to do. So based on that, I'm not sure it's worth the cost. I 
think compile-time checks are more effective when the failure is more directly 
related to a developer mistake.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit memory instrumentation

2012-07-24 Thread Maciej Stachowiak


On Jul 24, 2012, at 12:39 AM, Yury Semikhatsky yu...@chromium.org wrote:

 
 
 On Tue, Jul 24, 2012 at 12:47 AM, Maciej Stachowiak m...@apple.com wrote:
 
 On Jul 23, 2012, at 8:09 AM, Yury Semikhatsky yu...@chromium.org wrote:
 
 
 
 First option we consider is to define a class with the same set of fields as 
 the instrumented one, then have a compile time assert that size of the 
 reference class equals to the size of the instrumented one. See 
 https://bugs.webkit.org/attachment.cgi?id=153479action=review for more 
 details.
 
 Pros: compile time error whenever size of an instrumented class changes with 
 the appropriate modifications to the instrumentation function.
 Cons: it will require each committer to adjust the reference class and the 
 instrumentation on any modification that affects size of the instrumented 
 class. Changes that don't affect size of the class will go unnoticed.
 
 What is the advantage of this approach compared to just using the sizeof 
 operator on the relevant classes?
 
 Regards,
 Maciej
 
 
 Summing up sizeof type of each field and parent class and comparing it to 
 sizeof the class would do the same thing except that we would need sometimes 
 to manually handle alignment discrepancies on different platforms.

I'm not sure I understand the problem you are trying to solve. Isn't sizeof on 
the class the correct answer? Why would you need to manually add sizeof for the 
fields? You need sizes of objects referenced by pointer, but that won't be 
affected by alignment issues.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] WebKit memory instrumentation

2012-07-23 Thread Maciej Stachowiak


On Jul 23, 2012, at 8:09 AM, Yury Semikhatsky yu...@chromium.org wrote:

 
 
 First option we consider is to define a class with the same set of fields as 
 the instrumented one, then have a compile time assert that size of the 
 reference class equals to the size of the instrumented one. See 
 https://bugs.webkit.org/attachment.cgi?id=153479action=review for more 
 details.
 
 Pros: compile time error whenever size of an instrumented class changes with 
 the appropriate modifications to the instrumentation function.
 Cons: it will require each committer to adjust the reference class and the 
 instrumentation on any modification that affects size of the instrumented 
 class. Changes that don't affect size of the class will go unnoticed.

What is the advantage of this approach compared to just using the sizeof 
operator on the relevant classes?

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Easing printf based debugging in WebKit with an helper.

2012-07-19 Thread Maciej Stachowiak


On Jul 19, 2012, at 11:01 AM, Oliver Buchtala oliver.bucht...@googlemail.com 
wrote:

 
 Hi,
 
 I am probably one of those people who much dislike printf-debugging.
 What is your problem with using a debugger?
 
 Maybe because the displayed information is not appropriate?
 E.g., you would like
 someString.utf8().data()
 instead of
 someString
 
 FWIW, there is a gdb python API for changing the behavior... so called pretty 
 printers.
 It is not too difficult to write such pretty-printers.
 
 Maybe providing a set of useful pretty-printers is a better approach than 
 providing a set of debug functions?

I would love to see a set of useful pretty printers that we could share. Do you 
have any we could use as a starting point?

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Easing printf based debugging in WebKit with an helper.

2012-07-19 Thread Maciej Stachowiak


On Jul 10, 2012, at 8:52 AM, Brady Eidson beid...@apple.com wrote:

 
 On Jul 10, 2012, at 5:25 AM, Alexis Menard alexis.men...@openbossa.org 
 wrote:
 
 On Mon, Jul 9, 2012 at 6:53 PM, Brady Eidson beid...@apple.com wrote:
 
 On Jul 9, 2012, at 2:43 PM, Alexis Menard alexis.men...@openbossa.org 
 wrote:
 
 Hi,
 
 For those who secretly use printf debugging :). I know the
 recommended way is to use a debugger and it's not the point of this
 discussion.
 
 A lot of us do this, and sometimes it's necessary.  I agree with the gripe 
 and support adding something easier.
 
 So I propose wtf() and its stream operator.
 
 Usage :
 
 wtf()HelloWorld34.53322323; will output : Hello World 3 4.53322
 
 There is no reason to bring in stream operators - that are willfully absent 
 from WebCore - just for debugging.
 
 
 But it's really nice for that purpose, and somehow match std::cout
 
 And we quite purposefully don't use std::cout in the project.
 
 Overloading functions works just as well.
 
 I'm not sure to understand what you mean here…
 
 I mean relying on C++'s overloading of functions for the different types 
 you'd like to printf debug.
 
 void debug(WebCore::String);
 void debug(WebCore::Frame*);
 void debug(WebCore::Node*);
 
 etc etc etc.
 
 debug(someFrame);
 debug(someNode);
 debug(someString);
 
 Especially that last one would help me from remembering how to type 
 printf(%s, someString.utf8().data()) which is all I've ever really wanted.

In principle, we could also have this support multiple arguments, so you could 
write:

debug(frame: , someFrame,  node: , someNode,  string, someString);

This would be no more verbose than the  style, but could compile to a single 
function call at the call site and therefore could be relatively compact. I 
would find this easier to deal with than a unary function or a printf-style 
format string. The way you'd do this is by defining template functions which 
call a unary overloaded function for each argument:

templatetypename A, typename B debug(A a, B b)
{
debug(a);
debug(b);
}

templatetypename A, typename B, typename C debug(A a, B b, C c)
{
debug(a);
debug(b);
debug(c);
}

templatetypename A, typename B, typename C, typename D debug(A a, B b, C c, D 
d)
{
debug(a);
debug(b);
debug(c);
debug(d);
}

... and so on up to some reasonable number of arguments.

Regards,
Maciej






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

2012-07-13 Thread Maciej Stachowiak


On Jul 13, 2012, at 5:57 AM, Stephen Chenney schen...@chromium.org wrote:

 
 I'd be happy to add a term to the cost function:
 Cost per year with good comments:  t_maintainComments * n_patches + 
 t_understandWithComment * n_engineersNeedingToUnderstand + 
 t_discoverAndFixBadCommend * n_badComments
 
 I don't doubt there are poor comments, both outdated and useless. That's a 
 reviewing failure. You have simply highlighted the fact that any standard for 
 comments requires reviewer attention. Hence cost of maintaining comments.

I agree that reviewers should try to prevent inclusion of low-quality comments. 
I think the best way to avoid poor comments is for reviewers to be skeptical of 
every comment they see, and ask if the same information can be expressed as 
well in the code itself. There may be cases where it can't, typically why 
comments that explain the reason for doing something. But if there are more 
comment lines than code lines in a function, it probably needs rewriting.

 
 Thus, I'm much more interested in comments that pass the filter of people who 
 prefer fewer comments and thus would spend their limited comment budget on 
 ones that truly have value, than comments from people who believe in adding 
 lots of comments. My Bayesian inference is that comments from the latter 
 group have much lower average value per comment. When adding a comment, you 
 should really think about whether the value outweighs the cost, just as when 
 adding a line of actual functional code.
 
 Yes. I don't think you'll find much disagreement. I don't believe anyone is 
 arguing for lots of comments. The primary focus of this discussion, as I 
 recall reading, is: (1) class and member comments that describe system 
 behavior, (2) comments on invariants in code and (3) references to sections 
 of the spec that define behavior, and where we deviate.

Really? I think there is an implicit assumption on the part of some that good 
comments means many comments, or at least, more than we typically have. For 
your specific examples, my opinions would be:

(1a) Class comments can be useful if the class cannot be adequately and clearly 
described by its name. But ideally they should describe local properties of the 
class, not global properties of the system. They should especially avoid 
documenting facts that may change without the class holding the comment itself 
changing.
(1b) Member comments are almost always bad. You will not see them at the point 
of use, and new uses will typically be produced by copying existing uses, not 
by reading the header. It is almost always superior to use a better name for 
the member. If you think you need a member comment, you almost surely gave the 
member a bad name.
(2) Invariants in the code should be ASSERTs or COMPILE_ASSERTs, not comments.
(3) Spec section references are almost always not worth it. People working on a 
particular aspect of Web technology should be aware of the relevant specs. And 
when the spec is updated, thus invalidating all the section numbers, you have a 
bunch of out-of-date comments.
(3b) Documenting intentional noncompliance with a standard may be useful, but 
the key is to explain *why* it's necessary to deviate from the standard, not 
just how we're deviating, otherwise the next person to look at the code won't 
know whether it's ok to change the code to be compliant.

Regards,
Maciej

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

2012-07-13 Thread Maciej Stachowiak


On Jul 13, 2012, at 11:03 AM, Dana Jansens dan...@chromium.org wrote:

 On Fri, Jul 13, 2012 at 1:56 PM, Ryosuke Niwa rn...@webkit.org wrote:
 On Fri, Jul 13, 2012 at 5:57 AM, Stephen Chenney schen...@chromium.org 
 wrote:
 
 I don't doubt there are poor comments, both outdated and useless. That's a 
 reviewing failure. You have simply highlighted the fact that any standard for 
 comments requires reviewer attention. Hence cost of maintaining comments.
 
 I don't know how to review a patch and make sure all relevant comments are 
 updated.
 
 As I have illustrated before, you can be modifying a function X, then a 
 completely random function A which calls B that in turn calls C that in turns 
 D ... that in turn calls X may have a comment dependent on the previous 
 behavior of X without ever mentioning X. How am I supposed to know that there 
 is such a comment?
 
 How is that different than the same question but replace comment with 
 behaviour? In both cases A is no longer doing what it expected. Something 
 is going to break, and A will have to be fixed/updated, comment included.

Wrong behavior can often be observed through automated testing. Wrong comments 
cannot.

 - Maciej


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

2012-07-13 Thread Maciej Stachowiak


On Jul 13, 2012, at 11:13 AM, Alec Flett alecfl...@chromium.org wrote:

 
 And yes while incorrect behavior can be observed through automated testing, 
 automated testing does not catch all incorrect behavior, especially 
 unexpected never-before-seen behavior. Why do you think people write fuzzers? 
 This is yet another way that folks are arguing comments can be wrong, code 
 can't [thanks to compilers and automated testing], so don't write excess 
 comments 

Do you think this is not a valid reason to express things in code instead of as 
comments whenever possible?

Regards,
Maciej

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

2012-07-12 Thread Maciej Stachowiak


On Jul 12, 2012, at 6:50 AM, John Mellor joh...@chromium.org wrote:

 
 
 To take an arbitrary example, lets say that while iterating through a 
 ListHashSet something causes entries to be deleted. Intuitively it seems this 
 needn't invalidate the iterator, as long as the entry the iterator is 
 currently pointing to isn't removed. But is that actually the case in this 
 particular implementation? A well-documented library like Java's 
 LinkedHashSet will warn you if the set is modified at any time after the 
 iterator is created, in any way except through the iterator's own remove 
 method, the iterator will throw a ConcurrentModificationException and that's 
 that. I just tried to find this out in WebKit and had to read though 
 ListHashSetIterator, ListHashSetConstIterator, ListHashSetNode, 
 ListHashSet::remove, ListHashSet::unlinkAndDelete, HashTable::remove, 
 HashTable::internalCheckTableConsistency, 
 HashTable::removeWithoutEntryConsistencyCheck, 
 HashTable::removeAndInvalidateWithoutEntryConsistencyCheck, 
 HashTable::invalidateIterators, HashTable::shrink, HashTable::rehash, 
 ListHashSet::add, HashTable::add, HashTable::makeKnownGoodIterator, 
 HashTableIterator, HashTable::AddResult and ListHashSetTranslator::translate, 
 and even learn about using the template keyword for disambiguation. 
 Eventually there was enough information to conclude that yes, it probably is 
 safe since the ListHashSetNodes are allocated on the heap by 
 ListHashSetTranslator::translate, so even though the HashTable invalidates 
 its own iterators and HashTable::rehash may reallocate its storage, the 
 actual ListHashSetNode pointed to by ListHashSetConstInterator should 
 continue to exist. But constantly having to do such deep research makes 
 coding highly inefficient, and there's a high risk of making errors.

I think documenting the public interfaces of core data structures is probably a 
worthwhile tradeoff, since they change rarely but are used in many places. In 
fact, ListHashSet already has a class comment and documents some method 
behavior that is not obvious from the signature. Covering the issue you mention 
seems worthwhile as well.

https://bugs.webkit.org/show_bug.cgi?id=91106

Regards,
Maciej

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

2012-07-12 Thread Maciej Stachowiak


On Jul 12, 2012, at 1:47 PM, Stephen Chenney schen...@chromium.org wrote:

 
 
 On Thu, Jul 12, 2012 at 3:44 PM, Dirk Pranke dpra...@chromium.org wrote:
 On Thu, Jul 12, 2012 at 10:53 AM, Ryosuke Niwa rn...@webkit.org wrote:
  On Thu, Jul 12, 2012 at 10:43 AM, Stephen Chenney schen...@chromium.org
  wrote:
 
  As several people have shown, it is quite easy to come up with a formula
  that shows the cost of maintaining comments is much lower than the cost of
  living without.
 
 
  I object to that conclusion. I've never seen any scientifically sound data
  posted on this thread either for or against having comments. Furthermore,
  just because we can come up with a formula doesn't mean that the formula
  models the nature of the world well.
 
 This is certainly true. I doubt you will see such a study, because
 it's very culturally-specific (in the sense that every group working
 on a shared code base is a culture).
 
 I should have been clearer. In this email thread there have been guesstimates 
 of the form:
 
 Cost per year with poor commenting: t_understandWithoutComment * 
 n_engineersNeedingToUnderstand
 
 Cost per year with good comments:  t_maintainComments * n_patches + 
 t_understandWithComment * n_engineersNeedingToUnderstand
 
 All costs are per-code unit and will likely vary depending on the code. We 
 are better off without comments if:
 
 t_understandWithoutComment  t_maintainComments * n_patches / n_engineers + 
 t_understandWithComment
 
 We can argue about the appropriate values for t_* and n_*. The primary 
 observation is that the benefit of comments rises as more engineers need to 
 understand the code and patch levels (behavior changes) stay reasonably 
 constant. More behavioral changes argue for fewer comments.

Surely we would expect the project's n_patches to scale approximately linearly 
with n_engineers. Or if not, I'm not super concerned about helping the 
engineers who are not contributing patches.

You've also failed to account for the cost of misleading comments, and the cost 
of comments that add no information value (like m_refCount++; // increase 
reference count by 1). These can  significantly hurt understandability. You may 
think my example of a low-information comment is a parody, but I've noticed 
that there is a high correlation between people who want to follow a policy of 
lots of comments and tendency to add comments almost exactly like that. If 
anyone doubts me, I can privately point you to some examples of comments in 
WebKit code that literally restate what the adjecent line of actual code does. 
I hate reading code like that, because it turns 1-page functions into 3-page 
functions. 

Thus, I'm much more interested in comments that pass the filter of people who 
prefer fewer comments and thus would spend their limited comment budget on ones 
that truly have value, than comments from people who believe in adding lots of 
comments. My Bayesian inference is that comments from the latter group have 
much lower average value per comment. When adding a comment, you should really 
think about whether the value outweighs the cost, just as when adding a line of 
actual functional code.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Parallel image decoders are up for review

2012-07-09 Thread Maciej Stachowiak


On Jul 9, 2012, at 1:30 AM, KwangYul Seo sk...@company100.net wrote:

 Hi,
 
 Our team at Company 100 has worked on parallel image decoders for past a few 
 weeks and some patches are pending for review now. Here is the master bug for 
 parallel image decoders:
 
 https://bugs.webkit.org/show_bug.cgi?id=90375
 
 For the overall architecture and design, please refer to the following design 
 document. We will update the design document as we change code after review.
 
 https://docs.google.com/document/pub?id=12gf7MhNHfupeR3GdRF-h2Vzg86viCbwXq8_JByHKlg0
 
 Our implementation shows considerable speedup in image intensive sites and no 
 performance regression in PLT. Please refer to the master bug for specific 
 numbers.
 
 We do understand many paralleization techniques make code so complex to the 
 level that can't be accepted. So we tried our best to reduce the complexity 
 of code, but reviewers can help us reduce it further. 

Some questions:

(1) Do you know of any real-world sites that get a significant benefit? The 
linked page mentions an artificial test case that gets a large benefit, but 
shows an inconclusive result for the more general page load benchmark cited. 
There are definitely image-heavy real-world sites out there, such as 
http://cuteoverload.com or flickr.com photostream pages.


(2) Did you do any testing of time to first paint in addition to page load time?

Cheers,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Class-level comments in the source code

2012-07-06 Thread Maciej Stachowiak


Starting a new thread, since this has wandered a fair bit off topic (and Eric 
had a good point that it's kind of a threadjack).

On Jul 6, 2012, at 12:54 PM, Per Bothner per.both...@oracle.com wrote:

 The biggest annoyance I found is lack of class-level comments.  For example
 what is an Interpreter?  How many instances are there in the system?
 (I.e. is it a singleton class?  Is there one per window? One per thread?)
 What is the relationship to JSGlobalData, JSGlobalObject, RootObject.
 There are a lot of these classes, and it takes quite a bit of staring at
 the code to figure it out. Worse, it's hard to remember it all, so if I
 come back to the codebase after working on something else I have to
 figure out all out again: I might remember some aspects (like a class
 starting with JS is probably some kind of JavaScript object), but not
 a lot of other relationships and properties of the classes.

My recollection is that in past discussions, most folks were in favor of 
class-level comments that describe the purpose and nature of the class, 
particularly if not self-evident from the name. I think we'd likely take 
patches to take such comments. We also generally like why comments inside 
functions - comments that explain why something is done that way. 


What we don't really like is function-level what comments - ones that just 
restate what the code says. Stuff like this:

// increment reference count by one
refCount++;

Comments like this make the function longer and harder to read, without 
actually adding explanatory value.

In other cases, comments describe what a sequence of steps does where often a 
well-named function could achieve the same explanatory purpose. Or they 
describe a precondition or postcondition where often an ASSERT would be better.


Class-level comments don't really fall into these categories, and certainly the 
purpose of some of the objects you mention is not self-evident from the name 
(as with e.g. AffineTransform).

Documenting ownership and lifetime relationships is also useful. We have tended 
to do that as diagrams separate from the code, since the places where it is 
most needed tend to be complicated clusters of related objects. Perhaps we 
could include references to such diagrams in class comments too, if anyone was 
willing to maintain such diagrams for an extended period.


Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Class-level comments in the source code

2012-07-06 Thread Maciej Stachowiak


On Jul 6, 2012, at 2:37 PM, Per Bothner per.both...@oracle.com wrote:

 On 07/06/2012 02:02 PM, Maciej Stachowiak wrote:
 
 [... reasonable stuff I fully agree with - but one question ...]
 
 Documenting ownership and lifetime relationships is also useful. We have 
 tended to do that as diagrams separate from the code, since the places where 
 it is most needed tend to be complicated clusters of related objects.
 
 Wouldn't such diagrams be harder to keep up-to-date than comments?

Not necessarily. When you change an ownership relationship, you may have to 
update information about several classes, some of which may not have their code 
directly changed in the patch that modifies ownership model. In such cases, it 
would be easier to change one document than to find and update all the affected 
places. And if you fail to find one of the relevant places, you may end up with 
a false comment about ownership, which is more dangerous than no comment at 
all. That being said, I have not seriously tried either approach in WebKit so 
it's hard to predict which would be better in practice.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Feature announcement: webkit-patch patches-to-review

2012-06-28 Thread Maciej Stachowiak


On Jun 28, 2012, at 6:44 AM, Thiago Marcos P. Santos tmpsan...@gmail.com 
wrote:

 Hi WebKit,
 
 webkit-patch patches-to-review was reworked [1] and now, instead of giving 
 you the list of attachment ID of all the patches pending review, it will give 
 you the list of bugs + description + age of patches pending review that has 
 you on the CC list. The age here stands for how long in days the patch is 
 waiting for review, not to the last time the bug was changed.
 
 Example:
 
 $ ./Tools/Scripts/webkit-patch patches-to-review
 Logging in as usern...@webkit.org...
 Bugs with attachments pending review that has usern...@webkit.org in the CC 
 list:
 http://webkit.org/b/bugid   Description (age in days)
 http://webkit.org/b/86310   [GStreamer] 
 media/media-continues-playing-after-replace-source.html fails (14)
 http://webkit.org/b/86615   [EFL][DRT] EFL DRT needs deletebutton controller 
 (17)
 http://webkit.org/b/82864   [EFL] LayoutTestController needs implementation 
 of setTabKeyCyclesThroughElements (29)
 http://webkit.org/b/84589   [EFL][DRT] Implementation of showModalDialog 
 needed (50)
 http://webkit.org/b/68511   [EFL] Script for running WebKit-EFL Unit Tests 
 (276)
 http://webkit.org/b/54373   CSS '+' combinator with more than 2 combinations 
 doesn't work (401)
 Total: 6
 $
 
 Try webkit-patch help patches-to-review for few more options.
 
 [1] https://bugs.webkit.org/show_bug.cgi?id=89470

Very cool. I wish we had a web page that gave that same info in a similarly 
compact and convenient form.

 - Maciej


___
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

2012-06-27 Thread Maciej Stachowiak

From your design document, it sounds like this approach retains
guardRef/selfOnlyRef, and will not let a disconnected subtree keep the owner
document's children alive. Am I understanding correctly?

- Maciej

On Jun 27, 2012, at 5:55 AM, Kentaro Hara hara...@chromium.org wrote:

I wrote a document and implemented a patch of a new reference counting
algorithm. The reference counting algorithm guarantees that If a Node
X has 1~ reference count, then all the Nodes in the same tree are kept
alive. No performance regression. No additional byte in each Node
object.

A design document:
https://docs.google.com/a/chromium.org/document/d/1uYHpq7u5Sslj54UgzXjA7pYR53XjidpBcrCa-neOGQs/edit?pli=1

A patch: https://bugs.webkit.org/show_bug.cgi?id=88834

Comments are appreciated! (You can add a comment on the document side
by side, or post a comment to bug 88834.)

Thanks!

On Tue, Jun 12, 2012 at 4:07 PM, Filip Pizlo fpi...@apple.com wrote:

On Jun 11, 2012, at 11:35 PM, Ojan Vafai o...@chromium.org wrote:

On Mon, Jun 11, 2012 at 9:32 PM, Filip Pizlo fpi...@apple.com wrote:

I think that a lot of the performance penalty can be alleviated by:

1) Moving rarely executed paths into non-inlineable methods.

2) Marking the various refing methods ALWAYS_INLINE, after you've moved as
much as possible out of line.

3) Using LIKELY and UNLIKELY where appropriate.

We should only add these when we can point to a measurable benefit IMO
(maybe that's what you meant by appropriate?). My experience is that
marking codepaths LIKELY/UNLIKELY very rarely has a measurable performance
improvement with modern compilers + x86 processors. I can't speak for arm
since I have less direct experience optimizing for arm.

It's more for the benefit of the compiler than the hardware. It can, for
example, improve register allocation if the compiler knows that in case of
interference between variables used predominantly on cold paths and
variables used predominantly on hot paths, it should spill the cold ones.
The following is a good example:

x = some expression;
if (--thingy) doStuff(); // not inlineable
use x;

The function call will interfere with all caller-save registers. Which is
most registers. The compiler can do one of three things: (1) spill x, (2)
put x in callee-save register, of which there is a limited supply, or (3)
put x in a caller save but insert save/restore code around the doStuff()
call. Last I checked, gcc would prefer (2) or (1) if it ran out of registers
unless it knew that doStuff() was called rarely. Hence the utility of using
UNLIKEY in this case. If you know that it is unlikely then you want the
compiler to go with option (3), which greatly relieves register pressure at
the cost of making the rare call slightly slower.

You're right that it's not a huge win or even a measurable win in many
cases. The conditions have to be just right - for one it has to be a case
where the compiler wasn't already smart enough to do the right thing. And
current compilers are quite smart indeed so the benefits of these macros
will converge to zero over time.

In my experience it's a benefit in inline functions that get called from
many places, since after a lot of inlining the compiler can use all the help
it can get in trying to unravel which parts of the code matter and which
don't. I've seen it result in a 5% progression in the past, though
admittedly it was in runtime functions that get inlined *everywhere* like
allocation and GC write barriers.

In this particular case, I have a hunch that these macros might make a
difference. It's worth a shot since adding them doesn't require much
effort.

The reason I suggest these things is that you're adding a lot of code in a
bunch of methods, but a lot of that logic looks like it will execute
infrequently. That reduces inlining opportunities. And in places where the
methods are still inlined, you're adding code bloat that reduces icache
locality and may blow out branch prediction caches.

I'm also not sure that your benchmark is conclusive. What does the perf
test have in common with things we know we care about, like say PLT?

-Filip

On Jun 11, 2012, at 8:45 PM, Kentaro Hara hara...@chromium.org wrote:

Thanks ggaren!

I filed a bug here: https://bugs.webkit.org/show_bug.cgi?id=88834

I believe you could achieve (a) (i.e., preserve all reachable nodes
without help from the JavaScript garbage collector) with these semantics
in
the C++ DOM:

= Design =

ref():

++this-refCount
++root()-refCount

deref():

--this-refCount
--root()-refCount

Actually I've tried the approach yesterday but confirmed 25.9%
performance regression, even if I have TreeShared hold a pointer to
the root. Please look at the WIP patch and the performance test in
https://bugs.webkit.org/show_bug.cgi?id=88834.

What I've learned is that we must not insert any line to ref() and
deref(),

Re: [webkit-dev] SH4, MIPS, and legacy-ARM assemblers in JavaScriptCore

2012-06-22 Thread Maciej Stachowiak


Is there a way to reduce these costs other than deleting the slower-maintained 
JITs? For example, could we temporarily freeze the JIT (perhaps the whole JSC 
engine somehow) at old versions somehow for architectures that may take time to 
catch up?

Regards,
Maciej

On Jun 22, 2012, at 10:52 AM, Oliver Hunt oli...@apple.com wrote:

 The problem is that as we make changes we end up breaking the SH4, MIPS, 
 ARMvOld builds, which we are ostensibly not allowed to do, and so have to 
 spend significant amounts of time trying to ensure that the builds don't 
 break/start failing horribly, and then having committed the patch[es] we have 
 to spend multiple build bot cycles discovering all the cases that we missed.
 
 This consumes a lot of time that would be better spent working on the higher 
 level portions of the JIT, that benefit all platforms.
 
 --Oliver
 
 On Jun 21, 2012, at 11:44 PM, Zoltan Herczeg wrote:
 
 Hi Filip,
 
 we (Gabor Loki and me) are the maintainers of the traditional ARM port,
 and we are willing to fix all issues. Just let us know what we need to do.
 You can assign the necessary bug reports to us and we are available in the
 #squirrelfish (or #webkit) channel as well.
 
 Regards,
 Zoltan
 
 Hi all,
 
 We are actively trying to improve the WebKit JavaScript engine
 (JavaScriptCore), with new debugging, profiling, memory efficiency, and
 performance features.  Because JavaScriptCore is a JIT-based engine, this
 inevitably means doing JIT work, which in turn includes adding new
 instructions to the JIT assemblers and changing the API between the
 assemblers and the JIT.
 
 Currently, the maintenance situation in the assembler layer is not great.
 We have three well-supported assemblers, X86-32, X86-64, and ARMv7. Then
 we have three assemblers that appear to be on life support: legacy
 (non-THUMB2, pre-v7) ARM, SH4, and MIPS.  It is increasingly painful to
 maintain these three barely-supported assemblers.  None of these
 assemblers has been updated to support the new JIT or interpreter
 infrastructure, and there appears to be no ongoing effort to do so.  That
 means that for progress to be made on X86 and ARMv7, we need to
 increasingly scatter #if ENABLE(...) noise throughout the system to keep
 those other assemblers building.  Neither the active JavaScriptCore
 contributors, nor those running the bots for those hardware platforms,
 appear to have much interest in maintaining those assemblers, other than
 the occasional build fix.
 
 This is not a good situation to be in.
 
 So, I am curious: is anyone shipping with the legacy ARM assembler, the
 MIPS assembler, or the SH4 assembler?
 
 As a secondary question, if you are shipping the legacy ARM assembler, are
 you doing so because you have legacy ARM hardware or because you have not
 had the chance to switch to the new ARM assembler in your codebase?
 
 -Filip
 
 
 ___
 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] Rename of selfOnlyRef to guardRef - ok if I change it back? (was Re: DOM tree traversal on detached nodes)

2012-06-15 Thread Maciej Stachowiak


On Jun 15, 2012, at 12:12 AM, Roland Steiner rolandstei...@google.com wrote:

 Sorry for the late reply, this thread flew under my radar.
 
 I made the original name change, because I honestly was entirely confused 
 about the meaning of selfOnlyRef (it's done by Node on Document, so what is 
 self? Why is it needed?) - so I guess we'll have to agree to disagree which 
 one is clearer. ^_-

I can see how the name is less than perfectly clear, but since it's generally 
called as doc-selfOnlyRef(), I think it's pretty clear that the Node doing the 
calling can't be the relevant self. To me, guardRef() sounded like something 
for short-term protection in the scope of a function (the way we often use a 
local RefPtr on an object that may be destroyed), rather than like what it 
actually does. I think the main downside to selfOnlyRef() is that it's not 
fully clear what self-only means even if you know what self is. But it's 
hard to explain fully in a function name w/o excess verbosity.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Rename of selfOnlyRef to guardRef - ok if I change it back? (was Re: DOM tree traversal on detached nodes)

2012-06-15 Thread Maciej Stachowiak

On Jun 15, 2012, at 10:31 AM, Darin Adler da...@apple.com wrote:

On Jun 15, 2012, at 10:15 AM, Maciej Stachowiak m...@apple.com wrote:

But it's hard to explain fully in a function name w/o excess verbosity.

I think my favorite excess verbosity version is refDocumentButNotOtherNodes.

Things I dislike about that one:

- Seems to imply that normal ref() refs other nodes, which it doesn't really
(though it does act as a refcount controlling whether the document will
possibly remove its children while still alive)
- Extremely vague about which other nodes, to the point of giving the
implication that ref() might ref completely arbitrary other nodes. Or else it
gives the impression that it's just the right ref to use on Document but not on
other classes.
- No obvious sensible name for the corresponding refcount data member
(m_refDocumentButNotOtherNodesCount does not seem clear to me).

The way I'd fully explain the design is as follows:

- Document has two refcounts, the normal refcount and the self-only refcount.
- The document will not be destroyed so long as either refcount is nonzero.
- If the normal refcount is zero but the self-only refcount is zero, the
document will remove all of its own children.
- Descendant nodes of the document should hold a self-only ref to their owner
document, not a regular ref, to avoid reference cycles.

I am not sure how to get the key points across without being accurate or
misleading. A version that I think explains the complete design without saying
anything false or misleading:

refTheDocumentItselfButUnlikeTheRegularRefDontPreventTheDocumentsChildrenFromBeingRemovedToAvoidCyclesWhenRefingTheOwnerDocument

To make a reasonable name we probably need to focus on one of these aspects.
Perhaps one approach is to focus on when and why you should use this call,
rather than what it does:

refAsOwnerDocument() / m_refCountAsOwnerDocument (or m_ownerDocumentRefCount)
refAvoidingCycles() (or cycleAvoidingRef()) / m_cycleAvoidingRefCount

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Rename of selfOnlyRef to guardRef - ok if I change it back? (was Re: DOM tree traversal on detached nodes)

2012-06-15 Thread Maciej Stachowiak


On Jun 15, 2012, at 12:22 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Fri, Jun 15, 2012 at 12:14 PM, Maciej Stachowiak m...@apple.com wrote:
 I am not sure how to get the key points across without being accurate or 
 misleading. A version that I think explains the complete design without 
 saying anything false or misleading:
 
 refTheDocumentItselfButUnlikeTheRegularRefDontPreventTheDocumentsChildrenFromBeingRemovedToAvoidCyclesWhenRefingTheOwnerDocument
 
 To make a reasonable name we probably need to focus on one of these aspects. 
 Perhaps one approach is to focus on when and why you should use this call, 
 rather than what it does:
 
 refAsOwnerDocument() / m_refCountAsOwnerDocument (or m_ownerDocumentRefCount)
 refAvoidingCycles() (or cycleAvoidingRef()) / m_cycleAvoidingRefCount
 
 We probably need to qualify kinds of cycles we're avoiding: ones through 
 descendents (or subtree); e.g. this doesn't avoid cycles with JSC/V8 objects.

That's one reason I like refAsOwnerDocument() slightly better. It tells you 
when to use it, and when Node.cpp does m_document-refAsOwnerDocument(), it 
will make sense in context.

However, if you wanted to make the other version more exactingly precise, you 
could say something like refAvodingCyclesWithDescendents().

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

2012-06-14 Thread Maciej Stachowiak


On Jun 14, 2012, at 1:47 PM, Ryosuke Niwa rn...@webkit.org wrote:

 
 On Thu, Jun 14, 2012 at 1:44 PM, Peter Kasting pkast...@chromium.org wrote:
 On Thu, Jun 14, 2012 at 1:39 PM, Elliot Poger epo...@chromium.org wrote:
 Can someone please remind me why IMAGE+TEXT even exists?
 
 Wouldn't it be simpler to just mark a test as follows?
 IMAGE : allow image failure; go red if there is a text failure
 TEXT: allow text failure; go red if there is an image failure
 IMAGE TEXT: allow text and/or image failure
 The distinction is that IMAGE TEXT will allow image, text, or both to fail, 
 thus making transitions among the three generate no events.  IMAGE+TEXT says 
 specifically that we expect both to fail and that if one starts passing, 
 someone should do something.  (For example, maybe someone checks in a partial 
 rebaseline where they miss the image expectations.)
 
 Not to bike-shed on anything, but I think we should rename Text and Image to 
 TextOnly and ImageOnly. Every single person I know, including myself, had 
 never got the distinction between IMAGE TEXT and IMAGE+TEXT without someone 
 explaining it to him/her .

I think IMAGE+TEXT is not a very useful distinction from TEXT either. I checked 
for uses of TEXT that is not IMAGE+TEXT in the Chromium TextExpectations, and 
it seems that nearly all instances fall into one of the two following 
categories:

1) text-only test, so IMAGE+TEXT would not have different semantics from TEXT 
(the vast majority)
2) Flaky test that may actually pass, so distinguishing what happens with the 
image result is of limited utility (most of these are also text-only tests; 
only a small subset even have an image result)

Thus, I think Fail and ImageOnlyFail would be more useful and understandable 
categories than {TEXT, IMAGE, TEXT+IMAGE, TEXT IMAGE}. Fail would have the 
semantic that a text failure is expected, and image result if any can either 
pass or fail.

Regards,
Maciej

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

2012-06-14 Thread Maciej Stachowiak


On Jun 14, 2012, at 9:06 PM, Adam Barth aba...@webkit.org wrote:

 On Thu, Jun 14, 2012 at 9:02 PM, Ojan Vafai o...@chromium.org wrote:
 
 Seems like it will be a common error to mark a reftest failure as
 ImageOnlyFail and then be confused why it's not working, no?
 
 Maybe that can be solved with another name, like PixelOnlyFailure.

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.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

2012-06-13 Thread Maciej Stachowiak


On Jun 13, 2012, at 1:32 PM, Geoffrey Garen gga...@apple.com wrote:

 These tests regularly timeout on the Chromium debug bots and occasionally
 timeout on the Apple Lion bots.
 
 WebKit has a clear policy about this: Tests must be fast enough not to time 
 out. We can fix this issue by making these tests shorter. 
 
 I don't really see the connection to an abstract debate about fuzzers. 
 Fuzzers can be short-running, and non-fuzzers can be long-running.

Also, if a fuzzer is deterministic (which it should be, if it's going to be in 
LayoutTests), there is probably a fairly mechanical way to split it into 
multiple faster tests.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

2012-06-13 Thread Maciej Stachowiak


On Jun 13, 2012, at 3:58 PM, Darin Adler da...@apple.com wrote:

 On Jun 13, 2012, at 3:53 PM, Dirk Pranke dpra...@chromium.org wrote:
 
 * we use \ (backslash) as a delimiter instead of : and =
 
 Seems worse to me. When I see a backslash I assume it’s a line continuation 
 character or a C escape sequence.
 
 On the other hand, neither the : nor the = meant anything to me either, 
 and I suggested dropping the delimiter entirely, so I’m not sure my feedback 
 will get listened to!
 
 * As of now the keywords remain all UPPERCASE. I'm happy to change them to 
 all lowercase or mixed case if there's a consensus that such a change is 
 desired.
 
 I DON'T LIKE READING THINGS IN ALL UPPERCASE AND WOULD VERY MUCH LIKE THAT TO 
 CHANGE.

I also hate all-caps keywords and would prefer mixed case or all-lowercase, but 
I'm willing to discuss that for a future round of changes if we can't get 
agreement now.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On Jun 10, 2012, at 9:26 AM, Ojan Vafai o...@chromium.org wrote:

 On Sun, Jun 10, 2012 at 4:54 AM, Balazs Kelemen kbal...@webkit.org wrote:
 So the unit tests are superfluous.  In particular, if I had to pick between 
 only having unit tests or only having regression tests, I might pick unit 
 tests.  But if I already have regression tests then I'm unlikely to want to 
 incur technical debt to build unit tests, particularly since unit tests 
 requiring changing the infrastructure to make the code more testable, which 
 then leads to the problems listed above.
 
 There are many code paths are used rarely. In practice, we were having 
 regressions frequently when people modified the code. Since the codebase has 
 been unittested, the rate of regressions has gone down considerably. The 
 time you spend dealing with tests is considerably less than the time you 
 spend rolling patches in an out as you encounter different edge cases that 
 different configurations/flags hit.
  
 
 A quick note to unittests. I think it's easy to define a hard limit for 
 unittests, which is that: if I want to add a feature, or some customizing 
 option for a particular port, it should be less effort to write the unittest 
 than to write the actual code. I heard from my colleges a few times that it's 
 not always the case with nrwt. I can imagine that it's not trivial to setup 
 the unittest system for a module that has not been unittested so far but I 
 think it should rather be the job of those who are actively working on the 
 test harness, not of those who just need some work to be done for their port.
 
 While this is a nice ideal to strive for, I don't think this ever plays out 
 for testing on any project, e.g. it is very frequently harder to write tests 
 for my WebCore changes than to make the change itself. Certainly anything we 
 can do to make testing easier is better, but I don't see NRWT as more 
 difficult to test than any other code in the WebKit project.
 
 WebKit has a policy of every change requiring tests. I don't see why tooling 
 should be any different. It's unfortunate that NRWT started with 0 tests, so 
 there are still (very few now!) parts that aren't tested. It's hard to test 
 those parts if that's what your modifying. However, it's *especially* for the 
 cases of port-specific code that need testing. Those are exactly the 
 codepaths that break from lack of testing.

Do we have some data that shows NRWT suffering fewer regressions (per unit time 
or per N changes) than ORWT?

I am strongly in favor of automated tests in general, but I'm skeptical of it 
here for two reasons:

1) I have found the hackability of anything involving webkitpy and its unit 
tests to be poor. It takes a long time to make a simple change, and the need to 
add tests or modify tests is certainly part of it.

2) For code that ships to end-users or third parties, I am a strong advocate of 
comprehensive testing. I think testing is worthwhile even if it were 
hypothetically the case that faith-based programming was less total work. That 
is so because we are trading off the time of a couple of hundred WebKit 
engineers for quality of software experienced by hundreds of millions of users. 
So it's worth it to incur significant test infrastructure costs to benefit a 
much greater number of users.

But for the case of internal tools, I think the tradeoff is fundamentally 
different. The costs of maintaining test infrastructure and the costs of 
dealing with regressions are borne by more or less the same set of people. So 
if the work to maintain unit tests is greater than the cost of just dealing 
with whatever regressions slip through, then it's probably not worth it.

My own gut feeling is that ORWT never experienced enough regressions to justify 
the cost of a unit testing system.

Regards,
Maciej



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] DOM tree traversal on detached nodes


On Jun 6, 2012, at 6:27 PM, Darin Adler da...@apple.com wrote:

 On Jun 6, 2012, at 6:14 PM, Kentaro Hara hara...@chromium.org 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 vaguely recall that I originally implemented rule (b) for WebKit, and at the 
time I did it because:

(1) The exact behavior in this case seemed unimportant; I couldn't find any 
sites depending on it, nor did it seem likely that they would.
(2) It was the simplest apparent way to avoid leaking certain kinds of cycles 
that span GC and refcounting (I don't remember the details and this is likely 
no longer applicable)/
(3) It seemed that keeping ancestors alive would have the potential to hold 
large amounts of memory when much of it was unneeded.

An even more extreme example of (3) would result if both parentNode and 
ownerDocument were strong references, and ownerDocument was made to be a real 
reference, not a self-only reference (at some point renamed to guardRef). Then 
keeping a single node that was in an isolated DOM subtree alive would also keep 
that whole document alive. I am not sure what other browsers do in this case.

Of course, predictability might outweigh the potential memory use issues.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Rename of selfOnlyRef to guardRef - ok if I change it back? (was Re: DOM tree traversal on detached nodes)


On Jun 11, 2012, at 6:06 PM, Maciej Stachowiak m...@apple.com wrote:

  not a self-only reference (at some point renamed to guardRef). 

BTW I was able to find where it was renamed but not a good explanation of why. 
I think selfOnlyRef() was a much clearer name.

The history seems to be that it was renamed when moved from Document to 
TreeScope (without explanation in the bug or ChangeLog, and apparently 
retaining it's self-only referencing behavior per comments):

http://trac.webkit.org/changeset/82882
https://bugs.webkit.org/show_bug.cgi?id=57689

Then later it was moved back to Document but retaining the rename:

http://trac.webkit.org/changeset/83123
https://bugs.webkit.org/show_bug.cgi?id=57994

Would anyone object if I renamed it back? Alternately, could the reason for the 
new name be documented somewhere?

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[webkit-dev] Adding nonstandard features (was Re: Adding ENABLE_NAVIGATOR_BUILDTYPE to WebCore)


On Jun 7, 2012, at 1:10 PM, Adam Barth aba...@webkit.org wrote:

 On Thu, Jun 7, 2012 at 1:00 PM, Ryosuke Niwa rn...@webkit.org wrote:
 On Wed, Jun 6, 2012 at 1:51 PM, Annie Sullivan sulli...@chromium.org
 wrote:
 
 I wanted to let you know that I plan to add support for
 navigator.buildType (e.g., nightly, beta, final) to WebKit. This
 feature isn't on the standards track (but neither are a bunch of other
 similar properties on Navigator). This feature will be behind the
 ENABLE(NAVIGATOR_BUILDTYPE) feature define. See:
 https://bugs.webkit.org/show_bug.cgi?id=88358
 http://html.spec.whatwg.org/#navigator
 
 What is the rationale for adding this property on the navigator object
 instead of the chrome object we also expose if your'e not expecting this
 property to be ever standarized?
 
 Generally, we avoid implementing web visible features via the chrome
 object because that makes them Chrome-proprietary.  In this case, it
 seems entirely reasonable for other browsers (e.g., Firefox) to want
 to implement this feature.  By putting it on navigator, we invite them
 to implement it as well.


This thread seems mostly over, but taking the opportunity to make a broader 
point:

If we want to invite other browsers to implement a feature, then we should 
propose it to the relevant standards group (and prefix it or just don't add it 
if it seems likely to be rejected). Just implementing unprefixed, without 
discussing with a relevant standards group first, is not the best approach. It 
can needlessly damage our relations with other implementors and the relevant 
standards groups themselves. While browser implementors in general, and the 
WebKit project in particular, have not always done a great job of this, this, 
we've been trying to do this more consistently since we adopted a process for 
reviewing feature additions. [1]  We may even want to update that policy page 
to mention that you'll very likely be asked, is this on the standards track? 
and if the answer is no you need a particularly compelling reason.

Regards,
Maciej

[1] http://www.webkit.org/coding/adding-features.html
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Renaming DumpRenderTree to WebKitTestRunner or merging those two

2012-06-03 Thread Maciej Stachowiak

Already done for the WebKit2 WebKitTestRunner. It should be straightforward to 
adapt its IDL compiler.

 - Maciej

On Jun 2, 2012, at 11:03 PM, Adam Barth aba...@webkit.org wrote:

 We might also want to define the layoutTestController object with IDL
 so we don't have to hand-write bindings every time we change it.
 Using IDL for window.internals seems to have worked out well.  It
 would be nice to replicate that success with layoutTestController.
 
 Adam
 
 
 On Sat, Jun 2, 2012 at 6:55 PM, Ryosuke Niwa rn...@webkit.org wrote:
 Hi,
 
 One thing that has annoyed me ever since I started working on WebKit is that
 our test runner is called DumpRenderTree. We fixed this problem in WebKit2
 by renaming it to WebKitTestRunner but we've still got DumpRenderTree in
 WebKit1 land.
 
 Can we either rename DumpRenderTree to WebKitTestRunner or merge those two
 programs into one?
 
 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

Re: [webkit-dev] Renaming DumpRenderTree to WebKitTestRunner or merging those two

2012-06-03 Thread Maciej Stachowiak

I am on vacation so I won't be able to review your patch in detail, but from 
your description it sounds less appealing to me than the WKTR approach. It 
seems like bad layering to me to define the IDL interface in WebCore for 
something actually implemented completely outside of WebCore. It also seems 
better to use the public interface to the JS engine rather than the internal 
interface, even though this requires additional codegen back ends (one of which 
is already written). But, I may be missing advantages to your approach, since I 
only gave it a quick glance.

 - Maciej

On Jun 3, 2012, at 5:17 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Sun, Jun 3, 2012 at 1:45 PM, Maciej Stachowiak m...@apple.com wrote:
 Already done for the WebKit2 WebKitTestRunner. It should be straightforward 
 to adapt its IDL compiler.
 
 I've considered adapting WebKitTestRunner's code generator, but it appeared 
 quite tricky because ports don't share much code in DumpRenderTree unlike 
 WebKitTestRunner so we end up having to significantly modify each port's 
 build configuration files for DumpRenderTree. Also, WebKitTestRunner's code 
 generator only supports JSC at the moment, and supporting V8 there will 
 require a significant work and might clutter the codebase.
 
 Given that, the approach I'm taking in 
 https://bugs.webkit.org/show_bug.cgi?id=88183 at the moment is re-use 
 Internals object's infrastructure. So I'm defining 
 LayoutTestControllerBase.idl (interfaceName=LayoutTestController) and 
 LayoutTestControllerBase.h/cpp in WebCore, and letting LayoutTestController 
 in DumpRenderTree override it.
 
 One big advantage of this approach is that code generator in WebCore already 
 supportsand V8, so we'll be able to share more code between JSC  V8 
 implementations of LayoutTestController. In fact, my current plan is to 
 replace all usage of JSStringRef, etc... (and their V8 equivalents) by 
 std::string so that they can be identical between two implementation
 Of course, one big disadvantage is that we'll be introducing a separate IDL 
 file for LayoutTestController. However, it appears that we can use the same 
 object injection mechanism for WebKitTestRunner as well. So I'm hopeful that 
 we can ultimately share IDLs between DumpRenderTree and WebKitTestRunner.
 
 What do you think of this approach?
 
 - Ryosuke
 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Renaming DumpRenderTree to WebKitTestRunner or merging those two

2012-06-03 Thread Maciej Stachowiak



Sent from my iPad

On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak m...@apple.com wrote:
 I am on vacation so I won't be able to review your patch in detail, but from 
 your description it sounds less appealing to me than the WKTR approach. It 
 seems like bad layering to me to define the IDL interface in WebCore for 
 something actually implemented completely outside of WebCore.
 
 While you're right that it's somewhat of a layer violation to define the IDL 
 for layoutTestController, WebCoreTestSupport appears to be the most logical 
 place to share files between DumpRenderTree and WebKitTestRunner at the 
 moment unless we're going to create another project/library in Tools.

The downside is that they would be using internal WebCore interfaces instead of 
the public interface as originally intended. I do not think that is a good 
change, nor does it seem required just to share more code.

 
 It also seems better to use the public interface to the JS engine rather than 
 the internal interface, even though this requires additional codegen back 
 ends (one of which is already written). But, I may be missing advantages to 
 your approach, since I only gave it a quick glance.
 
 My intention is to contain all interactions with the JS engine within 
 WebCoreTestSupport so that you'd never see JSStringRef, JSContextRef, etc... 
 (or their V8 equivalents) in LayoutTestController code.
 
 Now, we wouldn't have this problem if we were merging DumpRenderTree into 
 WebKitTestRunner, but that appears to be unrealistic at the moment as far 
 I've read the code.

I think that is actually a good longer term goal, though I can see that the way 
to get there is non-obvious. Even putting them in one directory to more easily 
share some files would be good IMO. The tricky bit would be to ensure that 
ports where WK1 and WK2 versions are both applicable can build both 
simultaneously.

(I also agree with your earlier comment that DumpRenderTree isn't the greatest 
name.)

Maybe Sam has thoughts, since I think he is the one who decided to make WKTR 
completely separate from DRT in the first place.

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

2012-05-31 Thread Maciej Stachowiak


On May 31, 2012, at 3:11 PM, Ryosuke Niwa rn...@webkit.org wrote:

 On Thu, May 31, 2012 at 11:56 AM, Darin Adler da...@apple.com wrote:
 I am thinking we should rename layoutTestController to testController. Or if 
 you don’t like that name, maybe testHarness or some even better name.
 
 testController seems like a misnomer since it doesn't really control the 
 test itself. I would prefer testRunnerController or simply testRunner since 
 it's quite self-evident that methods on testRunner would act on the test 
 runner itself.
 
 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.
 
 That sounds like a good idea.
 
 Can we also rename LayoutTests to RegressionTests? I know Dave (Hyatt) 
 suggested to cleanup the render tree dump format to get rid of all hacks and 
 tweaks we've added over years, and my preference is to combine all these 
 efforts and put new types of tests in trunk/RegressionTests. We'll move tests 
 from LayoutTests to RegressionTests as we convert. We'll get rid of 
 LayoutTests directory once all tests have been converted to use testRunner 
 and new render tree dump format.

I don't think renaming the directory should be tired to changing the output 
format. In particular, it would be confusing to have both RegressionTests and 
LayoutTests exist at the same time for an extended period. I don't think it has 
worked out very well to have both WebCore/platform/ and Platform/ exist at the 
same time, for instance. There are other ways to track an incremental 
conversion effort.

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] testharness Wiki page added

2012-05-31 Thread Maciej Stachowiak


On May 31, 2012, at 2:18 PM, Jacob Goldstein jac...@adobe.com wrote:

 I added the following Wiki page to provide some information on testharness.js 
 (the JavaScript framework from W3C recently landed in WebKit):
 http://trac.webkit.org/wiki/Writing%20testharness%20Tests
 
 I also updated the text under Writing JavaScript-based DOM only test cases 
 on this page 
 http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree.  
 
 Note that I am recommending the use of testharness.js / testharnessreport.js 
 over js-test-pre.js/js-test-post.js, when applicable, since tests written 
 using testharness can be copied to the W3C test repository with only minor 
 changes.  
 
 Please review and send me any feedback on these pages.  I am considering 
 adding a detailed example to the testharness wiki page to demonstrate how the 
 API should be used.

In my experience, the W3C's DOM test harness results in tests that are more 
verbose, are less readable, and which have less readable output, as compared to 
js-test-pre.hs. I wish we would improve the W3C's upstream test harness instead 
of degrading the quality of our own tests.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Device and page scaling

2012-05-30 Thread Maciej Stachowiak


On May 30, 2012, at 4:24 PM, John Mellor joh...@chromium.org wrote:

 
 Maciej Stachowiak wrote:
  Can you explain why the target-densitydpi feature even exists? It
  seems ill-conceived to me, and the most straightforward fix would
  be to remove it. I have not heard anyone explain the use case for
  it. (I'm also not clear on the details of what it actually does, and
  neither the name nor the docs are enlightening.)
 
 Designers who insist on pixel-perfect rendering can use width=device-width, 
 target-densityDpi=device-dpi to make their site render at one CSS pixel per 
 screen pixel, letting you get crisp non-scaled borders etc. It does however 
 require the designer to manually adjust dimensions and font-sizes to 
 compensate for the dpi using window.devicePixelRatio, which is incredibly 
 onerous (especially in a cross-platform design, which must look the same on 
 devices that don't support target-densityDpi, hence everything needs to be 
 implemented twice).
 
 That's the main use case, though it's pretty niche (if you want pixel-perfect 
 UI, it's generally less hassle to just use high resolution images and scale 
 them down). The other values I don't know of any compelling use cases for; 
 I'll talk to the engineer who first added this and see if they have any good 
 ones.

It seems to me that you could better address this use case by supporting 
fractional CSS pixels, which hopefully our new subpixel layout code can enable. 
The tricky thing about target-densityDpi=device-dpi is that it forces you to 
sniff the device pixel ratio and change pretty much all your layout based on 
it. If historically it would actually alter what is reported for 
devicePixelRatio, then it would be pretty hard to make a site design that looks 
right on both 1x and 2x devices with target-densityDpi=device-dpi. So I'm 
skeptical that anyone has made good use of it.

 
 I don't know whether or not we can remove it (would need to check how popular 
 it is), but it might be possible to deprecate it (recommend against using 
 it). That's probably something we should discuss on www-style rather than 
 here.

I guess that would be the right place to discuss dropping it from the spec, but 
it seems like here is the right place to discuss dropping it from the 
implementation. From comments in the CSS Device Adaptation spec, it seems like 
it was only added because it was in Android.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Device and page scaling

2012-05-29 Thread Maciej Stachowiak


Can you explain why the target-densitydpi feature even exists? It seems 
ill-conceived to me, and the most straightforward fix would be to remove it. I 
have not heard anyone explain the use case for it. (I'm also not clear on the 
details of what it actually does, and neither the name nor the docs are 
enlightening.)

Regards,
Maciej

On May 29, 2012, at 7:03 PM, Fady Samuel fsam...@chromium.org wrote:

 This sounds good to me, but is there any reason why we can't support physical 
 device changes (switching monitors) and support target-densitydpi? This would 
 be highly desirable for us.
 
 Fady
 
 == Page::effectiveDeviceScaleFactor ==
 
 Page::effectiveDeviceScaleFactor starts off as matching
 Page::deviceScaleFactor but changes to reflect any target-densitydpi
 directives.  It's unclear to me how Page::effectiveDeviceScaleFactor
 should react when the underlying physical device changes its scaling
 factor.  Currently, that shouldn't occur, so I'm inclined to add an
 ASSERT and worry about it later.
 
 == Settings::defaultDeviceScaleFactor ==
 
 This variable will be deleted.
 
 Thoughts?
 Adam
 
 ___
 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] Importing W3C tests to webkit

2012-05-23 Thread Maciej Stachowiak


On May 23, 2012, at 2:16 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Wed, May 23, 2012 at 1:41 PM, Ryosuke Niwa rn...@webkit.org wrote:
 The only sane argument I've heard so far to gate pixel tests is that the
 correctness of such tests need to be manually inspected, which requires
 a
 lot of manual labor and is very error prone.
 
 I'm assuming the above includes the ongoing maintenance cost of
 keeping pixel tests up to date, as well as the cost at the initial
 checkin.
 
 I'm not concerned of those. Once the correct expected result is checked in,
 it's pretty easy to rebaseline tests per rendering engine changes assuming
 people who are rebaselining tests know what they're doing.
 
 You should be concerned; keeping pixel tests up-to-date is clearly a
 non-zero cost that only the chromium port thus far has been willing to
 bear, and I suspect that the cost of updating baselines is
 substantially higher than the cost of the initial review over time
 (since it's a recurring cost).

Are you concerned just about the actual pixel results or also about keeping 
render tree dumps up to date? We can address the pixel result issue by 
introducing a new test that dumps its render tree but does not do pixel testing.

I think there is a high value to importing standards test suites wholesale, 
even if they overlap with our existing coverage. Picking and choosing subsets 
makes things  more complicated. If there are significant externalities to 
adding particular kinds of tests, I would prefer we mitigate those 
externalities rather than run fewer tests.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Importing W3C tests to webkit

2012-05-23 Thread Maciej Stachowiak


On May 23, 2012, at 3:13 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Wed, May 23, 2012 at 2:30 PM, Maciej Stachowiak m...@apple.com wrote:
 
 Are you concerned just about the actual pixel results or also about keeping 
 render tree dumps up to date?
 
 Both are more maintenance than a text-only test. In my experience,
 maintaining pixel tests is more expensive, but I also don't have any
 experience maintaining a non-pixel-test-running port.

There are occasional changes that require rebaselining a very large number of 
render tree dumps. In my experience, this happens much less often than the 
frequency at which Chromium ports update pixel results, by several orders of 
magnitude.

 
 We can address the pixel result issue by introducing a new test that dumps 
 its render tree but does not do pixel testing.
 
 Yes, it might make sense to do this (although it's not obvious to me
 how we would do this without modifying the test sources). Maybe we
 would need to maintain a separate manifest or some other list
 somewhere to indicate which dirs or tests should include pixel
 results.

The quick and dirty way would be to add a SkipPixel directive (or whatever) to 
TestExpectations.

Cheers,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Changes to window.focus/window.blur controlled by a setting?

2012-05-21 Thread Maciej Stachowiak


On May 21, 2012, at 12:16 PM, Andrew Wilson atwil...@google.com wrote:

 
 
 On Mon, May 21, 2012 at 2:17 AM, Jochen Eisinger joc...@chromium.org wrote:
 Hey,
 
 in https://bugs.webkit.org/show_bug.cgi?id=86969 I'm changing window.focus 
 and window.blur to match Firefox's behavior: window.blur does nothing, and 
 window.focus only works when invoked from the window that actually opened the 
 former.
 
 The goal is to thwart so-called pop unders.
 
 The new behavior you describe will break notifications, since many pages will 
 want to bring themselves to the front when someone clicks on their 
 notification,

Not necessarily for or against the change, but we could (and probably should) 
make notifications do this automatically. Is there any case where clicking a 
notifiicatin should not bring the relevant page to the front?

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Deprecation of CSSStyleDeclaration - getPropertyShorthand method.

2012-05-21 Thread Maciej Stachowiak


On May 21, 2012, at 11:09 AM, Alexis Menard alexis.men...@openbossa.org wrote:

 Hi,
 
 Let's try the new deprecation process documented as
 http://trac.webkit.org/wiki/DeprecatingFeatures.
 
 I would like to propose the deprecation and removal of
 getPropertyShorthand of CSSStyleDeclaration.
 
 State of art :
 - This method is exposed to the Web.
 - Its purpose is to get whether a given CSS property was set from
 within a shorthand (i.e.
 shouldBeEqualToString(test0.style.getPropertyShorthand('overflow-x'),
 overflow); is true if the CSS code is setting the overflow and not
 overflow-x).
 - It is used in 4 layout tests (fast/inspector-support/style.html,
 fast/css/font-shorthand.html, fast/css/overflow-property.html,
 fast/backgrounds/repeat/resources/background-repeat-shorthand.js)
 - It is exposed in the Objective C API.
 - It is not implemented by any other vendors (Opera, Firefox, Internet
 Explorer).
 - There is no specification about it.
 - It was added in 2005 http://trac.webkit.org/changeset/11481.

[...]

 This function was added by Dave Hyatt and reviewed by Maciej
 Stachowiak, maybe you guys can tell us why you added it back then (if
 your memory is very good as we are talking about 2005 material)?

I don't know of any reason for this to exist other than for benefit of the 
inspector. It appears to no longer be used by the Web Inspector, thought it has 
its own function with the same name (assuming I am reading the code right). It 
would be fine to stop exposing it to the web if it is in fact unused or hardly 
used.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Re: [webkit-dev] Changes to window.focus/window.blur controlled by a setting?

2012-05-21 Thread Maciej Stachowiak


On May 21, 2012, at 12:24 PM, Andrew Wilson atwil...@google.com wrote:

 
 
 On Mon, May 21, 2012 at 12:21 PM, Maciej Stachowiak m...@apple.com wrote:
 
 On May 21, 2012, at 12:16 PM, Andrew Wilson atwil...@google.com wrote:
 
 
 
 On Mon, May 21, 2012 at 2:17 AM, Jochen Eisinger joc...@chromium.org wrote:
 Hey,
 
 in https://bugs.webkit.org/show_bug.cgi?id=86969 I'm changing window.focus 
 and window.blur to match Firefox's behavior: window.blur does nothing, and 
 window.focus only works when invoked from the window that actually opened 
 the former.
 
 The goal is to thwart so-called pop unders.
 
 The new behavior you describe will break notifications, since many pages 
 will want to bring themselves to the front when someone clicks on their 
 notification,
 
 Not necessarily for or against the change, but we could (and probably should) 
 make notifications do this automatically. Is there any case where clicking a 
 notifiicatin should not bring the relevant page to the front?
 
 Yes. For example, in gmail, if you click on an email notification, we open a 
 new window and display that email. We don't want to bring the parent to the 
 front in that case.

Fair enough. But it seems like we could find some way to handle notifications 
as a special case without giving windows the ability to alter the stacking 
order at any time, even if my first proposal is too crude.

Cheers,
Maciej


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

2012-05-18 Thread Maciej Stachowiak


On May 17, 2012, at 5:39 PM, Dirk Pranke dpra...@chromium.org wrote:

 I probably polarized things by saying that your input was less
 valuable than those of people who were long-time users. I did not mean
 to offend, and I'm sorry. I certainly didn't mean to imply that I was
 not listening or not open to feedback from anyone, and I hope I've
 made that clear. That does not mean I will agree to any proposal
 without argument, obviously.

Apology accepted. I, in turn, am sorry for getting overly grumpy at you.


On May 17, 2012, at 4:57 PM, Ojan Vafai o...@chromium.org wrote:

 
 All the proposals that are not just bikeshedding we seem to agree on and will 
 happen (the half-dozen things I listed above).

I think it's great that we came up with some changes that seem useful and which 
no one is objecting to. We should certainly do those ASAP! Unfortunately they 
are buried in the middle of a megathread. Maybe it would be worth it to fork 
off a new thread just to notify the folks who may not be following this one, 
which would identify the changes and give examples of a new syntax.

We can discuss further changes separately, with that set as a baseline.

 Sure TEXT, IMAGE, etc are not very clear, but noone has actually proposed 
 something better.

I believe proposals were made which were more clear, but were rejected for 
other reasons (mainly not being as familiar to people used to the format 
afaict). Let me add another. I would propose the following replacements for 
current states:

neither TEXT nor IMAGE
   == (continue to say nothing)

TEXT or TEXT+IMAGE
   == FAIL
FAIL would mean the test fails - for text-only tests, it means text failure, 
for render tree tests it means text failure (who cares if the pixel test 
somehow accidentally pass at that point, that's not a meaningfully distinct 
state), for ref tests it would mean a reference failure

IMAGE
   == PIXELFAIL or PIXELONLYFAIL
This would be applied only to render tree tests and only in the case where only 
the pixel test mode fails, not text test. We have historically called this mode 
pixel tests not image tests, let's be consistent. Not applicable to text 
tests or reference tests. Mutually exclusive with FAIL.

TEXT IMAGE
  == FLAKY
If one of the text tests or the image tests will fail but maybe not both, that 
means the test is nondeterministic, so it should be marked as flaky and its 
results should not affect greenness of the bots, so long as it does not hang or 
crash. It doesn't seem like we currently have a FLAKY result expectation based 
on the bots, you are supposed to indicate it by listing all possible kinds of 
failures, but that seems unhelpful. Also, a flaky test that sometimes entirely 
passes on multiple runs in a row will turn the bots red, which seems bad. Let's 
just have FLAKY state instead where we don't get upset whether the test passes 
or fails.



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On May 16, 2012, at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote:

 
 There was a semi-logical basis, in that the stuff on the right of the
 test clearly specified the outcome of the test (PASS, IMAGE, TEXT,
 etc.). The stuff on the left was less well defined: there's the bug
 numbers, the platform/configuration info (MAC LINUX RELEASE, etc.),
 and some other stuff that there is less of a good place for (SLOW,
 SKIP, WONTFIX).
 
 I think it makes sense to syntactically separate at least the
 platform/configuration keywords from the outcome keywords. It might be
 nice to separate the other things somehow as well, but I don't have
 any great ideas for things that are clearly better than the existing
 left/right convention.

SKIP and WONTFIX seem parallel to PASS to me. I assume TEXT means a failure of 
text output and IMAGE means a pixel test failure? In that case those are 
parallel too.

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.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On May 17, 2012, at 11:28 AM, Ryosuke Niwa rn...@webkit.org wrote:

 On Thu, May 17, 2012 at 11:08 AM, Peter Kasting pkast...@google.com wrote:
 On Thu, May 17, 2012 at 9:26 AM, Dimitri Glazkov dglaz...@chromium.org 
 wrote:
 I actually quite like
 the clear delineation between test modifiers and test expectations.
 
 Me too.  Please please please leave them on opposite sides.  All these 
 proposals that try to put them both before the test name are much harder for 
 me to understand.  Even dropping the all-caps format for keywords is a loss 
 for me as this differentiation makes it easier for me to scan the line and 
 distinguish the three important pieces of a line: what, where, and how.  
 
 I find most of the changes that have been proposed so far to be a significant 
 decrease in clarity, except for the idea to change BUGXX### into links (for 
 all bug trackers, not just non-WebKit ones), which I think would be fine; and 
 the idea of moving SLOW, SKIP, and WONTFIX from the left side to the right, 
 as these seem more like expectations about how the test will run than 
 platforms on which the test fails.
 
 Interesting. I find the new format (I've replaced [] by () since that worked 
 better for me) much easier to read than the old format because now all meta 
 data is in one place. For me, I'm most annoyed by the unreasonably long bug 
 URLs in the new format. Maybe putting the URL isn't such a great idea after 
 all :( 

Perhaps aligning the fields column after the bug URL would improve readability 
(though it makes things a little harder to edit):

crbug.com/24182Slow (Mac Debug)   
fast/css/large-list-of-rules-crash.html
crbug.com/24182Slow (Linux Mac Debug) 
fast/dom/Window/window-postmessage-clone-really-deep-array.html
crbug.com/24182Slow (Mac Debug)   
fast/forms/select-set-length-with-mutation-remove.html
crbug.com/24182Slow (All) fast/js/regexp-overflow.html
crbug.com/24182Slow (Debug)   
fast/js/toString-and-valueOf-override.html
crbug.com/24182Slow (Debug)   html5lib/webkit-resumer.html
crbug.com/24182Slow (Win Release) 
http/tests/loading/onload-vs-immediate-refresh.pl
crbug.com/79910Image (SnowLeopard)
fast/text/international/bidi-linebreak-001.html
crbug.com/79910Image (SnowLeopard)
fast/text/international/bidi-linebreak-002.html
crbug.com/79910Image (SnowLeopard)
fast/text/international/bidi-linebreak-003.html
webkit.org/b/58924 Pass Timeout (Mac) 
plugins/mouse-click-iframe-to-plugin.html
webkit.org/b/60099 Pass Crash (Debug) 
fast/dom/Attr/access-after-element-destruction.html
webkit.org/b/60097 Pass Text (Debug)  
fast/dom/HTMLLinkElement/link-and-subresource-test.html


I would also suggest omitting (All), particularly if it turns out to be the 
common case.

Regards,
Maciej


 
 crbug.com/24182 Slow (Mac Debug) fast/css/large-list-of-rules-crash.html
 crbug.com/24182 Slow (Linux Mac Debug) 
 fast/dom/Window/window-postmessage-clone-really-deep-array.html
 crbug.com/24182 Slow (Mac Debug) 
 fast/forms/select-set-length-with-mutation-remove.html
 crbug.com/24182 Slow (All) fast/js/regexp-overflow.html
 crbug.com/24182 Slow (Debug) fast/js/toString-and-valueOf-override.html
 crbug.com/24182 Slow (Debug) html5lib/webkit-resumer.html
 crbug.com/24182 Slow (Win Release) 
 http/tests/loading/onload-vs-immediate-refresh.pl
 
 crbug.com/79910 Image (SnowLeopard) 
 fast/text/international/bidi-linebreak-001.html
 crbug.com/79910 Image (SnowLeopard) 
 fast/text/international/bidi-linebreak-002.html
 crbug.com/79910 Image (SnowLeopard) 
 fast/text/international/bidi-linebreak-003.html
 
 webkit.org/b/58924 Pass Timeout (Mac) 
 plugins/mouse-click-iframe-to-plugin.html
 
 webkit.org/b/60099 Pass Crash (Debug) 
 fast/dom/Attr/access-after-element-destruction.html
 webkit.org/b/60097 Pass Text (Debug) 
 fast/dom/HTMLLinkElement/link-and-subresource-test.html
 
 
 BUGCR24182 SLOW MAC DEBUG : fast/css/large-list-of-rules-crash.html = PASS
 BUGCR24182 SLOW LINUX MAC DEBUG : 
 fast/dom/Window/window-postmessage-clone-really-deep-array.html = PASS
 BUGCR24182 SLOW MAC DEBUG : 
 fast/forms/select-set-length-with-mutation-remove.html = PASS
 BUGCR24182 SLOW : fast/js/regexp-overflow.html = PASS
 BUGCR24182 SLOW DEBUG : fast/js/toString-and-valueOf-override.html = PASS
 BUGCR24182 SLOW DEBUG : html5lib/webkit-resumer.html = PASS
 BUGCR24182 SLOW WIN RELEASE : 
 http/tests/loading/onload-vs-immediate-refresh.pl = PASS
 
 BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-001.html = 
 IMAGE
 BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-002.html = 
 IMAGE
 BUGCR79910 SNOWLEOPARD : fast/text/international/bidi-linebreak-003.html = 
 IMAGE
 
 BUGWK58924 MAC : plugins/mouse-click-iframe-to-plugin.html = PASS TIMEOUT
 
 BUGWK60099 DEBUG : fast/dom/Attr/access-after-element-destruction.html = PASS 
 CRASH
 BUGWK60097 DEBUG :

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


On May 17, 2012, at 12:53 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 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.

Which you like better esthetically may be a matter of taste. But it's an 
objective, scientifically established fact that all-caps text is harder to read 
than lowercase or mixed case, and reduces reading speed:

http://en.wikipedia.org/wiki/All_caps#Readability
http://uxmovement.com/content/all-caps-hard-for-users-to-read/

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On May 17, 2012, at 3:37 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Thu, May 17, 2012 at 3:21 PM, Ryosuke Niwa rn...@webkit.org wrote:
 On Thu, May 17, 2012 at 3:01 PM, Dirk Pranke dpra...@chromium.org wrote:
 
 As I said before, I believe they increase the readability of the file.
 
 
 I see them as pure noise.
 
 
 Clearly, different people can have different syntactic preferences :).
 
 I believe the cost of learning to put delimiters in is near zero,
 
 
 That clearly isn't near zero. Or else people wouldn't be complaining about
 it. To quote Darin's response:
 
 Seriously, syntax is a significant barrier. Having to know which special
 characters to use. I don’t see this “clear delineation” you speak of. Just
 special punctuation I have to use to satisfy the computer
 
 You don't have to re-quote this, I already did in this thread (and
 responded to it).
 
 With all due respect to Maciej and Darin, neither of them have spent
 any significant amount of time working with test_expectations.txt
 files. While I appreciate that it's nice for the syntax to be
 approachable for newbies, I'm not inclined to bias in favor of newbies
 over people who are experienced. Of the people who have actually used
 the file, so far you're the only person who's spoken up as not liking
 them. Since different people prefer different things, I'm inclined to
 go with the majority of experienced users here. I am sorry if that
 means you lose out; I don't like it if anyone is unhappy and would
 prefer it if we could please everyone.

If you reject the input of people who are not yet users of 
test_expectations.txt, you probably won't get new users of 
text_expectations.txt. That would be bad for the project, so I hope that's not 
your final answer.

More generally, I think understandability (whether for news or experts) should 
take priority over familiarity. 

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.

Regards,
Maciej




___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On May 17, 2012, at 4:17 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Thu, May 17, 2012 at 4:00 PM, Maciej Stachowiak m...@apple.com wrote:
 
 On May 17, 2012, at 12:53 PM, Dirk Pranke dpra...@chromium.org wrote:
 
 On Thu, May 17, 2012 at 12:47 PM, Ryosuke Niwa rn...@webkit.org wrote:
 
 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.
 
 Which you like better esthetically may be a matter of taste. But it's an 
 objective, scientifically established fact that all-caps text is harder to 
 read than lowercase or mixed case, and reduces reading speed:
 
 http://en.wikipedia.org/wiki/All_caps#Readability
 http://uxmovement.com/content/all-caps-hard-for-users-to-read/
 
 
 Ooo! Citation fight!
 
 http://www.whatmakesthemclick.net/2009/12/23/100-things-you-should-know-about-people-19-its-a-myth-that-all-capital-letters-are-inherently-harder-to-read/
 http://www.microsoft.com/typography/ctfonts/wordrecognition.aspx

Your citations do not contradict mine. They dispute the mechanism that makes 
people read all-caps text slower, not the fact that it happens. Even if it's 
true that in theory people could be trained to read all-caps text just as 
quickly, I think it is unwise to make text files that require this uncommon 
skill.

Regards,
Maciej


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On May 17, 2012, at 1:42 PM, Ojan Vafai o...@chromium.org wrote:

 On Thu, May 17, 2012 at 1:37 PM, Peter Kasting pkast...@chromium.org wrote:
 On Thu, May 17, 2012 at 1:34 PM, Ojan Vafai o...@chromium.org wrote:
 2. Make outcomes optional. If they are left out, then the test is skipped 
 (unless the test is marked SLOW, in which case it's expected to pass). There 
 is no SKIP modifier.
 
 I don't think we should do this.  It seems very subtle.  I'd rather be 
 explicit.
 
 I'm OK with the rest of your numbered proposals.
 
 I disagree, but I'm fine with punting this to the list of controversial 
 changes that we should discuss separately. FWIW, my main motivation here is 
 that it allows us to unify the Skipped file format with the 
 test_expectations.txt format. But again, we can discuss that separately.

Adding SKIP (or whatever) to every line of skipped files is not a big hurdle, I 
think we could live with that is a transitions tep. I think the bigger hurdle 
is supporting chaining across multiple directories.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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


On May 17, 2012, at 4:40 PM, Dirk Pranke dpra...@chromium.org wrote:

 On Thu, May 17, 2012 at 4:16 PM, Maciej Stachowiak m...@apple.com 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.

Regards,
Maciej

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