Re: [webkit-dev] Rolling out a patch requires a justification beyond a test failure in downstream projects
Was this an isolated incident then? On Tue, Dec 11, 2012 at 8:39 PM, Ryosuke Niwa rn...@webkit.org wrote: On Tue, Dec 11, 2012 at 1:44 PM, Ryosuke Niwa rn...@webkit.org wrote: First off, I don't think we should be rolling out patches based solely on a downstream test unless there is a clear evidence that the failure is a real regression in WebKit that affects more than just the said downstream project. To understand this perspective, imagine that I've created a new app called SuperWebKitApp that's built on Chromium, EFL, GTK+, Qt, etc... ports in my spare time, and started rolling out all Chromium, EFL, GTK+, Qt, etc... patches that break automated tests in SuperWebKitApp, just giving hyperlinks to my buildbots. That's pretty upsetting, isn't it? - R. Niwa ___ 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] unsigned vs unsigned int
If someone wanted this and understood python, I suspect that they could search for unsigned in that cpp.py and in about 10 min have a pretty good idea of how to add such a check. If you don't understand Python, then 30 minutes :) dave On Thu, Sep 13, 2012 at 2:12 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: Filed https://bugs.webkit.org/show_bug.cgi?id=96693 On Thu, Sep 13, 2012 at 10:47 PM, Ryosuke Niwa rn...@webkit.org wrote: No. I think we can update webkitpy/style/checkers/cpp.py to do that. Unfortunately, I don't understand that code base. - Ryosuke On Thu, Sep 13, 2012 at 1:34 PM, Oliver Hunt oli...@apple.com wrote: Does the style bot pick up unsigned int etc? --Oliver On Sep 13, 2012, at 1:19 PM, Ryosuke Niwa rn...@webkit.org wrote: Landed in http://trac.webkit.org/changeset/128499 (Thanks ggaren!) On Thu, Sep 13, 2012 at 12:29 PM, Ryosuke Niwa rn...@webkit.org wrote: Uploaded a patch on https://bugs.webkit.org/show_bug.cgi?id=96682. - Ryosuke On Thu, Sep 13, 2012 at 8:11 AM, Dan Bernstein m...@apple.com wrote: On Sep 13, 2012, at 1:29 AM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: Hi there, I was telling people to use unsigned instead of unsigned int, as I have been told that was the preferred style before, but apparently that is not in the style guide. The question is, should it be? Yes. A few greps in the code: unsigned - 18406 occurrences. unsigned int - 1721 unsigned i = - 1548 It does in fact seem to be the preferred style. Cheers Kenneth -- Kenneth Rohde Christiansen Senior Engineer, WebKit, Qt, EFL Phone +45 4093 0598 / E-mail kenneth at webkit.org ﹆﹆﹆ ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev -- Kenneth Rohde Christiansen Senior Engineer, WebKit, Qt, EFL Phone +45 4093 0598 / E-mail kenneth at webkit.org ﹆﹆﹆ ___ 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] How to access page GroupSettings from WorkerThread
A few possibilities, in order of increasing complexity and work 1. If it can't change, you could pass it in when the worker is created. 2. If it can change, you do a postMessage to the main thread to get the value. 3. If you can't get the value in an async manner, perhaps you can make a method to allow thread safe access. On Wed, Jun 13, 2012 at 6:42 PM, Charles Wei charles@torchmobile.com.cn wrote: Hi, Webkit-dev, I am working on JSC binding for IndexedDB. One problem I am facing now is to access the page groupsettings from the WorkerContext(especially the SharedWorkerContext) for IndexedDB file path settings to access the database when the indexeddb code runs in the workerthread. Anybody can guide me what's the best way to do this: access the page groupsettings from a workcontext? Thanks --Charles From My BlackBerry - This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful. ___ 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] webkitPostMessage
wrt #1, I believe that postMessage implements what is in the spec and
webkitPostMessage additional has support for ArrayBuffers which wasn't in
the postMessage spec yet but was going to be added. If the behaviors from
webkitPostMessage were added to postMessage, then it coudl be removed.
wrt #2, I don't know. I think it will break some tests if you go with the
spec behavior, but if you wish to try this, I don't know of any big reason
not to.
dave
On Sun, Apr 29, 2012 at 11:01 AM, Adam Barth aba...@webkit.org wrote:
I read https://trac.webkit.org/wiki/DeprecatingFeatures, but I'm
still unsure how to proceed with removing webkitPostMessage and
aligning postMessage with the spec. No one responded to my earlier
message, so I'm inclined to just post a patch.
Many thanks,
Adam
On Tue, Apr 10, 2012 at 9:08 PM, Adam Barth aba...@webkit.org wrote:
I'm trying to understand why we have both DOMWindow.webkitPostMessage
and DOMWindow.postMessage. I'm also trying to understand the
following comment in {JS,V8}DOMWindowCustom.cpp:
// This function has variable arguments and can be:
// Per current spec:
// postMessage(message, targetOrigin)
// postMessage(message, targetOrigin, {sequence of transferrables})
// Legacy non-standard implementations in webkit allowed:
// postMessage(message, {sequence of transferrables}, targetOrigin);
Specifically:
1) Can we remove webkitPostMessage? If we can't remove it now, is
there a time in the future at which we can remove it?
2) Can we adopt the behavior in the specification (and drop the
non-standard behavior)? If not, should we change the specification to
match our behavior?
Many thanks,
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] Eliminate potential null pointer dereference?
I think this all started with a lot of effort put into fixing an issue reported by a user where they said the most popular online forum in Malaysia is broken. Then folks had to do a lot of builds (bisecting) to track down where the problem was introduced. Then they had to figure out what had broken, etc. It was mentioned (by gr...@chromium.org) that this very issue had already been flagged by own internal runs of coverity on chromium (including webkit). Now, it seemed a shame that we knew about issues in WebKit and were just ignoring them. It would be nice to be able to catch these issues faster rather than wait for a user to report it, etc. which makes the expense overall go up. So I believe there has been some effort invested in fixing some issues pointed out by coverity which is what these changes are and I believe coverity is mentioned in other changes of this sort. I understand the other side as well that it would be good to figure out if it is really an issue and find a test to prove it. I guess this is more of what I think of as a BSD type of approach. It seems to be an area where reasonable people can disagree. oth, regarding the style of this particular change, I find it unusual as well. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Eliminate potential null pointer dereference?
On Thu, Apr 19, 2012 at 11:36 PM, Ryosuke Niwa rn...@webkit.org wrote: On Thu, Apr 19, 2012 at 10:35 PM, David Barr davidb...@google.com wrote: Regarding style, the change homogenizes the loop constructs within that method. I don't think that's necessary an improvement. e.g. we don't have a style guide that mandates it. I completely agree with Maciej here that if this is a reachable code, then the patch author should put a reasonable effort into creating a test case. And most importantly, these changes are clearly not code cleanup. On Thu, Apr 19, 2012 at 11:11 PM, David Levin le...@google.com wrote: I understand the other side as well that it would be good to figure out if it is really an issue and find a test to prove it. I guess this is more of what I think of as a BSD type of approach. It seems to be an area where reasonable people can disagree. The WebKit contribution guide lists this as a requirement ( http://www.webkit.org/coding/contributing.html): For any feature that affects the layout engine, a new regression test must be constructed. If you provide a patch that fixes a bug, that patch should also include the addition of a regression test that would fail without the patch and succeed with the patch. If no regression test is provided, the reviewer will ask you to revise the patch, so you can save time by constructing the test up front and making sure it's attached to the bug. So I don't think we can disagree on this topic. I'm sympathetic to the argument that it's hard to come up with a test case for things like this, but then the patch author should clearly state that in the change log, and most importantly the reviewer should be asking that during the review. You seem to be a bit confrontational here. I'm not sure why. I was talking about about doing a patch for something where one wasn't able to find a repro but it appeared like an issue might be there. Not whether the changelog should say that or not. It may be good to ask a clarifying question if something is unclear as opposed to responding like this. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Eliminate potential null pointer dereference?
On Fri, Apr 20, 2012 at 1:39 PM, Maciej Stachowiak m...@apple.com wrote:
On Apr 19, 2012, at 11:11 PM, David Levin wrote:
I think this all started with a lot of effort put into fixing an issue
reported by a user where they said the most popular online forum in
Malaysia is broken. Then folks had to do a lot of builds (bisecting) to
track down where the problem was introduced. Then they had to figure out
what had broken, etc.
It was mentioned (by gr...@chromium.org) that this very issue had already
been flagged by own internal runs of coverity on chromium (including
webkit). Now, it seemed a shame that we knew about issues in WebKit and
were just ignoring them. It would be nice to be able to catch these issues
faster rather than wait for a user to report it, etc. which makes the
expense overall go up.
So I believe there has been some effort invested in fixing some issues
pointed out by coverity which is what these changes are and I believe
coverity is mentioned in other changes of this sort.
I understand the other side as well that it would be good to figure out if
it is really an issue and find a test to prove it. I guess this is more of
what I think of as a BSD type of approach. It seems to be an area where
reasonable people can disagree.
oth, regarding the style of this particular change, I find it unusual as
well.
If the change was attempting to fix an issue found by a specific static
analyzer, then it should say so in the ChangeLog instead of No new tests /
code cleanup only. That goes double if a lot of such changes are being
made, or people not in the know will be really confused (I don't think
Alexey was the only one).
Totally agreed. I think that was a mistake here. (I haven't been involved
in this error but I did happen to notice other patches that mentioned
coverity. This would should have as well.)
Also, the fact that it was found by a static analyzer does not exempt the
contributor from making a reasonable effort to create a test case. We do
accept that sometimes it's not practical and in that case it's ok to
mention in the ChangeLog why it was not possible to make a test case.
However, code cleanup only is not a reason that applies to changes made
with the intent of producing a behavior change.
Yes there should have been a reasonable effort.
If we had a static analyzer that ran automatically as part of the WebKit
development process, and a shared goal to get its complaints down to 0,
then it might be reasonable to skip creating tests for issues that it
diagnoses. But that doesn't seem to be the situation here.
I wonder why there would need this criteria. Is it better to leave in
obvious wrong code?
In this case the code looked like approximately like this before the change:
while (foo) {
}
for (foo = foo-next();...)
Now either that while loop should be while (true) or else foo should be
null checked. Ideally, this code wouldn't have passed the 1st code review
(and it is unlikely that a test would have been required to do the fix at
that stage).
dave
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Fwd: Please add EditBugs to my account
Done. On Mon, Mar 26, 2012 at 11:47 AM, Tony Payne tpa...@chromium.org wrote: Error message in webkit-patch upload says to email webkit-committers, but that message is not going through and I've been told this is the proper list to request the EditBugs permission. Thanks, Tony -- Forwarded message -- From: Tony Payne tpa...@chromium.org Date: Thu, Mar 22, 2012 at 3:39 PM Subject: Please add EditBugs to my account To: webkit-committ...@lists.webkit.org I'm working on a patch to WebKit and need to upload the patch to bugs.webkit.org. Would you please add EditBugs permission to my account ( tpa...@chromium.org)? Thanks Tony ___ 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] Throwing SECURITY_ERR on cross-origin window.location property accesses
On Thu, Mar 22, 2012 at 9:21 PM, Adam Barth aba...@webkit.org wrote:
I wonder if we could expose the parent document's origin, so you could
write the check as follows:
if (parent.location.origin != location.origin)
return;
It's slightly subtle because we wouldn't want to expose the origin of
child frames (then you could probe the redirect structure of private
networks), but exposing ancestor origins seems tenable.
Let's suppose I have some unannounced product like
unannounced.google.comand say it embeds an iframe.
Isn't it a information disclosure issue that anyone can now detect the
usage of unannounced.google.com when they are framed (especially if
unannounced were more descriptive)?
dave
Adam
On Thu, Mar 22, 2012 at 9:16 PM, David Levin le...@chromium.org wrote:
Suppose I am using a framework does window.frameElement for various
reasons. When that framework is used in an iframe, it causes errors to
appear in the console.
For example, suppose the framework does this:
var a;
try {
a = window.frameElement; // WebKit outputs a console error here.
if (!a)
return;
} catch (e) {
return;
}
...
Now if I had this new method, I would change the framework to do this:
var a;
try {
// WebKit doesn't throw an exception for x-origin parent access, but
it
will puts errors in the console.
// Do an explicit access check to avoid having the error appear in
the
console.
var canAccessParent = window.parent.webkitCanAccess;
if (canAccessParent != undefined !canAccessParent)
return;
a = window.frameElement;
if (!a)
return;
} catch (e) {
return;
}
...
Does that help? (The use case is about avoiding spurious console error
messages, so console error messages are real errors.)
dave
On Thu, Mar 22, 2012 at 8:44 PM, Sam Weinig wei...@apple.com wrote:
Hey Dave,
Can you describe the use case for this new property? When would an
author
use it?
-Sam
On Mar 22, 2012, at 4:16 PM, David Levin le...@chromium.org wrote:
Resurrecting a really old thread so continue the conversation now that
I'm
hitting this issue :).
On Mon, Aug 16, 2010 at 2:16 PM, Sam Weinig sam.wei...@gmail.com
wrote:
I am not sure I agree. Does our behavior actually cause any real bugs
in
the places you have tracked down? The log spam really doesn't seem
like an
issue, we can remove it if we want to, but have found it useful in the
past.
Speaking as someone who working on a web app, the log spam is
a significant issue because:
It clutters up the console output making it harder to find the real
error.
(It is hard to explain how much worse this makes the experience until
you
have to deal with a sophisticated web page. Image looking at the logs
from
your app and seeing lots of error messages -- many of which are this
one and
then a real one hidden among them -- from personal experience.)
When more sophisticated end users see it, they think there is a problem
in
the app and report it (and it could be that they include this in their
error
report and ignore other more important things).
I understand that that the console/log spam is useful to detect real
issues as well (given that WebKit doesn't throw an exception in this
case).
Would people find it acceptable to have window.webkitCanAccess so that a
web page can use that property first to detect the cross origin issue
and
avoid doing an access which would trigger the console spam? (I would
also be
fine with an exception instead of log spam.)
Thanks,
dave
___
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] Throwing SECURITY_ERR on cross-origin window.location property accesses
On Fri, Mar 23, 2012 at 12:13 PM, Adam Barth aba...@webkit.org wrote:
That's a risk, but you could also worry that the name would leak in the
iframe's document.referrer property.
Frankly, I'm in favor of anything to help me get rid of the console error
:).
I was concerned that exposing the parent origin has complications that will
make it harder for this to get into WebKit, etc. but if folks find that
acceptable, it scratches my itch.
dave
Adam
On Mar 23, 2012 12:02 PM, David Levin le...@chromium.org wrote:
On Thu, Mar 22, 2012 at 9:21 PM, Adam Barth aba...@webkit.org wrote:
I wonder if we could expose the parent document's origin, so you could
write the check as follows:
if (parent.location.origin != location.origin)
return;
It's slightly subtle because we wouldn't want to expose the origin of
child frames (then you could probe the redirect structure of private
networks), but exposing ancestor origins seems tenable.
Let's suppose I have some unannounced product like unannounced.google.comand
say it embeds an iframe.
Isn't it a information disclosure issue that anyone can now detect the
usage of unannounced.google.com when they are framed (especially if
unannounced were more descriptive)?
dave
Adam
On Thu, Mar 22, 2012 at 9:16 PM, David Levin le...@chromium.org wrote:
Suppose I am using a framework does window.frameElement for various
reasons. When that framework is used in an iframe, it causes errors to
appear in the console.
For example, suppose the framework does this:
var a;
try {
a = window.frameElement; // WebKit outputs a console error here.
if (!a)
return;
} catch (e) {
return;
}
...
Now if I had this new method, I would change the framework to do this:
var a;
try {
// WebKit doesn't throw an exception for x-origin parent access,
but it
will puts errors in the console.
// Do an explicit access check to avoid having the error appear in
the
console.
var canAccessParent = window.parent.webkitCanAccess;
if (canAccessParent != undefined !canAccessParent)
return;
a = window.frameElement;
if (!a)
return;
} catch (e) {
return;
}
...
Does that help? (The use case is about avoiding spurious console error
messages, so console error messages are real errors.)
dave
On Thu, Mar 22, 2012 at 8:44 PM, Sam Weinig wei...@apple.com wrote:
Hey Dave,
Can you describe the use case for this new property? When would an
author
use it?
-Sam
On Mar 22, 2012, at 4:16 PM, David Levin le...@chromium.org wrote:
Resurrecting a really old thread so continue the conversation now
that I'm
hitting this issue :).
On Mon, Aug 16, 2010 at 2:16 PM, Sam Weinig sam.wei...@gmail.com
wrote:
I am not sure I agree. Does our behavior actually cause any real
bugs in
the places you have tracked down? The log spam really doesn't seem
like an
issue, we can remove it if we want to, but have found it useful in
the
past.
Speaking as someone who working on a web app, the log spam is
a significant issue because:
It clutters up the console output making it harder to find the real
error.
(It is hard to explain how much worse this makes the experience until
you
have to deal with a sophisticated web page. Image looking at the logs
from
your app and seeing lots of error messages -- many of which are this
one and
then a real one hidden among them -- from personal experience.)
When more sophisticated end users see it, they think there is a
problem in
the app and report it (and it could be that they include this in
their error
report and ignore other more important things).
I understand that that the console/log spam is useful to detect real
issues as well (given that WebKit doesn't throw an exception in this
case).
Would people find it acceptable to have window.webkitCanAccess so
that a
web page can use that property first to detect the cross origin issue
and
avoid doing an access which would trigger the console spam? (I would
also be
fine with an exception instead of log spam.)
Thanks,
dave
___
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] Throwing SECURITY_ERR on cross-origin window.location property accesses
Resurrecting a really old thread so continue the conversation now that I'm hitting this issue :). On Mon, Aug 16, 2010 at 2:16 PM, Sam Weinig sam.wei...@gmail.com wrote: I am not sure I agree. Does our behavior actually cause any real bugs in the places you have tracked down? The log spam really doesn't seem like an issue, we can remove it if we want to, but have found it useful in the past. Speaking as someone who working on a web app, the log spam is a significant issue because: 1. It clutters up the console output making it harder to find the real error. (It is hard to explain how much worse this makes the experience until you have to deal with a sophisticated web page. Image looking at the logs from your app and seeing lots of error messages -- many of which are this one and then a real one hidden among them -- from personal experience.) 2. When more sophisticated end users see it, they think there is a problem in the app and report it (and it could be that they include this in their error report and ignore other more important things). I understand that that the console/log spam is useful to detect real issues as well (given that WebKit doesn't throw an exception in this case). Would people find it acceptable to have window.webkitCanAccess so that a web page can use that property first to detect the cross origin issue and avoid doing an access which would trigger the console spam? (I would also be fine with an exception instead of log spam.) Thanks, dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Throwing SECURITY_ERR on cross-origin window.location property accesses
Suppose I am using a framework does window.frameElement for various
reasons. When that framework is used in an iframe, it causes errors to
appear in the console.
For example, suppose the framework does this:
var a;
try {
a = window.frameElement; // WebKit outputs a console error here.
if (!a)
return;
} catch (e) {
return;
}
...
Now if I had this new method, I would change the framework to do this:
var a;
try {
// WebKit doesn't throw an exception for x-origin parent access, but it
will puts errors in the console.
// Do an explicit access check to avoid having the error appear in the
console.
var canAccessParent = window.parent.webkitCanAccess;
if (canAccessParent != undefined !canAccessParent)
return;
a = window.frameElement;
if (!a)
return;
} catch (e) {
return;
}
...
Does that help? (The use case is about avoiding spurious console error
messages, so console error messages are real errors.)
dave
On Thu, Mar 22, 2012 at 8:44 PM, Sam Weinig wei...@apple.com wrote:
Hey Dave,
Can you describe the use case for this new property? When would an author
use it?
-Sam
On Mar 22, 2012, at 4:16 PM, David Levin le...@chromium.org wrote:
Resurrecting a really old thread so continue the conversation now that I'm
hitting this issue :).
On Mon, Aug 16, 2010 at 2:16 PM, Sam Weinig sam.wei...@gmail.com wrote:
I am not sure I agree. Does our behavior actually cause any real bugs in
the places you have tracked down? The log spam really doesn't seem like an
issue, we can remove it if we want to, but have found it useful in the
past.
Speaking as someone who working on a web app, the log spam is
a significant issue because:
1. It clutters up the console output making it harder to find the real
error. (It is hard to explain how much worse this makes the experience
until you have to deal with a sophisticated web page. Image looking at the
logs from your app and seeing lots of error messages -- many of which are
this one and then a real one hidden among them -- from personal
experience.)
2. When more sophisticated end users see it, they think there is a
problem in the app and report it (and it could be that they include this in
their error report and ignore other more important things).
I understand that that the console/log spam is useful to detect real
issues as well (given that WebKit doesn't throw an exception in this case).
Would people find it acceptable to have window.webkitCanAccess so that a
web page can use that property first to detect the cross origin issue and
avoid doing an access which would trigger the console spam? (I would also
be fine with an exception instead of log spam.)
Thanks,
dave
___
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] Ambiguity in the style guide
On Mon, Mar 19, 2012 at 10:37 PM, Martin Robinson mrobin...@webkit.orgwrote: Hello WebKittens, While I am loathe to take up list space with another style guide threads, Eric Seidel recently pointed out to me some ambiguities in the style guide at https://bugs.webkit.org/show_bug.cgi?id=81602. Namely sections three and four of the #include Statements section. The relevant sections are: Other #include statements should be in sorted order (case sensitive, as done by the command-line sort tool or the Xcode sort selection command). Don't bother to organize them in a logical order. and Includes of system headers must come after includes of other headers. The ambiguities are: 1. Are WTF and other WebKit headers included like #include project/foo.h considered system headers? My guideline has been if the header is include with instead of , then it comes after, which is consistent with the sort order of and , so it all seems to come down to sort using ascii order. 2. Exactly what sort order is desired (e.g. capitals before lower case)? Yes (which is case sensitive, as done by the command-line sort tool) Hopefully this isn't seen as just a pedantic exercise. I'm interested in answering these questions so that I can modify check-webkit-style to catch these errors. On the other hand, if the exact nature of these rules is seen as unimportant, perhaps we could just remove that part of the guide. --Martin ___ 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] Separating ENABLE(NOTIFICATIONS) and ENABLE(LEGACY_NOTIFICATIONS)
Will LEGACY_NOTIFICATIONS cover everything that is in there right now? Hopefully host apps won't have to define both NOTIFICATIONS and LEGACY_NOTIFICATIONS to keep their current functionality since NOTIFICATIONS sounds like it will be guarding work that is in progress. dave On Tue, Mar 13, 2012 at 2:40 PM, Jon Lee jon...@apple.com wrote: It should reflect whatever is in the notification spec. In the end, when everyone has migrated to the spec, we should be able to get rid of all the #if ENABLE(LEGACY_NOTIFICATIONS) blocks. So LEGACY_NOTIFICATIONS should isolate aspects of notifications that are either replaced by a newer API, or have been removed altogether from the spec. Jon On Mar 13, 2012, at 1:38 PM, Jian Li jia...@chromium.org wrote: What will NOTIFICATIONS cover after LEGACY_NOTIFICATIONS is being added? Does it cover new syntax only or any syntax that are not considered old? Jian On Tue, Mar 13, 2012 at 1:29 PM, Jon Lee jon...@apple.com wrote: LEGACY_NOTIFICATIONS, for the most part, is exactly what NOTIFICATIONS covers now. So yes, it includes HTML notifications and old syntax, and will not remove anything that already exists. Jon On Mar 13, 2012, at 1:25 PM, Jian Li jia...@chromium.org wrote: Jon, could you please provide what are going to be included in LEGACY_NOTIFICATIONS? Does LEGACY_NOTIFICATION only includes HTML notification and old syntax we're considering to deprecate? Jian On Mon, Mar 12, 2012 at 7:11 PM, Adam Barth aba...@webkit.org wrote: That sounds like a good approach. Chromium will likely need to remember to disable NOTIFICATIONS on any upcoming release branches (until the work is complete). Adam On Mon, Mar 12, 2012 at 6:58 PM, Jon Lee jon...@apple.com wrote: Hi WebKit! In order to ease the migration path for the nascent notifications API, I'd like to separate the current dependency between NOTIFICATION and LEGACY_NOTIFICATIONS. Currently, in order to support the legacy API, both defines are needed, but ends up also including the new API. Since the future is to eventually move to the spec'd API, I like to separate the two defines, so that NOTIFICATIONS covers the new API, and LEGACY_NOTIFICATIONS the previous one. Currently all ports that support notifications will support both. https://bugs.webkit.org/show_bug.cgi?id=80922 tracks the work, and once the patch lands, ports that wish to avoid exposing the new API should remove the NOTIFICATION define. Any concerns? Thanks, Jon ___ 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] Separating ENABLE(NOTIFICATIONS) and ENABLE(LEGACY_NOTIFICATIONS)
On Tue, Mar 13, 2012 at 2:42 PM, David Levin le...@chromium.org wrote: Will LEGACY_NOTIFICATIONS cover everything that is in there right now? By in there, I meant in WebKit and what hosts have been shipping. Hopefully host apps won't have to define both NOTIFICATIONS and LEGACY_NOTIFICATIONS to keep their current functionality since NOTIFICATIONS sounds like it will be guarding work that is in progress. dave On Tue, Mar 13, 2012 at 2:40 PM, Jon Lee jon...@apple.com wrote: It should reflect whatever is in the notification spec. In the end, when everyone has migrated to the spec, we should be able to get rid of all the #if ENABLE(LEGACY_NOTIFICATIONS) blocks. So LEGACY_NOTIFICATIONS should isolate aspects of notifications that are either replaced by a newer API, or have been removed altogether from the spec. Jon On Mar 13, 2012, at 1:38 PM, Jian Li jia...@chromium.org wrote: What will NOTIFICATIONS cover after LEGACY_NOTIFICATIONS is being added? Does it cover new syntax only or any syntax that are not considered old? Jian On Tue, Mar 13, 2012 at 1:29 PM, Jon Lee jon...@apple.com wrote: LEGACY_NOTIFICATIONS, for the most part, is exactly what NOTIFICATIONS covers now. So yes, it includes HTML notifications and old syntax, and will not remove anything that already exists. Jon On Mar 13, 2012, at 1:25 PM, Jian Li jia...@chromium.org wrote: Jon, could you please provide what are going to be included in LEGACY_NOTIFICATIONS? Does LEGACY_NOTIFICATION only includes HTML notification and old syntax we're considering to deprecate? Jian On Mon, Mar 12, 2012 at 7:11 PM, Adam Barth aba...@webkit.org wrote: That sounds like a good approach. Chromium will likely need to remember to disable NOTIFICATIONS on any upcoming release branches (until the work is complete). Adam On Mon, Mar 12, 2012 at 6:58 PM, Jon Lee jon...@apple.com wrote: Hi WebKit! In order to ease the migration path for the nascent notifications API, I'd like to separate the current dependency between NOTIFICATION and LEGACY_NOTIFICATIONS. Currently, in order to support the legacy API, both defines are needed, but ends up also including the new API. Since the future is to eventually move to the spec'd API, I like to separate the two defines, so that NOTIFICATIONS covers the new API, and LEGACY_NOTIFICATIONS the previous one. Currently all ports that support notifications will support both. https://bugs.webkit.org/show_bug.cgi?id=80922 tracks the work, and once the patch lands, ports that wish to avoid exposing the new API should remove the NOTIFICATION define. Any concerns? Thanks, Jon ___ 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] Moving to Git?
On Fri, Mar 9, 2012 at 4:01 PM, Ryosuke Niwa rn...@webkit.org wrote: git will touch Node.h twice by stashing and applying. This would mean that I would be rebuilding the world even if all changes I get from masters were in webkitpy or LayoutTests. Are there an easy way to work around this issue as well? (other than committing changes, of course) afaik not really, here's two that I've used: 1. git-new-workdir (multiple git working dirs so I don't switch branches as often) 2. ccache (cache the results of the compile) dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] optimizing browser handling of Facebook Timeline scrolling
Not sure what tools you have used but you may find this helpful: http://code.google.com/webtoolkit/speedtracer/ On Sat, Feb 11, 2012 at 10:02 PM, Steven Young styoung.bi...@gmail.comwrote: [cross posting from mozilla's dev lists] I'm on the Timeline team at Facebook, which is going to be the new format for everyone's profiles real soon now. https://www.facebook.com/about/timeline We'd like to improve its browser performance, so I'd appreciate any suggestions for things we should change to speed it up. In particular, we'd like to make scrolling down through new content smoother. There are often brief (e.g. 300 ms) browser lockups, and other times there just seems to be a general feeling of heaviness. I'm going to list some of the specific issues we've identified, which we are debating how best to fix, but I'm also very interested to hear whatever anyone else thinks are the biggest perf bottlenecks. A few problems: (1) HTML / DOM size and CSS Our HTML is huge. About half of it is coming from the light blue like/comment widgets at the bottom of most stories. Within those widgets, a major portion of it is always the same. (Some of that is only needed once the user clicks into the widget, but we don't want another server round trip to fetch it.) We also have a lot of CSS rules, and applying all that CSS to all those DOM nodes gets expensive. Experimentally, removing all like/comment widgets from the page does give noticeably smoother scrolling, although it doesn't completely fix the problem. Related: We've also noticed that if you scroll very far down a content-rich timeline, and then open and close the inline photo viewer, this causes a noticeable lag, as it re-renders all existing content on the page. To fix this, we investigated dynamically removing offscreen content from the DOM and replacing it with empty divs of the same height, but we decided it wasn't worth the code complexity and fragility. (2) Repaints There are several fixed elements on the page like the blue bar at the top, the side bar, and our date navigator with the months/years. Chrome's --show-paint-rects flag showed that under most circumstances these fixed-position elements forced full-screen repaints instead of incremental repaints. The rules for what triggers a repaint vary from browser to browser, but we would ideally like to fix this everywhere. The cost of full page repaints also sometimes varies dramatically even comparing Chrome on two fairly newish Mac laptops. (3) Javascript for loading content as you scroll down We dynamically load timeline sections (e.g. a set of stories from 2009) using our BigPipe system (https://www.facebook.com/note.php?note_id=389414033919) in an iframe. In a nutshell, the HTTP response to the iframe is sent with chunked encoding, a script tag at a time. Each script tag contains some code and and HTML content that is passed up to the parent window, which requests the CSS and JS associated with that HTML content. Once the CSS is downloaded, the HTML (timeline story markup) is inserted into an offscreen DOM element. Then, once the JS is loaded, we do some fairly complicated work before we actually display the content. First, we lay out the timeline stories in an offscreen element (position:absolute; left:-px) before inserting them into the viewable page. We then have JS which checks the heights of all the stories on in the offscreen element so it can swap stories back and forth between the two columns, to keep things sorted by time going down the page. To do this, we query and cache the stories' offsetTop values all at once where possible. Probably, we could eliminate all this height-checking and column balancing if we implemented a machine learning algorithm to predict the height of each unit in advance, on the server side. Next, in an attempt to reduce user-percieved browser freezing while scrolling, our JS does not add new content in to the bottom of the main column as soon as it comes back from the server. Instead, we queue it up until the user stops scrolling and add it in then. We use document fragments where possible to insert elements. Web Inspector's profiler showed improvements when dynamically inserting many link rel=stylesheet tags in this fashion since we stopped thrashing between style recomputation and JS execution for each stylesheet, and instead just had one longer style recomputation segment. We throttle scroll/resize events so they fire every 150 ms All the while this is happening, we're potentially receiving more script tags in the iframe and doing the same thing for other pieces of content. We would love any pointers you guys have. Thanks, Steve ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list
Re: [webkit-dev] Web Notifications API
On Mon, Feb 13, 2012 at 5:29 PM, Adam Barth aba...@webkit.org wrote: On Mon, Feb 13, 2012 at 5:23 PM, Jon Lee jon...@apple.com wrote: I also have concerns about backwards compatibility support. Aside from Gmail, what other web sites have integrated the notifications feature? I could only find example pages, one of which was using already an outdated API. IRCCloud is an example of a site that I use every day that uses notifications. Google Calendar fwiw ( http://www.howtogeek.com/howto/28573/how-to-enable-desktop-notifications-for-google-calendar-in-chrome/ ) 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] HTML notification usage
TLDR Sorry for the long delay in responding but the chromium folks need to delay slight longer before giving a response. On Mon, Jan 16, 2012 at 1:36 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: Could anyone brief explain the relation to http://www.html5rocks.com/en/tutorials/notifications/quick/ and http://dev.w3.org/2006/webapi/WebNotifications/publish/Notifications.html ? I haven't followed the work with regard to notifications, so is that work dead or are we removing legacy code? afaik Chromium is the only browser to support HTML notifications (maybe QT does as well? I thought I saw some patch there) and I don't think any other browser wants to support them. However other browsers are supporting text notifications. There are some Chromium extensions that potentially use them, so that creates some trouble for us to remove them but it doesn't necessarily make it impossible. On Mon, Jan 16, 2012 at 10:25 PM, Jon Lee jon...@apple.com wrote: Hello all, I've removed support for HTML notifications on the Mac via http://trac.webkit.org/changeset/105086. Does anyone find much usage for this kind of notification on their respective platforms? Would it be appropriate to remove support for this altogether? We're discussing this with folks that have the biggest stake. We're working as fast as we can and making progress in reaching a consensus -- thanks to Jian Li -- (but have been slightly slowed down by the snow fall up here in the Seattle are which has made our communications). Personally, I'd like to get rid of them BUT that decision isn't up to me, so we need a few more days to talk to people and come up with an appropriate course of action. (If we want to get rid of it, we'll likely need to leave it in for a bit to give users a little time to transition off of it.) (Hopefully the snow will melt shortly and it will make people more accessible.) dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] chromium-cg-mac results
On Tue, Jan 3, 2012 at 8:33 PM, Dirk Pranke dpra...@chromium.org wrote: On Tue, Jan 3, 2012 at 3:28 PM, Adam Barth aba...@webkit.org wrote: On Tue, Jan 3, 2012 at 3:22 PM, Nico Weber tha...@chromium.org wrote: On Tue, Jan 3, 2012 at 3:00 PM, Adam Barth aba...@webkit.org wrote: It looks like Chromium Mac has successfully moved to Skia. I'd wait with this assessment until a version of Chrome with Skia has shipped to stable. Things are looking really good so that should be smooth sailing, but it's a bit early to say we're successfully moved :-) Fair enough. However, I believe the Skia transition plan called for removing the chromium-cg-mac expectation much earlier than a Skia build shipping to stable. Originally, we were only supposed to have to maintain both sets of expectations for about a month. The transition has taken longer than expected, but it seems like we have sufficient confidence in Skia now that we can remove the chromium-cg-mac expectations. Has the skia transition hit beta yet? It seems like as soon as we get it onto a version that is pointing to a branched version of webkit, we should be completely safe to remove the directories on trunk (frankly, I'd agree with Adam that it's probably safe to remove it now, since we can always add them back in if we have to, but I can compromise as well). Remove it. It is a cost on everyone who enlists in WebKit. Those of us who aren't creating new enlistments are not affected much but that doesn't mean it isn't costly. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Announcement of Coding Style Change in WebKit EFL port
Feel free to patch the style checker as appropriate. I suspect there may be some changes needed here: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/style/checker.py#L180 Best wishes, dave On Tue, Dec 27, 2011 at 11:54 PM, Gyuyoung Kim gyuyoung@samsung.comwrote: Hello WebKit folks, As you may know, I have changed the coding style of EFL port via Bug 68209. This is to eliminate unnecessary difficulties that reviewers have experienced when they review EFL bugs, due to EFL specific coding style. Bug 68209 - [EFL] Change WebKit EFL coding style (https://bugs.webkit.org/show_bug.cgi?id=68209) Major changes are as below, 1. Use full variable name instead of abbreviation name. 2. Use standard boolean type instead of EFL boolean type. 3. Place '*' operator immediately after the data type. (For example, use char* userAgent. instead of char *userAgent.) 4. Use C++ type casting instead of C type casting. 5. Remove void parameter from internal functions. 6. Use camelCase instead of underbar(_) in variable names. I made a wiki page for this rule. I will update this page whenever EFL port coding style guide is changed. (http://trac.webkit.org/wiki/EFLWebKitCodingStyle) To sum up, EFL port adheres to WebKit coding style except for public header from now. (Because public header is used by EFL applications, public header will continue to follow the coding style rule of EFL.) We hope that reviewing of EFL port patches will be much easier after this change. Happy new year !! Regards, Gyuyoung Kim. ___ 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] WebKit branch to support multiple VMs (e.g., Dart)
On Wed, Dec 7, 2011 at 1:13 AM, Dominic Cooney domin...@chromium.orgwrote: On Wed, Dec 7, 2011 at 4:53 PM, Oliver Hunt oli...@apple.com wrote: Web Content Engine The project's primary focus is content deployed on the World Wide Web, using standards-based technologies such as HTML, CSS, JavaScript and the DOM. --Oliver I intended the question about whether standards compliance was a goal rhetorically, in that it seems to me that this goal is honored or ignored capriciously. Every issue must be viewed in its own context and conflating them doesn't help make either issue clearer unless there is concerning pattern (but one issue isn't that). With respect to the issue that you're passionate about, Dominic, perhaps a more targeted conversation is in order. best wishes, dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] run-webkit-tests is moving to parallell testing by default (this weekend)
I believe there are some tests (copy/paste) that it would be very hard to fully shard due to how they work. dave On Mon, Dec 5, 2011 at 11:08 AM, Ojan Vafai o...@chromium.org wrote: I looked at one example that didn't exit early: http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/35153/steps/layout-test/logs/stdio In that case, the http tests were the long tail and took 6 minutes longer than all the other tests. We don't split the http tests up because every time we've tried it's caused too much flakiness. It's unclear if the flakiness points to a bug in the test harness (e.g. in how we setup apache) or to bugs in the tests themselves or both. If someone has time to look into this, this is probably the biggest benefit to be found in NRWT runtime when running tests in parallel. FYI, NRWT outputs a log of the runtime after each run: 2011-12-03 03:09:30,018 58036 printing.py:462 INFO worker/9: 4696 tests, 1746.63 secs 2011-12-03 03:09:30,018 58036 printing.py:462 INFO worker/8: 1177 tests, 1693.47 secs 2011-12-03 03:09:30,018 58036 printing.py:462 INFO worker/3: 1408 tests, 2033.51 secs 2011-12-03 03:09:30,018 58036 printing.py:462 INFO worker/2: 941 tests, 2119.65 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/1: 1121 tests, 2041.97 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/0: 1453 tests, 2515.75 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/7: 1189 tests, 1731.12 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/6: 3556 tests, 2114.37 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/5: 948 tests, 2097.13 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/4: 1411 tests, 1716.66 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/15: 795 tests, 2027.16 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/14: 1123 tests, 1732.72 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/13: 425 tests, 2021.25 secs 2011-12-03 03:09:30,019 58036 printing.py:462 INFO worker/12: 1175 tests, 1710.09 secs 2011-12-03 03:09:30,020 58036 printing.py:462 INFO worker/11: 3462 tests, 2096.30 secs 2011-12-03 03:09:30,020 58036 printing.py:462 INFO worker/10: 1449 tests, 1722.68 secs 2011-12-03 03:09:30,020 58036 printing.py:462 INFO31120.45 cumulative, 1945.03 optimal That shows you that, if we fully sharded all the tests, they would in theory take 1945 seconds to run, but worker/0 (the worker that runs the http tests) took 2515 seconds to run. Ojan On Mon, Dec 5, 2011 at 6:55 AM, Adam Roben aro...@apple.com wrote: On Dec 2, 2011, at 6:55 PM, Eric Seidel wrote: The SnowLeopard bot went from a 1 hr 4 min (!?!) cycle time, to 38 min (still !?!). I suspect our Mac test bots could use a dose of RAM. Many of them only have 3GB, since when you're running tests one by one you don't really need much more. -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 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using C++ constant local variables in WebKit
On Tue, Nov 29, 2011 at 6:19 PM, Darin Adler da...@apple.com wrote: On Nov 28, 2011, at 1:38 PM, David Kilzer wrote: In a discussion on Bug 71921https://bugs.webkit.org/show_bug.cgi?id=71921, Antti, Darin Adler and I started a discussion about using C++ constant pointers in WebKit. Does the WebKit community have a consensus opinion on the matter? I thought we were discussing local variables in general, not pointer-typed ones specifically. * Pros - Documents use of variable. I would say “documents the fact that the variable’s value is not changed”. I think it’s overstating things to say it “documents use”. - Prevents misuse of variable in a later patch (by a different author) through enforcement of const-ness. Prevents one specific type of misuse: Setting the variable to another value. And that may not be misuse despite the fact that the original author didn’t plan on changing it. - May help compiler optimize code. (We weren't sure whether modern compilers do this on their own or not.) Doesn’t. * Cons - Darin Adler doesn't ever recall fixing a bug in WebKit where a constant pointer would have helped. While true, not really a “con”; just weakens the “pro” argument above that this prevents misuse. - Slightly more verbose syntax for constant pointers to a constant string (const char * const pointer;) or even a constant pointer to a mutable string (char * const pointer;). Not sure this is a con. Just stating what the C++ syntax. This is the con I am aware of: - Less brief than omitting const. I’m not strongly opposed to using const more, but I am mildly opposed to it. I can see your point of view. All of this seems to apply equally to const_iterator as well. Are you mildly opposed to it as well? Or is something different about it? Minor plug (which is why I happened this think about this): An additional current downside for const_iterator is that hashtable const_iterator has an unfortunate issue where it can't be compared ==, != to an iterator which isn't nice. https://bugs.webkit.org/show_bug.cgi?id=73370 dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style bot complains of missing binary data in diff when deleting .png test results
Perhaps you could give a bug that has an example of what you are talking about. For me it is hard to guess at what the complaint by the style bot is. dave On Wed, Nov 30, 2011 at 5:20 PM, Alan Stearns stea...@adobe.com wrote: If I delete a .png test result and I make a git diff without using the --binary flag, the style EWS bot complains. I can see why it would complain if I were rebasing the file - you need the binary data to see what's changed. It makes less sense to me to add the binary data to the diff if the file is just being deleted. Should VCSUtils.pm detect a ... and /dev/null differ line and let it through? Are there dependencies on the binary data in svn-apply or other tools? I'm planning on replacing some pixel-based verification with reftests in the near future, and so I'll be deleting quite a few .png files. I don't mind slinging around all that binary data, but if it's not really needed I'd rather leave it out. Thanks, Alan ___ 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] EWS bots don't run when missing binary data in diff [was Re: Style bot complains of missing binary data in diff when deleting .png test results]
On Wed, Nov 30, 2011 at 5:42 PM, Alan Stearns stea...@adobe.com wrote: David, This is a bug where I accidentally turned on a pixel result, then needed to remove the .pngs when I fixed the problem: https://bugs.webkit.org/show_bug.cgi?id=73343 The patch had two lines like this: Binary files a/LayoutTests/platform/efl/fast/regions/no-split-line-box-expected.png and /dev/null differ Which resulted in this output from style-queue: Failed to run [u'/mnt/git/webkit-style-queue/Tools/Scripts/svn-apply', u'--force'] exit_code: 9 Error: the Git diff contains a binary file without the binary data in line: Binary files a/LayoutTests/platform/efl/fast/regions/no-split-line-box-expected.png and /dev/null differ. Be sure to use the --binary flag when invoking git diff with diffs containing binary files. at /mnt/git/webkit-style-queue/Tools/Scripts/VCSUtils.pm line 667, ARGV line 45. Ah, your issue is with how the patch is applied and that it fails and it causes the style bot to fail. This seems like a general EWS issue. It would be good to run the builds run for your patch as well even though you didn't have the binary in there. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Putting layout tests on a separate repository (was Supporting w3c ref tests and changing our convention)
On Mon, Nov 7, 2011 at 8:00 PM, Ryosuke Niwa rn...@webkit.org wrote: On Nov 4, 2011 4:59 PM, Dirk Pranke dpra...@chromium.org wrote: Actually, we should be encouraging people to use reftests now, since every port but two supports them, and we should be moving the last two over ASAP. I'd argue that we should not encourage people from writing reftests until we migrate those two ports to NRWT... It sounds like those last two port don't run pixel tests, so there wouldn't be a reduction in coverage by using reftests even before they are migrated, right? dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] It's time to remove the Haiku port
On Fri, Nov 4, 2011 at 3:04 PM, Ryan Leavengood leaveng...@gmail.comwrote: I personally would like to see WebCore evolve into being as self-contained as possible, with clear APIs for a platform to attach to. I suspect the pain hasn't been big enough so far for any person/organization to decide to address this. You do sound enthusiastic about it though, and I suspect there would be people willing to review these changes as long as they didn't slow down WebKit or make it harder to maintain. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] It's time to remove the Haiku port
On Fri, Nov 4, 2011 at 3:24 PM, Ryan Leavengood leaveng...@gmail.comwrote: There may be a time when I do this work, but I have bigger fish to fry at the moment... Exactly :) ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] watchlist: A tool to alert you about patches of interest.
The watchlist is a simple way to watch for new patches that interest you. The
watchlist is automatically applied to patches by a bot (currently the style
bot).
I'm happy to answer questions about it here or in irc (and/or review any
patches you make to the config file, but of course I don't mind others
reviewing those patches or answering questions either).
Here the details on how to use it from
https://wiki.webkit.org/wiki/WatchList
How to use the watch list
https://wiki.webkit.org/wiki/WatchList#Howtousethewatchlist
You’ll need to create a definition which matches patches that you are
interested in or find one that already exists. You’ll need to add a rule to
cc yourself on the bug (or add a message to the bug).
Details
https://wiki.webkit.org/wiki/WatchList#Details
The watchlist file is here:
http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/watchlisthttp://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/watchlist
Here’s an example version:
{
DEFINITIONS: {
ThreadingFiles: {
filename: rSource/JavaScriptCore/wtf/ThreadSpecific\.
r|Source/JavaScriptCore/wtf/ThreadSafeRefCounted\.
},
ThreadingUsage: {
more: r(CrossThreadCopier|CrossThreadRefCounted)(?!\.(h|cpp)),
},
},
CC_RULES: {
ThreadingFiles|ThreadingUsage: [ levin+thread...@chromium.org, ],
},
MESSAGE_RULES: {
ThreadingUsage: [ Are you sure you want to using threading?!?, ],
},
}
Definitions section
https://wiki.webkit.org/wiki/WatchList#Definitionssection
The definitions section is where you define what you want to look for. If it
is a filename pattern, use “filename”. Filename matches are a prefix match.
If is a code pattern use “more” or “less”, if you want to know if more or
less of instances in that pattern occur. (more use the regex to find a match
modified lines in a patch. Then it searches the to see if more instances of
that exact text occur on a per file basis.)
A definition is said to match if all of its clauses are true for any file in
a patch. If you could look for more instances of a pattern occurring only
within a group of a files by using both “filename” and “more” together.
CC rules
https://wiki.webkit.org/wiki/WatchList#CCrules
The cc rules section is where you list who should be added when a definition
matches. You can or together definitions but I only recommend doing this
when the definitions are highly related.
Message rules
https://wiki.webkit.org/wiki/WatchList#Messagerules
The message rules is where you list any messages that should be added to a
bug when a definition matches.
Trying out your change
https://wiki.webkit.org/wiki/WatchList#Tryingoutyourchange
Do a change locally that should trigger your rule and run: webkit-patch
apply-watchlist-local
It should tell you who would be cc’ed and any messages that would be added
to the bug.
Check your change for mistakes
https://wiki.webkit.org/wiki/WatchList#Checkyourchangeformistakes
Mistakes will slow things down or mess up your change. Run
check-webkit-style on your patch to catch many of these errors.
Appendix: Details about the regex used in the example:
https://wiki.webkit.org/wiki/WatchList#Appendix:Detailsabouttheregexusedintheexample:
One twist in the “more” match is the ?!\.(h|cpp)). This prevents matching
mentions of CrossThreadRefCounted.h due to includes, build files, etc. which
I don’t care about.
The r is a python thing which means that the \ in the string don’t escape
characters (r”\n” is r“\” +”n”) which is handy when you need the \ to escape
regex characters.
Python’s regex format is documented here:
http://docs.python.org/library/re.htmlhttp://docs.python.org/library/re.html
Best wishes,
dave
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] reminder: don't do blanket style fixups
Trailing whitespace guidelines aren't part of the WebKit style guide on purpose. Thus, it wasn't a style clean-up, so I guess that would make it a change to fit someone's preference. dave On Wed, Oct 19, 2011 at 6:38 PM, Ojan Vafai o...@chromium.org wrote: I saw a patch get committed recently that just fixes trailing whitespace across a large swath of the codebase. In general, we prefer that pure style cleanups are only done as a precursor to actually modifying the code. Otherwise, while they serve to make the style consistent, they also make it considerably harder to dig through the commit history for a given chunk of code. ___ 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] Change to style guideline: should use type instead of type* for out arguments
It feels like the material about RefPtr belongs here: http://www.webkit.org/coding/RefPtr.html instead of the style guide since that is where everything else about how to properly use RefPtr/PassRefPtr is. On Mon, Oct 10, 2011 at 10:42 AM, Ryosuke Niwa rn...@webkit.org wrote: FYI, a patch has been posted to https://bugs.webkit.org/show_bug.cgi?id=69766 to make this change along with get prefix convention for getters that return values via out arguments. - Ryosuke On Tue, Oct 4, 2011 at 2:06 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, It came to my attention that some people are using raw pointers to pass out-arguments (e.g. bug 69366). In my understanding, we use pass by reference for out arguments when they have to be modified in callees. If there's no objection, I'm going to file a bug upload a patch to state this explicitly in the style guideline. Best, Ryosuke Niwa Software Engineer Google Inc. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] New feature flag proposal: Joystick API
On Thu, Oct 6, 2011 at 11:07 AM, Alexey Proskuryakov a...@webkit.org wrote: I share Simon's concern that that providing low level access to every possible controller creates fragmentation, with purportedly HTML content that only works on a few devices. There is no clear cut border here - it's been mentioned that even touch events can be seen as rare - and then I advocate that adding more mouse specific events is a bad idea for the same reason. Isn't a balance of usefulness vs fragmentation vs trusting developers? When touch events are exposed, I'd expect that developers who care about having a board appeal will have alternatives for users without a touch interface but having a web page be able to respond to touch events seems useful for web pages to do to shine in that context. (In fact developers are inclined to provide this other interface to have the broad appeal.) Doesn't the same thing apply to these other cases? dave As we add joystick/gamepad support, should steering wheels be next on the agenda? 3d mice? - WBR, Alexey Proskuryakov 06.10.2011, в 10:01, Darin Fisher написал(а): This proposal has matured somewhat, so an update is in order. FYI: http://dvcs.w3.org/hg/webevents/raw-file/default/gamepad.html http://www.w3.org/2010/webevents/charter/2011/Overview.html We are working behind the ENABLE(GAMEPAD) flag at the moment. Mozilla is also building a prototype of this API. We are doing development on the trunk (disabled by default) so that we can more easily solicit game developers for feedback using our existing Chrome distribution channels. This feature should have a very light touch on existing cross platform code. We encourage folks to share feedback on the API design to webevents-pub...@w3.org. Regards, -Darin On Wed, Aug 24, 2011 at 9:19 AM, Simon Fraser simon.fra...@apple.comwrote: I think it's too early to implement this. We should wait until it's a W3C draft at least. window.addEventListener(MozJoyConnected..), really? Simon On Aug 24, 2011, at 8:36 AM, Scott Graham wrote: Hi, I wanted to let everyone know that I propose to add a new feature flag, JOYSTICK. http://webkit.org/b/66859 This flag will enable an API and events for accessing joysticks and related devices. There's a prototype effort happening in Mozilla also (https://wiki.mozilla.org/JoystickAPI), and the design is intended to be similar. As it will not necessarily make sense for all ports, nor be implemented immediately in all ports, a feature flag seems appropriate. Please let me know if you have any concerns or comments. Thanks ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Please use platform prefixes in bug titles
On Wed, Oct 5, 2011 at 6:07 PM, Alexey Proskuryakov a...@webkit.org wrote: 05.10.2011, в 17:47, James Robinson написал(а): That would certainly be nice, but it wouldn't be as useful as having the platform in the bug description. The Platform field is not very obvious in the bugzilla UI and doesn't show up at all in bugzilla-generated emails, ChangeLogs, or commit messages. Many of our bugs are filed via tools like webkit-patch or sheriffbot which could be taught about the Platform entry, but doing so would decrease the usability of the tools and I suspect many people would get it wrong. Additionally, platform field isn't the same as a prefix. A prefix promises that there are no changes in cross platform code, suggesting that core developers can safely skip the bug in review queue. Cross-platform changes for platform specific bugs are sometimes valid, but should get more scrutiny. Dave Levin filed a bug about this today, https://bugs.webkit.org/show_bug.cgi?id=69462 check-webkit-style: Should catch when folks mistakenly put [chromium], etc. in a bug title. btw, I'm not planning to work on this for now but would be happy to give advice if someone took it up. dave - WBR, Alexey Proskuryakov ___ 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] Compile-time assertions for object sizes
On Thu, Sep 29, 2011 at 12:11 PM, Simon Fraser simon.fra...@apple.comwrote:
On Sep 29, 2011, at 11:40 AM, Andreas Kling wrote:
Dear WebKittens,
I'd like to add some compile-time assertions for the sizes of various
objects. The motivation comes a patch fixing bloat in InlineBox[1].
There are two major problems with this:
1. The sizes will differ on 32- and 64-bit platforms.
2. The sizes will differ based on compiler flags.
One idea is to add a file that would only be built on (for example)
64-bit Mac and then at least that bot would break if an object changes size.
That's obviously not ideal though.
Any suggestions? :)
You could group the bits together into a struct:
struct {
m_foo: 1;
m_bar: 1;
...
} m_bits;
COMPILE_ASSERT(sizeof(m_bits) = sizeof(uint32_t), Too_many_bits);
This wouldn't' be sensitive to architecture.
Simon
Perhaps you'll find some inspiration in
http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/SizeLimits.cppwhere
this is done for a few structs in wtf.
I tried to use sizeof(int*) where I needed to deal with 32 vs 64 pointer
issues and I put in an ifdef for a debug related size thing.
dave
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] webkit-dev [check-webkit-style - shows warnings on the untouched code in repo]
If you run check-webkit-style on a file, it will show all style issues for the whole file. However, you can run it on a diff (which is what webkit-patch upload will do). Then it will only flag lines that you changed. It still may flag issues that you didn't cause but you changed those lines, so it is recommended to fix the style issues on those lines (and on no other lines) unless it would cause a lot of other lines to change (like unindenting a whole section of code). Hope that helps! dave On Tue, Sep 27, 2011 at 10:59 PM, Kishore Ganesh kbolise...@innominds.comwrote: Hi All, We Have a patch to upload for the bug id : 39986. The changes are in RenderTable.cpp and RenderTableSection.cpp. When we run check-webkit-style on each of these files, It shows few errors that are not related to our patch. Can some suggest, **· **If anyone has already seen such behaviour **· **If we can ignore these Or we need to cleanup all the warnings though its not related to the fix for 39986? Here is the output from the script… ** ** $ Tools/Scripts/check-webkit-style Source/WebCore/rendering/RenderTable.cpp Source/WebCore/rendering/RenderTable.cpp:36: Alphabetical sorting problem. [bu ild/include_order] [4] Source/WebCore/rendering/RenderTable.cpp:89: Should have only a single space af ter a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/rendering/RenderTable.cpp:141: A case label should not be indent ed, but line up with its switch statement. [whitespace/indent] [4] Source/WebCore/rendering/RenderTable.cpp:145: One line control clauses should n ot use braces. [whitespace/braces] [4] Source/WebCore/rendering/RenderTable.cpp:1092: An else statement can be removed when the prior if concludes with a return, break, continue or goto statement. [readability/control_flow] [4] ** ** $ Tools/Scripts/check-webkit-style Source/WebCore/rendering/RenderTableSection. cpp Source/WebCore/rendering/RenderTableSection.cpp:27: You should add a blank line after implementation file's own header. [build/include_order] [4] Source/WebCore/rendering/RenderTableSection.cpp:216: Boolean expressions that s pan multiple lines should have their operators on the left side of the line inst ead of the right side. [whitespace/operators] [4] ** ** Regards, Kishore ___ 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] Vendor Prefixing, was Re: New feature announcement - Implement HTML5 Microdata in WebKit
TL;DR (from Charles himself): My concern, on this WebKit development mailing list, is that introducing another method -without- vendor prefixing may create some tension that WebKit developers would like to avoid. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] starting implementation of postMessage tranferables
*Summary*: Implementing postMessage with transferable support as webkitPostMessage. *Details*: *Spec*: http://www.whatwg.org/specs/web-apps/current-work/multipage/comms.html#posting-messages This describes window.postMessage and the same is true for Message Ports and Worker Context. It doesn't mention Array Buffers but that will be mentioned soon and is in the spec that talks about Array Buffers. Tracking bug: https://bugs.webkit.org/show_bug.cgi?id=64629 *Plan*: 1. Add webkitPostMessage to all of these places to isolate us from possible spec changes. It will have the same functionality as postMessage. 2. Add the ability to transfer Message Ports. 3. Add the ability to transfer Array Buffers. We don't plan to put ifdef's around this as we'll do an implementation for all ports and js engines and each patch is complete by itself. (Even when using webkitPostMessage, one has to be careful to verify that the items being transfered may be transfered. After step 1, the answer is no to everything.) I didn't send this earlier because we were doing this in postMessage and it was in the spec but it would have been good to send out the email even then. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is an active port? [WAS: Do you maintain OS(WINCE)?]
On Thu, Sep 15, 2011 at 12:26 PM, Dirk Pranke dpra...@chromium.org wrote: On Thu, Sep 15, 2011 at 12:17 PM, Geoffrey Garen gga...@apple.com wrote: On Sep 14, 2011, at 1:02 PM, Dirk Pranke wrote: Maybe we need a webkit-port-maintainers@ list that one could easily cc rather than trying to add people by hand? Sounds helpful. Not sure exactly how it would work, though. (How would you add yourself to the list?) Subscribe through the listserv, just like webkit-dev? fwiw, I could totally see that working. Here's how I would use it: In general I would ignore the email except when I was on duty for keeping things green. Then, I would watch it carefully and get the right people to help out with the Chromium side of things as needed. dave -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Committing EFL baselines
On Mon, Sep 12, 2011 at 1:48 PM, Peter Kasting pkast...@chromium.orgwrote: On Mon, Sep 12, 2011 at 1:36 PM, Ryosuke Niwa rn...@webkit.org wrote: This is pretty much unreviewable, so I pretend to commit this directly, in batches (one commit per toplevel directory in LayoutTests/platform/efl) in the next weeks. Any objections or suggestions? It'll be nice if we could spend some time analyzing the differences between EFL and other ports to minimize the size of patch first. In particular, if we have pixel tests that don't need to be pixel tests at all, or font rendering differences due to explanatory text that could be moved to an HTML comment inside the test itself, we can obviate the need for port-specific baselines in many of those cases (and eliminate more baselines from ports already in the tree, and reduce the burden on other ports that want to run pixel tests, and reduce the maintenance cost of changing the tests). Are y'all suggesting that efl port should do these items (converting pixel tests to non-pixel tests) before committing their baselines? dave PS fwiw, at one point, I did some work to figure out which tests were changing most often to see where people would get the most return on their investment and to my memory many tests with a high churn rate were svg (which seem like they would be harder to convert to a non-pixel test format). PK ___ 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] RefPtr/PassRefPtr Question
fwiw, check-webkit-style has been fixed: http://trac.webkit.org/changeset/94803 On Wed, Sep 7, 2011 at 7:56 AM, Darin Adler da...@apple.com wrote: On Sep 6, 2011, at 6:24 PM, Maciej Stachowiak wrote: On Aug 31, 2011, at 3:31 PM, David Levin wrote: Ignore me. I'm missing the . I suppose if you want a RefPtr, then the style checker is wrong and the parameter should be allowed to be a RefPtr. Feel free to file a bug and I'll get to it (-- it may take me a week or two at the moment). It should definitely be allowed - it's a good way to represent I need a T and I won't take ownership, but I want to guarantee that my caller is holding on to this. Yes. Also a good way to represent a RefPtr out parameter for a function with more than one return value. Also occasionally useful to make a function that conditionally takes ownership of something passed in. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] ping-pong with fast/files/create-blob-url-crash-expected.txt
On Wed, Aug 31, 2011 at 5:57 AM, Philippe Normand ph...@igalia.com wrote: Sorry for the last one. Looks like the patch in https://bugs.webkit.org/show_bug.cgi?id=66045 is intended to fix this issue for both JSC and V8 code generators, if I understood correctly. Short story: http://trac.webkit.org/**changeset/94023http://trac.webkit.org/changeset/94023 is correct and we should fix the result that in there now. More: In general, when JSC and V8 disagree, we've gone with JSC in the platform independent file. http://trac.webkit.org/changeset/93713 and http://trac.webkit.org/** changeset/94168 http://trac.webkit.org/changeset/94168 didn't do this so they aren't following what is standard practice. The original issue crept in with r93713 which changed this output for v8 but didn't figure out why -- No idea as to why the code the code generator acts differently now. dave Philippe On Wed, 2011-08-31 at 13:40 +0200, Osztrogonac Csaba wrote: Hi, It seems you guys are playing ping-pong with this platform independent expected file. Could you decide which one is the correct? br, Ossy http://trac.webkit.org/changeset/93713 http://trac.webkit.org/changeset/93713/trunk/LayoutTests/fast/files/create-blob-url-crash-expected.txt -PASS: Not enough arguments +PASS: Type error http://trac.webkit.org/changeset/94023 http://trac.webkit.org/changeset/94023/trunk/LayoutTests/fast/files/create-blob-url-crash-expected.txt -PASS: Type error +PASS: Not enough arguments http://trac.webkit.org/changeset/94168 http://trac.webkit.org/changeset/94168/trunk/LayoutTests/fast/files/create-blob-url-crash-expected.txt -PASS: Not enough arguments +PASS: Type error ___ 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] lots of red in the tree.
On Tue, Aug 30, 2011 at 12:54 PM, David Levin le...@chromium.org wrote: On Tue, Aug 30, 2011 at 10:55 AM, Jarred Nicholls jar...@sencha.comwrote: On Tue, Aug 30, 2011 at 1:52 PM, David Levin le...@chromium.org wrote: It means that I'm much more likely to cause regressions because I miss new test failures caused by my changes among the 12 to 62 failures already occurring on the OS X bots.1 Yeah it was wigging me out this morning. I think there are a few solutions to the current situation: 1. Live with it. All of the red and green reminds us of the festive winter holidays. presents! Downside: More regressions get in as nobody notices them much even if they try to be careful. Upside: Requires no more extra work, so it is quick to do! 2. Get folks working on every red test. Was thinkin' about diving head first into this. Downside: May not be able to get folks to drop what they are doing and work on them. Upside: More stable code, easier to work with, etc. 3. Add them to skipped and file bugs. Good first step. Some of the tests are flaky though - I'll get different results on subsequent runs. I'm ok with letting flaky ones run for now as they are re-run by the harness and won't hide new failures. I would like to take care of those that are always failing. I'm in favor of this one -- Adding them to skipped and filing bugs. So far it sounds like there are no objections, so I plan to start doing this soon. I'll add new baselines (which may be incorrect) and file bugs to fix the results (cc'ing folks whom I'll guess will be interested in that issue). If it is crashing, I'll add to skipped and file bugs. dave dave Downside: Not having the tree red may lower the urgency and having them in skipped list may mean that folks just ignore them. Upside: We'll catch regressions more quickly and perhaps stop the current decent which it seems like we've been proceeding on. 4. Your idea! What do other folks think? Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- *Sencha* Jarred Nicholls, Senior Software Architect @jarrednicholls http://twitter.com/jarrednicholls ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] RefPtr/PassRefPtr Question
Any of these should work: RefPtrT myLocal; bool success = myFunc(myLocal); Uses templatetypename U PassRefPtr(const RefPtrU); Or RefPtrT myLocal; bool success = myFunc(myLocal.release()); Or RefPtrT myLocal; bool success = myFunc(myLocal.get()); Uses PassRefPtr(T* ptr) The second form is prefered if you won't be using myLocal again in the function. I would use the first form if you are using myLocal again. dave On Wed, Aug 31, 2011 at 3:16 PM, David Hyatt hy...@apple.com wrote: I am getting complaints from check-webkit-style in a bug regarding PassRefPtr/RefPtr usage, and I can't figure out what I should be doing. It yells at me no matter what I try. The scenario I have is that a function is wanting to transfer ownership but it's not doing it via a return value. Instead it is filling in a reference parameter. The current code looks like this: Caller: RefPtrT myLocal; bool success = myFunc(myLocal); With the function being: bool myFunc(RefPtrT result); With this setup though, I get yelled at by the style checker and it tells me that the parameter should be a PassRefPtr. However I don't get how I can do that, since then I have: PassRefPtrT myLocal; and I get yelled at for making a PassRefPtr local variable. What's the right way to write this code such that it will pass? Is this just a flaw in the style checker? It sure seems like a RefPtrT reference parameter should be allowed... dave (hy...@apple.com) ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] RefPtr/PassRefPtr Question
Ignore me. I'm missing the . I suppose if you want a RefPtr, then the style checker is wrong and the parameter should be allowed to be a RefPtr. Feel free to file a bug and I'll get to it (-- it may take me a week or two at the moment). dave On Wed, Aug 31, 2011 at 3:28 PM, David Levin le...@google.com wrote: Any of these should work: RefPtrT myLocal; bool success = myFunc(myLocal); Uses templatetypename U PassRefPtr(const RefPtrU); Or RefPtrT myLocal; bool success = myFunc(myLocal.release()); Or RefPtrT myLocal; bool success = myFunc(myLocal.get()); Uses PassRefPtr(T* ptr) The second form is prefered if you won't be using myLocal again in the function. I would use the first form if you are using myLocal again. dave On Wed, Aug 31, 2011 at 3:16 PM, David Hyatt hy...@apple.com wrote: I am getting complaints from check-webkit-style in a bug regarding PassRefPtr/RefPtr usage, and I can't figure out what I should be doing. It yells at me no matter what I try. The scenario I have is that a function is wanting to transfer ownership but it's not doing it via a return value. Instead it is filling in a reference parameter. The current code looks like this: Caller: RefPtrT myLocal; bool success = myFunc(myLocal); With the function being: bool myFunc(RefPtrT result); With this setup though, I get yelled at by the style checker and it tells me that the parameter should be a PassRefPtr. However I don't get how I can do that, since then I have: PassRefPtrT myLocal; and I get yelled at for making a PassRefPtr local variable. What's the right way to write this code such that it will pass? Is this just a flaw in the style checker? It sure seems like a RefPtrT reference parameter should be allowed... dave (hy...@apple.com) ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] lots of red in the tree.
On Tue, Aug 30, 2011 at 10:55 AM, Jarred Nicholls jar...@sencha.com wrote: On Tue, Aug 30, 2011 at 1:52 PM, David Levin le...@chromium.org wrote: It means that I'm much more likely to cause regressions because I miss new test failures caused by my changes among the 12 to 62 failures already occurring on the OS X bots.1 Yeah it was wigging me out this morning. I think there are a few solutions to the current situation: 1. Live with it. All of the red and green reminds us of the festive winter holidays. presents! Downside: More regressions get in as nobody notices them much even if they try to be careful. Upside: Requires no more extra work, so it is quick to do! 2. Get folks working on every red test. Was thinkin' about diving head first into this. Downside: May not be able to get folks to drop what they are doing and work on them. Upside: More stable code, easier to work with, etc. 3. Add them to skipped and file bugs. Good first step. Some of the tests are flaky though - I'll get different results on subsequent runs. I'm ok with letting flaky ones run for now as they are re-run by the harness and won't hide new failures. I would like to take care of those that are always failing. I'm in favor of this one -- Adding them to skipped and filing bugs. dave Downside: Not having the tree red may lower the urgency and having them in skipped list may mean that folks just ignore them. Upside: We'll catch regressions more quickly and perhaps stop the current decent which it seems like we've been proceeding on. 4. Your idea! What do other folks think? Dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- *Sencha* Jarred Nicholls, Senior Software Architect @jarrednicholls http://twitter.com/jarrednicholls ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Git mirror out of sync?
From an irc conversation, it sounds like this is being worked on but git-svn just hangs, hard to tell why its broken dave On Wed, Aug 17, 2011 at 7:56 AM, Nico Weber tha...@chromium.org wrote: Hi, git fetch origin git log origin/master suggests that the git mirror stopped syncing at r93166. Can someone kick it? Thanks, Nico ___ 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] LayoutTests results fallback graph
On Mon, Jul 11, 2011 at 10:46 AM, Adam Barth aba...@webkit.org wrote: There are two main benefits: 1) A tree is much easier to understand than a rats nest of interwoven fallback paths. I find it impossible to know what the fallback path is. Even with Adam's graph, it is hard to figure it out as Mark demonstrated and I can understand why. Largely this seems to be because it is a complex directed acyclic graph as opposed to a tree. I think this would be very beneficial. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] hook to re-author commit queue patches seems broken/off
For example, this patch http://trac.webkit.org/changeset/89287 says it is by commit-qu...@webkit.org but it should say mrobin...@webkit.org. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Lets use PassRefPtr for arguments less; lets use RefPtr for locals and data members more
On Mon, Jun 20, 2011 at 5:25 PM, Maciej Stachowiak m...@apple.com wrote: On Jun 20, 2011, at 9:19 AM, Alexey Proskuryakov wrote: 20.06.2011, в 03:22, Maciej Stachowiak написал(а): For a shared ownership model there are multiple possible definitions of whether a function takes ownership to an object passed as an argument. Here are some of my attempts to describe the bright line: a) Hands off ownership to what could possibly be the sole owner in most code paths. b) Keeps a reference to the object after the function completes in most code paths. c) Takes a reference to the object at least once in most code paths. d) Hands off ownership to what could possibly be the sole owner in some code paths. e) Keeps a reference to the object after it completes in some code paths. f) Takes a reference to the object at least once in some code paths. Is the bright line rule you have in mind (b) or perhaps (e)? Or something not listed here at all? Yes, (b) or (e). I haven't thought about whether most code paths or some code paths makes more sense, but I'm not sure there are a lot of cases in our code where it makes a difference. I don't think it makes sense to talk about a sole owner of a refcounted object. If there is a sole owner at any given time, it is at best temporary. PassRefPtr is about clearly and efficiently transferring a reference, it doesn't need it to necessarily be the sole then-existing reference. I think that to make this complete, the rules need to be transitive. A function that passes its argument to another function taking a PassRefPtr should itself take a PassRefPtr. That's the case in https://bugs.webkit.org/show_bug.cgi?id=52981, for instance. It doesn't seem that analyzing code for most/some code paths takes ownership would be any easier that analyzing it for any caller gives away ownership. In general it only requires inspecting the source of the function, and possibly the signatures of function it calls. The analysis for any caller gives away ownership requires searching over all of WebCore to find possible call sites. And it can easily change If we think the transitivity problem is a hazard, we could make PassRefPtr refuse to implicitly convert from raw pointers. Then to pass a raw pointer to a function that uses PassRefPtr you'd have to make a RefPtr first (or adopt, if the ref is unowned). That would make takes ownership analysis purely local to the function source. I thought the problem was that a Pass*Ptr could silently 0 itself out and people use it directly. So the real problem functions are anything the fact that a Pass*Ptr can be used like a pointer and that another Pass*Ptr can slurp out its contents so easily (and you can't spot this in the code well). Maybe each of those operations should be more explicit? (Then we could probably even catch these issues automatically. If *.passPtr() is called and then *.get() is called later in the same function, mark it as an error. Not perfect but good enough.) Yet it's the latter where PassRefPtr is beneficial. Why base the rule on something that's disconnected from actual benefit? Because it's simpler to read the source of your own function than to visit all call sites, and it's more obvious that when you change what the function does you may need to change the signature. This seems much easier, less error prone, and more stable. dave Regards, Macij ___ 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] Lets use PassRefPtr for arguments less; lets use RefPtr for locals and data members more
On Mon, Jun 20, 2011 at 6:11 PM, Alexey Proskuryakov a...@webkit.org wrote: 20.06.2011, в 17:25, Maciej Stachowiak написал(а): Yet it's the latter where PassRefPtr is beneficial. Why base the rule on something that's disconnected from actual benefit? Because it's simpler to read the source of your own function than to visit all call sites, and it's more obvious that when you change what the function does you may need to change the signature. I do not see how you are describing a practical coding situation here. I do not start with a dozen call sites all over the code base, and then write a function they all call. On the other hand, when adding a new call site that wants to pass ownership away, and the called function doesn't take a PassRefPtr, it's immediately obvious that it's not going to work. Even when following a cargo cult rule is easier (not uncommon!), the problem of it being disconnected from the actual benefit still remains. Here's a few benefits: 1. It makes the code more self-documenting. It clearly indicates that this function intends to take a reference to the item. 2. It is consistent with the rules for PassOwnPtr. It is nice to have one set of things in mind that are consistent. 3. Just like one shouldn't document a function based on who calls it because that may change, it makes sense to base the argument types on the how the function uses them not on the callers. - WBR, Alexey Proskuryakov ___ 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] Lets use PassRefPtr for arguments less; lets use RefPtr for locals and data members more
On Mon, Jun 20, 2011 at 9:39 PM, Alexey Proskuryakov a...@webkit.org wrote: 20.06.2011, в 21:29, David Levin написал(а): Here's a few benefits: 1. It makes the code more self-documenting. It clearly indicates that this function intends to take a reference to the item. 2. It is consistent with the rules for PassOwnPtr. It is nice to have one set of things in mind that are consistent. 3. Just like one shouldn't document a function based on who calls it because that may change, it makes sense to base the argument types on the how the function uses them not on the callers. 1 and 3 sound very closely related. Yet I'm not sure if they are good. True, I didn't think of it that way when I wrote it. :) I feel like they are good, but I could be wrong. (I always have felt like knowing who is owning something is one of those tricky things when reading code so I've appreciated this but it may be of limited value when taking about ref counted items.) And I like 2 but it isn't critical. PassRefPtr is useful even if the function doesn't take any kind of ownership. It's useful when callers want to get rid of ownership, and then they can do that efficiently. I don't understand this. How it is more efficient to pass ownership if a function isn't taking ownership? It seems like passing in a raw pointer would be the most efficient in this case. As Maciej mentioned, eventually we may be able to get rid of PassRefPtr, and use C++0x move semantics. But applicability of move semantics also depends on how a function is called, not on what it is going to do with its arguments. Good point. Makes sense. dave - WBR, Alexey Proskuryakov ___ 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] Why is gtest in the Source directory?
It sounds like you have a helpful mental mapping for what belongs in each directory that we haven't written anywhere. Perhaps you could write it down and send it to webkit-dev so that we can make it a common model. It feels like a related topic is why are the unit tests under Tools as opposed to being by the files that they test. Being by the files they tests would make them easier to find and change as needed. (For an example of where this is done, see http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common where there is file.py and file_unittest.py) dave On Thu, May 12, 2011 at 8:36 AM, Dan Bernstein m...@apple.com wrote: Is gtest required to build any of the WebKit ports? If not, can it be moved out of Source and into Tools? Thanks, —Dan ___ 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] Why is gtest in the Source directory?
On Thu, May 12, 2011 at 10:36 AM, Adam Barth aba...@webkit.org wrote: Here's a straw-man proposal: Tests/ -- All testing code. Do test harnesses go here too? Layout/ # Would be nice to have a better name since these test much more than Layout DumpRenderTreeTests? Integration? AutomatedHtmlTests? Performance/ Unit/ with a directory structure that mirrors Source for the tests Manual/ Source/ -- Only the source code necessary for building WebKit. JavaScriptCore/ WebCore/ WebKit/ ThirdParty/ ANGLE/ Tools/ WebKitTestRunner/ MiniBrowser/ Scripts/ In this organization, gtest probably doesn't belong in Source because it's not a dependency of WebKit. I agree but I felt like it lacked explicit definition, so I added some. It probably belongs somewhere in Tests. Maybe inside the Unit directory somewhere? I think so. I wonder why we wouldn't put all tests related code under Tests. For example WebKitTestRunner, DumpRenderTree, CSSTestSuiteHarness seem to fit in that category and be similar in some ways to gtest in that they are the test harness. Adam On Thu, May 12, 2011 at 10:24 AM, Dmitry Lomov dslo...@google.com wrote: I think more important issue to consider is should WebKit unit-tests (TestWebKitAPI) live under Tools. Unit-tests evolve with the product and an organic part of it - it doesn't feel that they constitute a tool. Three possible locations come to mind: - ./Tests - separate top-level directory, sibling to Source and LayoutTests - ./Source/Test - separate directory, but under Source to emphasize that tests are part of the source code - Scattered around by the files that tests test, as David suggests. In terms of gtest directory placement, I think it is nice to have all third-party libraries live in the same place. What do people think? Kind regards, Dmitry On Thu, May 12, 2011 at 9:38 AM, David Levin le...@chromium.org wrote: It sounds like you have a helpful mental mapping for what belongs in each directory that we haven't written anywhere. Perhaps you could write it down and send it to webkit-dev so that we can make it a common model. It feels like a related topic is why are the unit tests under Tools as opposed to being by the files that they test. Being by the files they tests would make them easier to find and change as needed. (For an example of where this is done, see http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/commonwhere there is file.py and file_unittest.py) dave On Thu, May 12, 2011 at 8:36 AM, Dan Bernstein m...@apple.com wrote: Is gtest required to build any of the WebKit ports? If not, can it be moved out of Source and into Tools? Thanks, —Dan ___ 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] Why is gtest in the Source directory?
On Thu, May 12, 2011 at 11:33 AM, David Levin le...@chromium.org wrote: On Thu, May 12, 2011 at 10:36 AM, Adam Barth aba...@webkit.org wrote: Here's a straw-man proposal: Tests/ -- All testing code. Do test harnesses go here too? Layout/ # Would be nice to have a better name since these test much more than Layout DumpRenderTreeTests? Integration? AutomatedHtmlTests? Performance/ Unit/ with a directory structure that mirrors Source for the tests Manual/ Source/ -- Only the source code necessary for building WebKit. JavaScriptCore/ WebCore/ WebKit/ ThirdParty/ ANGLE/ Tools/ WebKitTestRunner/ MiniBrowser/ Scripts/ In this organization, gtest probably doesn't belong in Source because it's not a dependency of WebKit. I agree but I felt like it lacked explicit definition, so I added some. It probably belongs somewhere in Tests. Maybe inside the Unit directory somewhere? I think so. I wonder why we wouldn't put all tests related code under Tests. For example WebKitTestRunner, DumpRenderTree, CSSTestSuiteHarness seem to fit in that category and be similar in some ways to gtest in that they are the test harness. Dmitry Lomov pointed out to me that DRT is a tool on its own. Now, I see a reasonable distinction here. In short, test harnesses are tools. oth, gtest is part of the test code itself. Adam On Thu, May 12, 2011 at 10:24 AM, Dmitry Lomov dslo...@google.com wrote: I think more important issue to consider is should WebKit unit-tests (TestWebKitAPI) live under Tools. Unit-tests evolve with the product and an organic part of it - it doesn't feel that they constitute a tool. Three possible locations come to mind: - ./Tests - separate top-level directory, sibling to Source and LayoutTests - ./Source/Test - separate directory, but under Source to emphasize that tests are part of the source code - Scattered around by the files that tests test, as David suggests. In terms of gtest directory placement, I think it is nice to have all third-party libraries live in the same place. What do people think? Kind regards, Dmitry On Thu, May 12, 2011 at 9:38 AM, David Levin le...@chromium.org wrote: It sounds like you have a helpful mental mapping for what belongs in each directory that we haven't written anywhere. Perhaps you could write it down and send it to webkit-dev so that we can make it a common model. It feels like a related topic is why are the unit tests under Tools as opposed to being by the files that they test. Being by the files they tests would make them easier to find and change as needed. (For an example of where this is done, see http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/commonwhere there is file.py and file_unittest.py) dave On Thu, May 12, 2011 at 8:36 AM, Dan Bernstein m...@apple.com wrote: Is gtest required to build any of the WebKit ports? If not, can it be moved out of Source and into Tools? Thanks, —Dan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] git repository seems out of date
(From two different machines) when I do a git fetch, I seem to only be getting up to r86081. Is something wrong with the main repository? If so, would someone fix it? Thanks, dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Renaming the blog (was Re: WebKit blog post proposal: Remote debugging with Web Inspector.)
What's what with WebKit On Mon, May 2, 2011 at 10:01 AM, Eric Seidel e...@webkit.org wrote: Along the vein of cutesy CSS names: http://trac.webkit.org/browser/trunk/Source/WebCore/css/CSSPropertyNames.in was rather inspiring: -webkit-writing-mode: blog content: blog Style Overflow src=webkit -webkit-perspective onwebkit That's all I got. I'm not actually a big fan of any of those. But I'm sure *someone* witty reads this list. :) -eric On Mon, May 2, 2011 at 8:44 AM, Simon Fraser simon.fra...@apple.com wrote: On May 2, 2011, at 1:45 AM, Adam Barth wrote: On Sun, May 1, 2011 at 2:27 AM, Maciej Stachowiak m...@apple.com wrote: On 2011-04-30, at 22:11, Pavel Feldman wrote: provocativeIn return, can I ask to rename the WebKit blog from Surfin' Safari to something more WebKit-specific?/provocative I've wondered the same thing myself on several occasions. Back in the day, we named it after Hyatt's old blog. At this point, it might be that no one remembers it or gets the reference. Does anyone have any clever references involving WebKit? The only clever thing I can think of (and it's not very clever) is to make some play on -webkit-foo vendor-prefixed CSS features. Something like: content: -webkit-blog; That'll go down well with all the web authors who hate vendor prefixes :) We could be boring and just call it The WebKit Blog. Simon ___ 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] How to enable WebGL on WebKit QT port?
It looks like there need to be a few build fixes in that code due to recent changes in the code. See http://trac.webkit.org/changeset/85343 for the types of changes to be done. Then feel free to submit a patch to fix this for others -- http://www.webkit.org/coding/contributing.html dave On Mon, May 2, 2011 at 11:32 AM, Won J Jeon wjj...@gmail.com wrote: I tried to build a WebKit QT port with WebGL support by enabling '--3d-canvas' and '--3d-rendering' (--no-accelerated-2d-canvas by default) but it has the following error on GraphicsContext3DQt.cpp: ../../../Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp: In constructor ‘WebCore::GraphicsContext3D::GraphicsContext3D(WebCore::GraphicsContext3D::Attributes, WebCore::HostWindow*, bool)’: ../../../Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:617: error: no matching function for call to ‘WTF::OwnPtrWebCore::GraphicsContext3DInternal::OwnPtr(WebCore::GraphicsContext3DInternal*)’ ../../../Source/JavaScriptCore/wtf/OwnPtr.h:57: note: candidates are: WTF::OwnPtrT::OwnPtr(const WTF::OwnPtrtypename WTF::RemovePointerT::Type) [with T = WebCore::GraphicsContext3DInternal] ../../../Source/JavaScriptCore/wtf/OwnPtr.h:48: note: WTF::OwnPtrT::OwnPtr() [with T = WebCore::GraphicsContext3DInternal] make[1]: *** [obj/release/GraphicsContext3DQt.o] Error 1 Is there any other steps that I need to follow in order to make WebGL working? Regards, Won ___ 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] How to enable WebGL on WebKit QT port?
On Mon, May 2, 2011 at 1:39 PM, Won J Jeon wjj...@gmail.com wrote: Dear David, Thanks for your response. However, my code base is r85509 and even with the code changes that you mentioned, I got the same error message. Any idea? Yes, there still need to be some changes to fix the build that you are doing. Perhaps you can do them and submit a patch with the fix -- http://www.webkit.org/coding/contributing.html http://trac.webkit.org/changeset/85343 will not fix your problem. However, you can see what was done in it to fix similar problems in other places in the code. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] WebKit unit test framework
On Wed, Apr 20, 2011 at 3:00 PM, Timothy Hatcher timo...@apple.com wrote: I think having something the WebKit community owns and controls is preferred over importing and using a third-party library. So that makes me prefer TestWebKitAPI (or something built from/on it) over gtest. And TestWebKitAPI already has a very simple test for WTF::Vector — just begging to be expanded. I agree that this sounds attractive at first. However, it feels like there is a bigger picture. There is a lot of work already done in gtest which would be nice to have (-- some of which will likely be necessary). Here's a list right off the top of my head (from what I've seen of both): - TestWebKitAPI doesn't print out values when a test fails. (gtest has this support.) - I don't think there is any documentation for TestWebKitAPI (For, gtest there is http://code.google.com/p/googletest/wiki/Documentation). - I don't there are any test for the framework in TestWebKitAPI -- to be hackable in WebKit with confidence, this is needed. (gtest has extensive testing.) - TestWebKitAPI seems to only run one test at a time. When the ability is added to run all test, it would also be good to add the ability to run a set of test (gtest already has this). - In addition, gtest has a nice output (including a nice color output when supported by the terminal as well as output options which integrate better with automation -- see generating an xml report in http://code.google.com/p/googletest/wiki/AdvancedGuide). - The SetUp/TearDown functionality is a nice way isolate this type of stuff out of the way of tests. - In addition, the design of the api has gone through lots of discussion by concerned parties to work well. (This is more attention that we'd be able to expend on this.) Someone could add these items to TestWebKitAPI eventually and some of these items may never get done due to the cost/benefit ratio of doing them for something just used in WebKit project. In short, it seems to me that the effort to do any of this would be better invested in other places where there isn't already something that works for us. fwiw, we could go with what we do with bugzilla where we start with gtest and people change the code if needed. dave On Apr 18, 2011, at 11:36 AM, David Levin wrote: *Issue: *There has been a long standing bug to add unit tests to WebKit ( https://bugs.webkit.org/show_bug.cgi?id=21010). It was also mentionedhttp://lists.macosforge.org/pipermail/webkit-dev/2009-January/006359.htmlon webkit-dev that it would be helpful in various cases. *Landscape:* Surveying WebKit, it is looks like there are at least three testing frameworks being used: TestWebKitAPI/WebKitAPITest (in Tools), QTest, gtest (in Source/WebKit/chromium/). However, only one TestWebKitAPI has been used so far (as far as I can tell) for testing core WebKit items like WTF (though I was unaware of TestWebKitAPI until Friday). It seems like a good way to think about the issue of which to use in general in WebKit would be to decide on what would be desired in our framework and then see how each matches up. Here's my take on this. (It may be biased toward what I am familiar with but I welcome others to add their own criteria.) Criteria Musts: - Compatible license with WebKit - Builds/Can be built on the many platforms and build systems supported by WebKit (ideally without extra installs). Useful: - Easy to write tests - Hackable to suit our needs - Well tested features (to support hackability/stability) - Supports filtering of tests so you can run just the test you care about (and easily listing the tests). - Supports writing out values when there is test failure. (For example, if the is verifying that A == B but that is not true, then the values of A and B should be printed.) - Well documented thanks, dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev — Timothy Hatcher ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] WebKit unit test framework
On Wed, Apr 20, 2011 at 4:01 PM, Sam Weinig wei...@apple.com wrote: Is a death test as scary as it sounds? :) Useful if you want to verify that the program crashes. fwiw, chromium uses this to verify that asserts fire in debug in particular scenarios. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] WebKit unit test framework
On Wed, Apr 20, 2011 at 4:59 PM, Darin Fisher da...@chromium.org wrote: I believe both maruel and jcivelli have had experience contributing changes to gtest. While I wouldn't characterize its code as simple, I haven't had trouble understanding it. It is a fairly mature project, having been used internally at Google for ages. It seems to be fairly well maintained, and the code is clean to my eyes. Chances are good that it already has solutions for much of what you may wish of a unit testing framework. By the way, I was originally not in favor of using gtest for Chromium. It seemed too complicated at first blush. I had created a very simple testing framework that I liked for all the reasons you state below. That was ~5 years ago. However, I quickly became more than convinced that it was worth it to use an established tool for unit testing. It has so many nice features--features I didn't even know I would appreciate. It was also really easy to use. -Darin On Wed, Apr 20, 2011 at 4:01 PM, Sam Weinig wei...@apple.com wrote: So, my questions for people who have used gtest is, Is it hackable? What kind of changes have you had success making? Darin's email said it so well that I hate to follow up with this, but in the interest of full disclosure, I'll include this info even though it may make folks feel less comfortable. (Before Darin's email) I talked to two people from chromium land who did changes to it. I believe those changes were to add support for FLAKY_ and FAILS_. Here's what those two people had to say to me about hacking gtest: Person 1: The codebase was somewhat hackable. The people maintaining it were not welcoming though. Most of the patches I tried to send ended up being R- because it was not important for them. However, they ended up writing a plugin API for it, and with that API it is a lot easier to make the needed improvement to gtest without having to do gtest changes. Person 2: gtest makes heavy use of templates and has many levels of indirection both of which made the code more difficult to deal with. Using it is fine. Hacking on it is definitely harder. In short, if we really need to make changes in gtest itself, it sounds pretty possible to do them. Getting them upstreamed may be harder, but we could always fork if needed. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] choosing a unit test framework
In the interest of moving this along, I'll move to using the criteria listed in the previous email to evaluate the test frameworks. *Musts:* - Compatible license with WebKit - Builds/Can be built on the many platforms and build systems supported by WebKit (ideally without extra installs). TestWebKitAPI and gtest meet these criteria. I don't think that QTest does. *Useful:* - Easy to write tests - Maintained TestWebKitAPI, gtest have these. - Hackable to suit our needs TestWebKitAPI meets this better as it is solely in WebKit. gtest is an open source project with outside contributions accepted. Also, gtest is flexible to allow for many uses and widely used in many scenarios so it is less likely to need modifications. - Supports filtering of tests so you can run just the test you care about (and easily listing the tests). TestWebKitAPI and gtest supports listing the tests and running a single test. Also, gtest supports running all tests, or matching a simple pattern (Xml*). - Well tested features (to support hackability/stability) - Supports writing out values when there is test failure. (For example, if the is verifying that A == B but that is not true, then the values of A and B should be printed.) - Well documented gtest has these features. To me, both gtest and TestWebKitAPI seem close. I feel like there is a slight edge for gtest in these criteria (plus it has other features that aren't used as often but are nice when you have the scenario). I'm sure that TestWebKitAPI could meet these criteria as well. One nice thing about having these features already done, tested and documented is that it will allow us to focus on other places where we can add value (as opposed to working on another unit test framework). dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] choosing a unit test framework
On Tue, Apr 19, 2011 at 2:42 PM, Ryosuke Niwa rn...@webkit.org wrote:
Can we see examples of tests that use TestWebKitAPI / gtest? I think it'll
be helpful for people who don't know how the said two testing frameworks
work.
Ideally, we compare the same test written in TestWebKitAPI and gtest to
decide which framework is better.
Here's a test using TestWebKitAPI
#include Test.h
#include JavaScriptCore/Vector.h
TEST(WTF, VectorBasic)
{
Vectorint intVector;
TEST_ASSERT(intVector.isEmpty());
TEST_ASSERT(intVector.size() == 0);
TEST_ASSERT(intVector.capacity() == 0);
}
Here's the same test written using gtest:
#include gtest.h
#include JavaScriptCore/Vector.h
TEST(WTF, VectorBasic)
{
Vectorint intVector;
ASSERT_TRUE(intVector.isEmpty());
ASSERT_EQ(0, intVector.size());
ASSERT_EQ(0, intVector.capacity());
}
With respect to writing a simple test, they both make it easy to write one,
so I don't think this is the deciding factor.
Dave
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] WebKit unit test framework
*Issue: *There has been a long standing bug to add unit tests to WebKit ( https://bugs.webkit.org/show_bug.cgi?id=21010). It was also mentionedhttp://lists.macosforge.org/pipermail/webkit-dev/2009-January/006359.htmlon webkit-dev that it would be helpful in various cases. *Landscape:* Surveying WebKit, it is looks like there are at least three testing frameworks being used: TestWebKitAPI/WebKitAPITest (in Tools), QTest, gtest (in Source/WebKit/chromium/). However, only one TestWebKitAPI has been used so far (as far as I can tell) for testing core WebKit items like WTF (though I was unaware of TestWebKitAPI until Friday). It seems like a good way to think about the issue of which to use in general in WebKit would be to decide on what would be desired in our framework and then see how each matches up. Here's my take on this. (It may be biased toward what I am familiar with but I welcome others to add their own criteria.) Criteria Musts: - Compatible license with WebKit - Builds/Can be built on the many platforms and build systems supported by WebKit (ideally without extra installs). Useful: - Easy to write tests - Hackable to suit our needs - Well tested features (to support hackability/stability) - Supports filtering of tests so you can run just the test you care about (and easily listing the tests). - Supports writing out values when there is test failure. (For example, if the is verifying that A == B but that is not true, then the values of A and B should be printed.) - Well documented thanks, dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Who are the EFL reviewers?
On Mon, Apr 11, 2011 at 3:40 AM, Tomasz Morawski t.moraw...@samsung.comwrote: Hi, Is it possible to promote some other peoples to reviewers in the EFL port? A step that I usually suggest to chromium folks before becoming reviewers is to actually do reviews on patches. Do everything except the r+ (with the submitter's permission). These are helpful to show when the reviewer nomination happens (as supporting evidence). If there are folks in the efl community who feel that they like to be reviewers, perhaps they can start doing this. (Then someone who is a reviewer can come along and give the final r+. I view it as a kind of mentorship thing because the reviewer should do a review as well and then the original person can learn from that if there were things that they missed.) dave Tomasz The problem is that as I am not working on the port myself, I find it quite hard to review their API's without getting input from someone else working on EFL. I think Antonio feels likewise. Kenneth On Mon, Apr 11, 2011 at 2:13 AM, Antonio Gomestoniki...@gmail.com wrote: Mostly myself and Kenneth. We do what we can, but we also to work on our stuff (as everybody else :). It really needs other reviewers to help out with reviewing, since EFL port guys are working really hard on it. On Sun, Apr 10, 2011 at 7:51 PM, Eric Seidele...@webkit.org wrote: We seem to have a zillion EFL patches up for review. Who are the EFL reviewers? (I think part of the trouble is that it seems the EFL port is trying to do too much in WebKit. I'm not sure where the EFL browser is, but some of the patches look like they should be re-directed to that project instead of WebKit.) -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- --Antonio Gomes ___ 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] WebKit2 build system
Hi, I was looking at a patch (https://bugs.webkit.org/show_bug.cgi?id=57535) in which it uses the OS() macro in some headers (to add a conditional include to define time_t). Unfortunately, when these headers are included from WebKit2 files, the build breaks (on various platforms) because OS() isn't defined. Typically OS() is defined by including config.h but that file doesn't seem to be in the cpp files for WebKit2. What is the proper way of fixing this? - include wtf/Platform.h directly in the header files affected (which seem wrong to me). - include config.h in WebKit2 files (but this appears not to be done). - other? Thanks, dave On Mon, Nov 29, 2010 at 1:20 PM, laszlo.1.gom...@nokia.com wrote: Hi, I'd like to warm up this old thread. Dependency on prefix header support seems to be a problem for ARM compiler (ARMCC/RVCT) builds as well (e.g. Symbian build). I've filed a bug to see if we can eliminate this build system dependency - https://bugs.webkit.org/show_bug.cgi?id=50174. We're considering posting a patch that explicitly includes the prefix header in all WebKit2 cpp files (just like config.h) - as suggested earlier. Concerns/better suggestions are welcome (before we touch 200+ files). Thanks, Laszlo -Original Message- From: webkit-dev-boun...@lists.webkit.org [mailto: webkit-dev-boun...@lists.webkit.org] On Behalf Of ext Kenneth Christiansen Sent: Wednesday, July 07, 2010 2:36 PM To: Sam Weinig Cc: webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] WebKit2 build system Currently it seems that at least icecc does not. Also, qmake which is the build system for the Qt port does not support prefix headers directly and we thus have to emulate it using the precompiled header support. Kenneth On Wed, Jul 7, 2010 at 2:27 PM, Sam Weinig sam.wei...@gmail.com wrote: It should not be necessary to use WebKitPrefix.h as a precompiled header, it is only necessary for it to be used a prefix header. Does discc also not support prefix headers? ___ 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] WebKit2 build system
Question answered (in irc): WebKit2 files should have config.h Thanks! On Mon, Apr 11, 2011 at 9:58 AM, David Levin le...@google.com wrote: Hi, I was looking at a patch (https://bugs.webkit.org/show_bug.cgi?id=57535) in which it uses the OS() macro in some headers (to add a conditional include to define time_t). Unfortunately, when these headers are included from WebKit2 files, the build breaks (on various platforms) because OS() isn't defined. Typically OS() is defined by including config.h but that file doesn't seem to be in the cpp files for WebKit2. What is the proper way of fixing this? - include wtf/Platform.h directly in the header files affected (which seem wrong to me). - include config.h in WebKit2 files (but this appears not to be done). - other? ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] bugid in ChangeLog
Here's a change that I felt worth getting someone to glance at but didn't feel worth the overhead of a bug: http://trac.webkit.org/changeset/81305 Since I was gardener and this was affecting the bots, it was a timely situation. (Sometimes getting in your fix right before another break comes in is important in these cases.) dave PS Dmitry found a flaw in my original change log text -- due to my haste, I originally had put in the wrong valgrind error. On Mon, Mar 28, 2011 at 9:58 AM, Jeremy Orlow jor...@chromium.org wrote: Can you please explain why? Its very little overhead and is useful for tracking regressions and such. J On Mar 28, 2011 9:52 AM, Darin Adler da...@apple.com wrote: On Mar 27, 2011, at 1:31 AM, Jeremy Orlow wrote: I'd even go a bit further and say that if something is worth a review (even if it's over the shoulder), it's worth a bug + a bug number. This is where I do not agree. Review is a requirement, but I don’t think bugs.webkit.org should be. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] bugid in ChangeLog
On Mon, Mar 28, 2011 at 10:44 AM, Jeremy Orlow jor...@chromium.org wrote: On Mon, Mar 28, 2011 at 10:06 AM, David Levin le...@chromium.org wrote: http://trac.webkit.org/changeset/81305 PS Dmitry found a flaw in my original change log text -- due to my haste, I originally had put in the wrong valgrind error. This seems like the kind of thing that'd be nice to put in the bug after the fact, no? :-) I don't understand what you mean. I created a patch with a bad changelog on my machine. Then, I changed it to be correct *before* I checked in. I don't understand the purpose of this (tps report ;) ). dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] bugid in ChangeLog
On Mon, Mar 28, 2011 at 11:48 AM, Maciej Stachowiak m...@apple.com wrote: On Mar 28, 2011, at 11:21 AM, Darin Adler wrote: On Mar 28, 2011, at 10:44 AM, Jeremy Orlow wrote: Sure I am open to discussion about that. I think that some check-ins, especially LayoutTest ones, don’t need change log entries. Personally, I like when the layout test rebaselines include why they needed to be done (citing the revision that necessitated the rebaselining). At that point I think they are informative and useful as opposed to rebaseline tests which is useless. The long list of tests rebaselined could be omitted without loss of informaiton, imo. What we need is a rationale for when change log entries are needed, and when that rationale was not met, the check-in could happen without one. Two benefits of ChangeLog entries apply to almost any kind of change: 1) They strongly encourage you to write a reasonably good commit message. (It might be possible to achieve that without ChangeLogs, but I have never seen a project that fails to use ChangeLogs and yet has comparably detailed commit messages.) 2) They let you see the change history of a file offline, even if you're not using a version control system that stores every revision locally. I think #1 is the most important, even if it's an indirect effect. It certainly applies to test cases. It might not apply to regression test meta-files, but I think it's better to have no exceptions than a very narrow exception. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] bugid in ChangeLog
On Sat, Mar 26, 2011 at 3:24 AM, Patrick Gansterer par...@paroga.comwrote: Hi, Sometimes folks commit changes without bug numbers. If those changes breaks things it's hard to find the correct context for the change. Can we make the bug number a requirement for a commit when it has a corresponding bug? IMHO it would be great if the style bot and the reviewer complain about missing bug numbers. For the style bot, the style checker is here: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/style/checker.py One would need to add a checker for the change log. You can see a similar change done here where Adam Roben added one to catch errors in xml files after he had a problem with someone messing this up: http://trac.webkit.org/changeset/74149 I'm willing to review such a change (or give tips as needed). dave - Patrick ___ 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] UA string changes blog draft
The blog post begs the question made me wonder. Why was Macintosh; kept when it is redundant with Intel Mac OS X 10_6_7? The reasoning seem analogous to what was given for why Windows; was removed. dave On Fri, Mar 25, 2011 at 11:44 AM, Peter Kasting pkast...@google.com wrote: I've incorporated all the existing feedback into the draft. Feel free to take another look. PK ___ 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] UA string changes blog draft
Ugh my strikethrough on begs the question was lost (and I meant that phrase as a joke). On Fri, Mar 25, 2011 at 11:54 AM, David Levin le...@google.com wrote: The blog post begs the question made me wonder. Why was Macintosh; kept when it is redundant with Intel Mac OS X 10_6_7? The reasoning seem analogous to what was given for why Windows; was removed. dave On Fri, Mar 25, 2011 at 11:44 AM, Peter Kasting pkast...@google.comwrote: I've incorporated all the existing feedback into the draft. Feel free to take another look. PK ___ 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] Build system update
On Wed, Mar 23, 2011 at 3:33 AM, Adam Barth aba...@webkit.org wrote: On Wed, Mar 23, 2011 at 12:22 AM, Mark Rowe mr...@apple.com wrote: In any case, I'm glad we've found a technically feasible solution. We've had at least one technically feasible solution from day zip: check in the generated project files. From my perspective, approach (2) is more desirable than checking in generated project files because approach (2) encapsulates Apple-internal build process to Apple folks, more specifically to the Apple folks who interact with the Apple-internal build system. Checking in generated project files, on the other hand, imposes a maintenance burden on all WebKit contributors. Living with the chromium system of generating the files on the fly, I always find it bothersome when the slowest step (by far) of my sync is generating these project files. So I personally prefer checking in the generated files (and letting this delay be on the person making the change -- rather than distributing this generation step to everyone). (As Mark mentions) this seems to also have the benefit of disrupting people's workflow the least. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] SnowLeopard debug buildbot not there
tL;dr Why isn't there a SnowLeopard debug buildbot? Related: Why does the commit queue (appear) to only run release builds through tests? Details: Yesterday, I did a build of WebKit on SnowLeopard and hit ~12 crashes (mostly in inspector tests). Then I realized that we don't have such a bot, so perhaps that's why these crashes aren't getting noticed by people. It also looks like the commit queue only runs release builds. I have a concern that not having this allows the code to become less stable than it should be. thanks, dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Towards a unified build system
On Tue, Mar 1, 2011 at 1:13 PM, Jeremy Orlow jor...@chromium.org wrote: We could quickly find ourselves in the position of generating build files for many different ports every time we submit patches, which could add several minutes to the submit process The alternative is to get everyone to spend this time :). (Seems better to submit them with the change from this perspective.) (which is even worse if you lose a race with another committer and have to update/rebase and try again). I did a log (git whatchanged --diff-filter=AD --pretty=oneline --abbrev-commit) and it appeared that there were about ~4 check-ins per day that added or deleted files, so this race wouldn't be hit very often. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Proposal: Let's make the WebKit2 test bot a core builder
r+'ed (only replied to keep others from clicking just to find out it was already done). On Sun, Feb 6, 2011 at 8:52 PM, Maciej Stachowiak m...@apple.com wrote: https://bugs.webkit.org/show_bug.cgi?id=53901 On Feb 6, 2011, at 2:37 PM, Adam Barth wrote: Go for it. The only requirement to be a core build is that the bot is green. If the bot is red for long stretches, we can remove it. Adam On Sun, Feb 6, 2011 at 2:16 PM, Maciej Stachowiak m...@apple.com wrote: I'd like to add SnowLeopard Intel Release (WebKit2 Tests) to the set of core builders. For the past few weeks, I have kept it green or close to green. This bot is successfully running 20266 tests, and has lately been more green than many bots that are core builders. The most common source of new failures is due to tests being added that rely on missing DumpRenderTree functionality and should be skipped. At times, though, there are also functional test regressions that for whatever reason do not show up on any of the other test bots. At times, people have mentioned to me that they left a failure in only because the bot is non-core and they didn't notice. I believe making this a core buildbot would reduce the level of redness. Any objections? Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] More information for crashing tests now available on build.webkit.org
Kudos! On Fri, Feb 4, 2011 at 11:25 AM, Adam Roben aro...@apple.com wrote: Hi all- The results.html pages on build.webkit.org now make it much easier to triage crashing tests on Mac and Windows XP (Windows Vista/7 are blocked by http://webkit.org/b/44135). When a test crashes, you'll see something like this on Mac: fast/events/tabindex-focus-blur-all.htmlhttp://trac.webkit.org/export/77639/trunk/LayoutTests/fast/events/tabindex-focus-blur-all.html stderrhttp://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r77639%20(14584)/fast/events/tabindex-focus-blur-all-stderr.txtcrash log (com.apple.WebCore: JSC::Bindings::Instance::willDestroyRuntimeObject(JSC::Bindings::RuntimeObject*) + 156)http://build.webkit.org/results/SnowLeopard%20Intel%20Leaks/r77639%20(14584)/fast/events/tabindex-focus-blur-all-crash-log.txt …and like this on Windows: fast/events/tabindex-focus-blur-all.htmlhttp://trac.webkit.org/export/77636/trunk/LayoutTests/fast/events/tabindex-focus-blur-all.html stderrhttp://build.webkit.org/results/Windows%20XP%20Debug%20(Tests)/r77636%20(24918)/fast/events/tabindex-focus-blur-all-stderr.txtcrash log (WebKit!JSC::Bindings::Instance::willDestroyRuntimeObject+95)http://build.webkit.org/results/Windows%20XP%20Debug%20(Tests)/r77636%20(24918)/fast/events/tabindex-focus-blur-all-crash-log.txt The new crash log link will take you to a textual crash log for that test. The function name you see is our best guess at the crashing module, function, and offset (in decimal on Mac, hexadecimal on Windows), based on the crash log. Please file any bugs you find with this feature on bugs.webkit.org, and CC me. Please also file bugs for any ideas you have for making this more useful! -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
[webkit-dev] Modification to WebKit reviewer nominations.
As the WebKit community continues to grow rapidly, it is useful to consider how to retain its health and culture. One important aspect of our community has been the way that people work together regardless of their affiliations. In order to emphasize this tradition, the WebKit reviewers have agreed to update the policy for new reviewers nominations. In short, new nominations will require one more supporting reviewer who is not in the same place of employment or project as the nominee. You can see the change here: http://trac.webkit.org/changeset/76367 Thank you, dave on behalf of the WebKit reviewers ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] More thoughts on cleaning up the root directory
On Mon, Dec 27, 2010 at 7:17 PM, Maciej Stachowiak m...@apple.com wrote: On Dec 27, 2010, at 1:11 PM, Sam Weinig wrote: On Mon, Dec 27, 2010 at 11:14 AM, Adam Barth aba...@webkit.org wrote: Is it really a good idea to move platform out of WebCore? Lots of stuff there seems quite WebCore related. There seem to be a couple people who aren't sold on moving platform out of WebCore. It sounds like we should hold off on doing that and discuss it separately down the road. On Mon, Dec 27, 2010 at 2:47 AM, Hajime Morita morr...@google.com wrote: Platform/ (was WebCore/platform) I'd like to keep platform directory under WebCore if there is no strong reason. Ok. I think different people have slightly different ideas about what should go into this folder. That sounds like a complex topic that we might need to discuss more later. I think moving Platform out from WebCore is great long term goal, but right now, there is simply too many layering violations for it to be feasible. For those curious, the intent is for nothing in Platform to be dependent on anything else in WebCore (eg. dom, html, rendering, loader), so something like platform/qt/RenderThemeQt.cpp would be considered a layering violation. There are bugs filed on many of these violations, but the work has not be completed. Indeed, that's the reason I suggested to Adam that we should move platform out of WebCore eventually. It would make the layering intent more clear and would let us enforce the layering by making it a compile-time error to depend on other parts of WebCore inside the platform directory. fwiw, this change https://bugs.webkit.org/attachment.cgi?id=73254action=prettypatch if/when finished would help a lot with enforcing it. (The enforcement in there is likely too aggressive as previously discussed). However, I don't think we should make this change part of the initial reorg. It's something we could do down the line once we have had time to fix up more of the layering violations. Note: we could also whitelist specific files with known layering violations if we want to make this change before we eliminate all layering violations. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Renaming directories
Sounded like MIT license is considered permittable: https://lists.webkit.org/pipermail/webkit-dev/2010-January/011406.html On Wed, Dec 22, 2010 at 1:05 PM, Kenneth Russell k...@google.com wrote: On Wed, Dec 22, 2010 at 10:53 AM, Mark Rowe mr...@apple.com wrote: On 2010-12-22, at 10:45, Kenneth Russell wrote: I see. The GLU tessellator was integrated because it was the only viable option for helping implement GPU-accelerated path rendering in WebKit. (This work is still in early stages, and the tessellator isn't yet being compiled in to WebKit.) The code is covered by an MIT X11 style license which is compatible with the BSD license WebKit uses. I'm not a lawyer so I can't speak to the compatibility of the various open source licenses. I will note however that the committer agreement that all committers have signed explicitly states that only the BSD and LGPL licenses are permitted for code checked in to the WebKit repository. I apologize, I didn't mean to violate the WebKit committer agreement. There was extensive discussion on IRC and in person before checking in this and related code. If its continued presence poses a problem then please let's try to figure out a solution that allows continued development of this functionality. -Ken ___ 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] Post positive reviews
r- On Mon, Dec 13, 2010 at 12:01 PM, Evan Martin e...@chromium.org wrote: That's a good point. The overwhelming majority of reviews of WebKit are negative -- they even have a shorthand for it, r-. It seems inevitable that the project is going to drop out of Google anytime now. :( On Mon, Dec 13, 2010 at 11:30 AM, Eugene Zola angelar...@gmail.com wrote: Google’s Huge Change and How it affects you. •Anyone can now post bad reviews and kill your rank. •We post good reviews and improve your rank. •We post good reviews to keep others from killing your rank. Google: Judge, Jury and Online Shopping Executioner Google rank is based on reviews of your business? Google Statement: ...in the last few days we developed an algorithmic solution which detects the merchant from the Times article along with hundreds of other merchants that, in our opinion, provide an extremely poor user experience. The algorithm we incorporated into our search rankings represents an initial solution to this issue, and Google users are now getting a better experience as a result. This means that anyone can write bad reviews about your business and lower your ranking. We knew that getting good reviews and not getting bad reviews was always important. Now it is a must to have good reviews for your business to keep the rank safe or to improve rank with Google. We post positive reviews for your company. We have the experience and ability to post hundreds of positive reviews that are all unique content and posted on unique IP addresses. Visit www.reviewpostingservice.com com for more information. ___ 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] Waiting time for patch with commit? status.
I don't know if my behavior is typical but I almost never scan the commit queue. I occasionally scan the review queue. So changing an already r+'ed bug to have cq? may not get noticed. I'd ping someone to be sure (and there are more committers than reviewers so you have more options even). dave On Tue, Nov 30, 2010 at 10:58 AM, Eric Seidel e...@webkit.org wrote: Marking commit-queue? is just like marking review?. Mails get sent, but you often have to bug people to get your review+ or commit-queue+. -eric On Tue, Nov 30, 2010 at 8:40 AM, Jia Pu j...@apple.com wrote: Hello, If I flag a patch with commit?, do I need to notify some commiters? Or are those commit? patches get periodically processed by someone, I just need to wait. If the latter is the case, what's the average waiting time. Thanks. Jia ___ 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] Faster Git SVN updates
Added by dimich, but it is a tip from Albert and I think how most chromium enlistments for git are setup iirc). dave On Thu, Nov 18, 2010 at 3:32 PM, Eric Seidel e...@webkit.org wrote: There is this section of the WebKit Git wiki ( http://trac.webkit.org/wiki/UsingGitWithWebKit): If you don't fetch new revisions from Subversion very often and find fetching them one by one too slow, you can modify the svn section in your .git/config file to point directly to the *refs/remotes/origin/master* rather then*refs/remotes/trunk* which is how it is set up by default. In this case 'git svn fetch' will be way faster if done after git fetch or git pull, since it'll realize it already has all the revisions locally. Edit your svn entry to look like this: [svn-remote svn] url = http://svn.webkit.org/repository/webkit fetch = trunk:refs/remotes/origin/master and then re-build the svn index by doing 'git svn fetch' once. It's some magical setup by which your git svn fetchs will be much faser. But I've heard it's buggy? Can lead to local repository corruption? Can someone set me straight? The current git svn fetch is *super* slow. Especially if you're behind by more than a day or two. If there was a way to make this faster method safe, by wrapping it in some other (error-checking) command which knew how to fall back to git svn rebase, etc. when necessary I would love to make it the default method for all WebKit get users. Thoughts? -eric ___ 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] deleteOwnedPtr(T* ptr)
Do or do not. Then, there is no question. :)
btw, in another code base, you may familiar with, the comment is the
variable name:
http://www.google.com/codesearch?q=type_must_be_completeexact_package=chromium
On Thu, Nov 11, 2010 at 12:24 AM, Finnur Thorarinsson
fin...@chromium.orgwrote:
Umm... shouldn't this behavior be commented so that people are not left
wondering why it fails and trying to fix it?
On Wed, Nov 10, 2010 at 16:04, Darin Adler da...@apple.com wrote:
On Nov 10, 2010, at 2:33 PM, Daebarkee Jung wrote:
I found that the following lines made errors:
// OwnPtrCommon.h
template typename T inline void deleteOwnedPtr(T* ptr)
{
typedef char known[sizeof(T) ? 1 : -1];
if (sizeof(known))
delete ptr;
}
I am very curious about why the author wrote like the above.
What could be the author's intention?
The code is to prevent issues like the ones described on these websites:
http://stackoverflow.com/questions/1767679/incomplete-type-memory-leaks
http://bytes.com/topic/c/answers/611877-gcc-class-forward-declarations-destructor-calls
http://connect.microsoft.com/VisualStudio/feedback/details/231177/delete-of-pointer-to-incomplete-class
If we delete a pointer and the object has incomplete type, we get
undefined behavior. Instead this code causes compilation to fail if the
object has incomplete type. The use of a negative number for the size of an
array is a way to guarantee we get a compilation error.
Your alternate version might also work; I’m not sure.
-- Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Coding style change - Indentation of forward declarations in headers
On Mon, Nov 1, 2010 at 10:06 AM, Brady Eidson beid...@apple.com wrote:
On Nov 1, 2010, at 10:00 AM, David Hyatt wrote:
Yeah I agree with Peter. I think blank lines after { and before } would
improve the readability of the 2nd example even without indentation.
namespace WebCore {
class AuthenticationChallenge;
class CachedFrame;
class HistoryItem;
class ProtectionSpace;
class ResourceLoader;
class ResourceRequest;
}
I agree this also makes it more readable, but...
This doesn't seem worth making a special case for to me.
If the general sentiment is I don't have a strong preference either way,
then I would argue that we should allow its continued used simply because so
much code already uses it.
On the other hand, less rules/exceptions = less mental clutter for me to
keep track of = easier for folks to get right, imo.
But I do believe that you are simply stating a rule that is already followed
and just not written, so while I prefer less rules/exception, I don't object
to this.
dave
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Should we recommend explicit constructors as part of WebKit style?
It sounds like there is consensus. Here's a patch for adding this to the style guide: https://bugs.webkit.org/show_bug.cgi?id=47646 dave On Wed, Sep 29, 2010 at 9:41 AM, Darin Adler da...@apple.com wrote: On Sep 28, 2010, at 4:31 PM, Maciej Stachowiak wrote: I think the rule should be something like: 3. Do not explicit when the single-argument constructor can be thought of as a type conversion - the class will be in some sense an alternate form of its sole parameter. Do use explicit when the single-argument constructor is *not* reasonably thought of as a type conversion - the single argument just happens to be the sole initialization parameter. Or to put it another way - can you imagine having a type conversion operator overload that does the same thing as this constroctor? Applying this rule to your two examples, Vector(int) should be explicit, because it doesn't convert the int to a vector, it uses the int as a size instead of the default one. But String(AtomicString) or AtomicString(String) need not be explicit, since they convert from one string type to another, carrying largely the same data. I realize this rule requires some judgment, so it's worse than a purely mechanical rule, but I think it accurately captures the proper use of explicit. This seems like a good rule. I agree completely that it’s the right principle. When the conversion between types is costly enough, there may be some cases where we don’t want to offer an implicit constructor. Specifically, the implicit conversion from String to AtomicString may be a mistake; we might be better off if we had to utter some explicit function name every time we wanted a string looked up in the atomic string table. I don’t think of this, though, as a rule for when to use “explicit”. It’s a rule for when to use a constructor that can be used implicitly for type conversion. If a constructor can’t be used that way, an explicit constructor might be OK, or you might not want the other constructor to exist at all. For example, if we want to be more explicit about the cost of looking up an AtomicString, the syntax for making an AtomicString from a String should probably be a named function, not AtomicString(string). -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] PSA: Don't try to hold onto temporaries with references
On Tue, Oct 5, 2010 at 3:42 AM, Peter Kasting pkast...@chromium.org wrote:
On Mon, Oct 4, 2010 at 4:23 AM, Leandro Graciá Gil
leandrogra...@chromium.org wrote:
In summary, looking at code like this
B b = c-foo();
...
b.m();
If c-foo() returns a temporary (return B();), then it is safe.
Maybe I'm wrong, but are you completely sure about this one? I would say
that the temporary object created in return B() will cease to exist as soon
as it returns (just after the constructor finishes).
foo() is returning a temp by value. On the caller side, that value is
copied to a (hidden) temp whose lifetime is the same as the lifetime of |b|,
and then |b| is set to be a reference to that temp.
By contrast, if foo were returning a temp by reference, then the reference
would be invalid on return because the (foo()-scoped) temp it referred to
would be destroyed when foo() exited.
Thanks Darin and Peter.
I left out an important detail: the full function signature .(I mentally
used my standard way of writing such code.)
#1 was B foo() { return B();}
vs
#2 was const B foo() { return m_b; }
I suspect that the the example code written to test it looked like this:
B foo() { return B();}
PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] PSA: Don't try to hold onto temporaries with references
Thanks Peter and Darin. In summary, looking at code like this B b = c-foo(); ... b.m(); If c-foo() returns a temporary (return B();), then it is safe. If c-foo() returns a reference to a member variable (return m_b;), then it is up to the lifetime of of c-m_b. The cases that Adam changed were instances of the former. dave On Mon, Oct 4, 2010 at 12:36 PM, Peter Kasting pkast...@chromium.orgwrote: On Sun, Oct 3, 2010 at 10:31 AM, Darin Adler da...@apple.com wrote: What you say here about object lifetime is not correct. I thought the same thing a year or so back. But the C++ language keeps these objects alive until the end of the block. Correct. One helpful section from the standard (12.2/5 Temporary objects): The temporary to which the reference is bound or the temporary that is the complete object to a subobject of which the temporary is bound persists for the lifetime of the reference except as specified below. A temporary bound to a reference member in a constructor’s ctor-initializer (12.6.2) persists until the constructor exits. A temporary bound to a reference parameter in a function call (5.2.2) persists until the completion of the full expression containing the call. Adam's changes will not make any functional difference. PK ___ 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] Should we recommend explicit constructors as part of WebKit style?
This came up before: https://lists.webkit.org/pipermail/webkit-dev/2010-May/012873.html but I'd like to understand it a bit better. It feels there were two points of view: 1. Use explicit only when necessary to prevent an undesirable implicit conversion like int to vector. 2. Use explicit except when it is desirable to allow an implicit conversion that makes the code simpler. For example, the String - AtomicString makes the bindings generator code simpler since it doesn't need to know which the underlying method takes. Are there any reasons beyond personal preference to select either of these? Starting list: Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument. Pro's for #2 It would prevent accidental mistakes that happen with implicit constructors. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Should we recommend explicit constructors as part of WebKit style?
On Tue, Sep 28, 2010 at 4:30 PM, Oliver Hunt oli...@apple.com wrote: Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument. Could check-webkit-style be beaten into forcing this for us? Yep, check-webkit-style could check this automatically, but it wouldn't be subtle, so it would flag cases where we want an implicit constructor. It would just have to point to the guideline. (The check is currently turned off because this isn't WebKit style.) On Tue, Sep 28, 2010 at 4:29 PM, Darin Fisher da...@chromium.org wrote: We're there some recent mishaps with implicit constructors that motivate this thread? I've noticed Adam Barth putting comments in reviews about it (and check-webkit-style could do it automatically for him). I tend to think it is a good thing (and have been bitten in the past by not doing it), but I don't usually notice it in reviews. dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Bug system: Platform and OS fields don't quite align
On Tue, Sep 14, 2010 at 10:28 AM, Adam Barth aba...@webkit.org wrote: We're not very good at using these fields in Bugzilla. In this situation, folks will usually prefix the summary of the bug with [Chromium]. This is a signal to reviewers that they might be more or less interested in reviewing the patch depending on whether they like/feel comfortably with reviewing patches to WebKit/chromium. Note that bugs should only have [Chromium] in the title if the patch *only* touches Chromium specific files. (Right now, I consider this to be skia related files, v8 related files, and then the files that are obviously chromium due to name or directory structure -- I say right now because I see there are efforts to make skia useful outside of Chromium.) Adam On Tue, Sep 14, 2010 at 10:20 AM, Mike Belshe m...@belshe.com wrote: Hi, I tend to hit code which is often chromium-platform specific. It's hard to know the appropriate Platform and OS fields for such a bug. For instance, I am working on a small change to WebKit/chromium/src/WebKit.cpp. It's not a Mac bug, its not a PC bug, its a Chromium bug. Chromium is a platform of sorts. Chromium bugs could be OS-specific (like Windows, MacOS, etc). So I think the platform field would be the right place to surface such a thing. Should the bug system surface a platform for Chromium? If so- perhaps there are other platforms to surface as well. I'm not sure when a particular flavor of webkit warrants a field in the bug system. If we aren't willing to reflect this through the bug system, what are the right values for these fields? I'm guessing the apple guys want to filter out these bugs - how do you do it today? Thanks, Mike ___ 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] Blob scheme implementation
On Tue, Sep 14, 2010 at 5:42 PM, Adam Barth aba...@webkit.org wrote: What do you think of the idea of having a re-useable BlobCore module that all the ports can share? I don't think this is a good idea. This re-usable module would only be used by the Safari WebKit port. As I understand it, Chromium wouldn't be able to re-use it due to not re-using WebKit types in general. With only one port using it, the module seems like it would not be able to have a good design. So if there is a change, it seems better to just write it for the Safari WebKit port and as other ports want to implement it, if they find commonality, it would be in their best interest to refractor the existing code for better re-use. dave Adam On Tue, Sep 14, 2010 at 5:39 PM, Jian Li jia...@chromium.org wrote: When I implemented the blob scheme handling, I intentionally tried to have some common implementation that could be used by all applicable platforms. But it seems that introducing BlobResourceHandle derived from ResourceHandle might not be a good hook up point because ResourceHandle is only a wrapper around the platform loading logics. To fix this problem, I can move all the blob handling logic to the platform specific layer (for WebKit mac) and it is up to other individual platform to implement it when needed. Jian On Tue, Sep 14, 2010 at 5:22 PM, Adam Barth aba...@webkit.org wrote: Jian Li just had a conversation in #webkit about where the code for implementing the Blob URL scheme should live. I thought I'd open the discussion to webkit-dev in case folks had a different perspective. As part of the fileapi, we're introducing a new URL scheme, called blob, which represents a bucket of bits, usually from a file, but potentially from another location. Assigning these objects URL is helpful because then they integrate with the rest of the web platform. For example, you can use them as images via the img element or videos via the video element, etc. Currently, the blob URL scheme is implemented with a subclass of ResourceHandle (our primary network abstraction) called BlobResourceHandle. My sense is that this isn't the right place in the architecture to add support for the blob URL scheme. The issue is that each port has a table somewhere that maps URL schemes to networking backends. In Chromium, for example, that mapping is provided by URLRequestFactory, which lives in the net module. By implementing the blob URL scheme at the ResourceHandle layer, we're short-circuiting that table. In some sense, this is analogous to adding an HTTPResourceHandle and implementing the HTTP protocol inside of WebCore. While its true that most (all?) users of WebKit will need to wire WebCore up to an HTTP library, that doesn't necessarily mean that WebCore should contain an implementation of the HTTP protocol. In the same way, even if a large number of WebKit users will wish to support the blob URL scheme, that doesn't necessarily mean that WebCore should contain an implementation of the scheme. I can certainly see the appeal of sharing the blob URL implementation code between different ports of WebKit, but we can achieve that goal in a number of ways, including creating an external library or a separate BlobCore module. 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] Blob scheme implementation
On Tue, Sep 14, 2010 at 6:53 PM, Oliver Hunt oli...@apple.com wrote: On Sep 14, 2010, at 5:56 PM, David Levin wrote: On Tue, Sep 14, 2010 at 5:42 PM, Adam Barth aba...@webkit.org wrote: What do you think of the idea of having a re-useable BlobCore module that all the ports can share? I don't think this is a good idea. This re-usable module would only be used by the Safari WebKit port. As I understand it, Chromium wouldn't be able to re-use it due to not re-using WebKit types in general. With only one port using it, the module seems like it would not be able to have a good design. What about Gtk, Qt, Wx, Efl...? Where possible these days we seem to implement a single impl in webcore that is used by everyone Indeed, it is a worthy goal. In fact, the current implementation does that, but there were some concerns about how it was accomplished. So Adam's proposal is to implement a re-usable module which is wired up by each individual port down in the port's specific implementation by each platform's maintainers. I was simply remarking that it doesn't seem good to start out with a generalization that only one port is using. (In the past when I've done generalizations of this sort, it tends to be too generic and a lot more complicated/complex than necessary.) Instead if the code were just done for WebKit OSX, when a second port starts to do something, it could break out the code that is generic because that would result in a design that is done at the right level. dave --Oliver ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Use of Frame by ResourceHandle
On Sat, Sep 11, 2010 at 11:07 PM, Adam Barth aba...@webkit.org wrote: On Sat, Sep 11, 2010 at 10:52 PM, Darin Fisher da...@chromium.org wrote: On Sat, Sep 11, 2010 at 10:42 PM, Adam Barth aba...@webkit.org wrote: On Sat, Sep 11, 2010 at 10:02 PM, Darin Fisher da...@chromium.org wrote: I don't understand. WebWorkers use ThreadableLoader, which routes the network request back to the main thread where there is an associated Frame. (SharedWorkers have a dummy frame associated with them.) See. The dummy frame sounds unfortunate. It solved/avoided a load of problems/complexity. What are your concerns? Having fake versions of objects add complexity to all the code that expects to talk to real versions of those objects. For example, SVG-in-img creates a ton of fake objects and has been the source of a lot of bugs (including security bugs). It seems like having a notion of a networking context makes more sense than pretending shared workers are associated with a rectangular region of a screen somewhere. A clarification: The fake frame only happens in Chromium. It is due to the fact that workers are in a different process from the real frame. In !chromium platforms, the real frame is used to send the request for both dedicated and shared workers. (It is a bit unfortunate in the shared worker case because closing that frame will kill the xhr request but the reasoning has been that code should be resilient to xhr failures as they can happen for a number of reasons.) dave In general, there are also situations on the main thread where we'd like to perform a load without a Frame. I'd have to look at the details, but there are long-standing bugs about applying XSLT to Frame-less documents. Also, the PingLoader doesn't have a Frame available (it's job is to make image requests that outlive the Frame). PingLoader has an associated Frame when it kicks off the load. That is the critical time when Frame association is usually needed. What happens when code later in the loading cycle assumes this Frame is still present? To avoid exploding, that code needs to understand that in this tiny corner of the loader, life is different, which is a big testing and maintenance burden. For example, you cannot load any network requests in Chromium unless you know what Page (you need to know the routing ID of the tab) is requesting the resource. I assume PingLoader still generates the FrameLoaderClient::dispatchWillSendRequest notification, right? I don't think so. PingLoader talks directly to ResourceHandle. PingLoader knows about the Frame, but it looks like it only uses it to determine the outgoing referrer, to addExtraFieldsToSubresourceRequest, and to grab the networking context. How do you get a frame-less document? Via XMLHttpRequest.responseXML? Perhaps it could use the Frame of the script execution context? (Which script execution context is a good question.) There are are lots of ways to get a Frameless document. For example, JavaScript can call document.implementation.createDocument. Also, the DOMParser will given you a document. XMLHttpRequest will give you one. You can get one by having an XSLT. The PageCache has some. There was a patch that someone was pushing at some point to chain these documents back to a master document that has a frame. That's certainly one approach, but I don't think it should be necessary. In general, there is no necessary connection between network requests made by WebCore and Frames. Techniques that aim to associate a frame with every network request won't work in some cases because such a Frame might not exist. There always has been such an association. Right, and there are bugs we've never been able to fix because of that coupling. I would like to understand the concerns better. I guess it means that I need to understand the frame-less document issue and why you think having a dummy frame associated with shared workers is a problem. Here's an example bug from 2006 that's marked Critical: https://bugs.webkit.org/show_bug.cgi?id=10313 The patch attached to that bug is a giant workaround for the fact that the loader is too dependent on Frame. 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] Use of Frame by ResourceHandle
On Sun, Sep 12, 2010 at 12:37 PM, Adam Barth aba...@webkit.org wrote: On Sun, Sep 12, 2010 at 12:33 PM, David Levin le...@chromium.org wrote: On Sat, Sep 11, 2010 at 11:07 PM, Adam Barth aba...@webkit.org wrote: Having fake versions of objects add complexity to all the code that expects to talk to real versions of those objects. For example, SVG-in-img creates a ton of fake objects and has been the source of a lot of bugs (including security bugs). It seems like having a notion of a networking context makes more sense than pretending shared workers are associated with a rectangular region of a screen somewhere. A clarification: The fake frame only happens in Chromium. It is due to the fact that workers are in a different process from the real frame. In !chromium platforms, the real frame is used to send the request for both dedicated and shared workers. (It is a bit unfortunate in the shared worker case because closing that frame will kill the xhr request but the reasoning has been that code should be resilient to xhr failures as they can happen for a number of reasons.) Once the Frame is gone, XHR stops functioning for the SharedWorker permanently? That sounds like a bug that should be fixed. Nope only for that xhr request. Basically, the code picks a frame to send out the request through everytime that a request is done. If that frame goes away, then the request will fail but another request will go through a different frame. One idea of how to fix this is to use a fake frame for sending out the request from the shared worker :). The other idea is that code should be resilient to occasional xhr failures. dave Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Script programming language: Perl, Python, or Ruby?
From reading this thread and the bug, it sounds like there is one key issue installing something new on build machines. As discussed before in regards to python, there was a desire not to go above a certain version so that the tiger build machine could run a script. That seemed reasonable to me. Similarly there is a desire in this case to avoid Ruby because it means installing something new on other build machines. This also seems reasonable to me. There are other issues that seem tangential which are being brought up and only confuse the subject for me: - Speed -- without numbers this is meaningless to me. - A hang that might be reduced -- since it will remain, it sounds like there is a deeper issue to fix. - People like python -- me too, but perhaps not a deciding factor. It probably didn't help that the subject of the email focuses on languages in a highlander smack down style as if there can be only one. Focusing again, given the build machine issue: - Is it best to have a separate script that is similar to PrettyPatch solve the issue? - Or replace the script? - Or can the dependency on PrettyPatch be controlled via a commandline option and just turned off on build machines? - Or have folks explain why they don't want to install more on their build machine? dave ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] style question for empty for loops
Option 1 Seems in keeping with what is done for constructors/functions w/o
bodies, so for consistency, I think it it preferable.
On Thu, Aug 26, 2010 at 11:35 AM, Chris Fleizach cfleiz...@apple.comwrote:
Which is preferred?
for (...; ...; ...) { }
or
for (...; ...; ...)
{ }
___
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] style question for empty for loops
On Thu, Aug 26, 2010 at 1:12 PM, Chris Fleizach cfleiz...@apple.com wrote:
webkit-check-style should probably be amended as well
Please file a bug. Feel free to cc ham...@chromium.org, le...@chromium.org,
cjerdo...@webkit.org
On Aug 26, 2010, at 12:48 PM, James Robinson wrote:
The style guide currently covers this
http://webkit.org/coding/coding-style.html:
4. Control clauses without a body should use empty braces: Right:
for ( ; current; current = current-next) { }
Wrong:
for ( ; current; current = current-next);
- James
On Thu, Aug 26, 2010 at 12:22 PM, Chris Fleizach cfleiz...@apple.comwrote:
On 26. aug. 2010, at 11.49, Darin Adler wrote:
On Aug 26, 2010, at 11:35 AM, Chris Fleizach wrote:
for (...; ...; ...) { }
So maybe this is the best option. I can add a style guide check for that,
unless there are objections
___
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
