[webkit-dev] ENABLE(INSPECTOR) check in Console::lastWMLErrorMessage
Hi, Looks like it should add ENABLE(INSPECTOR) check in Console::lastWMLErrorMessage() (/trunk/WebCore/page/Console.cpp) Thanks. Regards Ryan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] ENABLE(INSPECTOR) check in Console::lastWMLErrorMessage
Hello Ryan, Looks like it should add ENABLE(INSPECTOR) check in Console::lastWMLErrorMessage() (/trunk/WebCore/page/Console.cpp) Thanks. Could you file a bug using this link? http://webkit.org/new-inspector-bug Please include any relevant error messages you're getting, such as a compiler warning if there is a missing ENABLE(INSPECTOR) check. It looks like you're compiling with WML enabled but without INSPECTOR enabled. That would catch this problem! If thats it the case someone will likely jump on this pretty quick. If you want you can assign this to me and I can handle this tomorrow. Better yet, if you'd like to try your hand and contributing a patch that would be great! You can start here: http://webkit.org/coding/contributing.html Thanks! - Joe ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Root (accessible) element in WebKitGtk+
Hi,
I've recently started to look at a11y issues in WebKitGtk (nothing too
impressive, just taking a look by the moment :-)), and I found the
implementation a bit strange, and not sure whethers there's a reason for
that I'm missing (most likely, I'd say):
// WebKitTools/DumpRenderTree/gtk/AccessibilityControllerGtk.cpp
AccessibilityUIElement AccessibilityController::rootElement()
{
WebKitWebView* view = webkit_web_frame_get_web_view(mainFrame);
// The presumed, desired rootElement is the parent of the web view.
GtkWidget* webViewParent = gtk_widget_get_parent(GTK_WIDGET(view));
AtkObject* axObject = gtk_widget_get_accessible(webViewParent);
return AccessibilityUIElement(axObject);
}
As far as I can understand that code, the AtkObject being returned here
is the a11y object for the parent of the WebView widget, which it's
going to be another GtkWidget, most likely an GtkVBox, or some kind of
container like that.
However, as I can extract from the (a11y) tests, the root element should
return instead the a11y object associated to the root of the document
being rendered, which seems to be the body element, as far as I can
understand from other similar tests.
Hence, if my guessings are right, that would mean the implementation of
this function would be wrong and that perhaps some kind of solution as
the proposed in the attached patch would be a better one, but not sure
enough yet as to file a bug, so I'd appreaciate if someone could shed
some light on this topic before.
So that's it... any idea/comment on this?
Thanks in advance,
Mario
PS: FYI, the proposed patch gets the same results wrt a11y tests (same
number of failed/succeeded tests) with just a difference: the test
aria-controls-with-tabs.html stops outputting the following error to
stderr:
atk_object_ref_accessible_child: assertion `ATK_IS_OBJECT (accessible)'
failed
atk_object_ref_accessible_child: assertion `ATK_IS_OBJECT (accessible)'
failed
atk_object_ref_accessible_child: assertion `ATK_IS_OBJECT (accessible)'
failed
So maybe it's not a so bad patch after all :-)
diff --git a/WebKit/gtk/webkit/webkitprivate.h b/WebKit/gtk/webkit/webkitprivate.h
index 2642d50..d1a02c6 100644
--- a/WebKit/gtk/webkit/webkitprivate.h
+++ b/WebKit/gtk/webkit/webkitprivate.h
@@ -325,6 +325,9 @@ extern C {
webkit_web_frame_clear_main_frame_name(WebKitWebFrame* frame);
WEBKIT_API AtkObject*
+webkit_web_frame_get_root_accessible_element(WebKitWebFrame* frame);
+
+WEBKIT_API AtkObject*
webkit_web_frame_get_focused_accessible_element(WebKitWebFrame* frame);
WEBKIT_API gchar*
diff --git a/WebKit/gtk/webkit/webkitwebframe.cpp b/WebKit/gtk/webkit/webkitwebframe.cpp
index 344f94e..49002d1 100644
--- a/WebKit/gtk/webkit/webkitwebframe.cpp
+++ b/WebKit/gtk/webkit/webkitwebframe.cpp
@@ -1116,7 +1116,7 @@ gsize webkit_gc_count_javascript_objects()
}
-AtkObject* webkit_web_frame_get_focused_accessible_element(WebKitWebFrame* frame)
+AtkObject* webkit_web_frame_get_root_accessible_element(WebKitWebFrame* frame)
{
g_return_val_if_fail(WEBKIT_IS_WEB_FRAME(frame), NULL);
@@ -1133,10 +1133,22 @@ AtkObject* webkit_web_frame_get_focused_accessible_element(WebKitWebFrame* frame
return NULL;
AtkObject* wrapper = priv-coreFrame-document()-axObjectCache()-getOrCreate(root)-wrapper();
-if (!wrapper)
+return wrapper;
+#else
+return NULL;
+#endif
+}
+
+AtkObject* webkit_web_frame_get_focused_accessible_element(WebKitWebFrame* frame)
+{
+g_return_val_if_fail(WEBKIT_IS_WEB_FRAME(frame), NULL);
+
+#if HAVE(ACCESSIBILITY)
+AtkObject* root = webkit_web_frame_get_root_accessible_element (frame);
+if (!root)
return NULL;
-return webkit_accessible_get_focused_element(WEBKIT_ACCESSIBLE(wrapper));
+return webkit_accessible_get_focused_element(WEBKIT_ACCESSIBLE(root));
#else
return NULL;
#endif
diff --git a/WebKitTools/DumpRenderTree/gtk/AccessibilityControllerGtk.cpp b/WebKitTools/DumpRenderTree/gtk/AccessibilityControllerGtk.cpp
index ab9f021..022ced1 100644
--- a/WebKitTools/DumpRenderTree/gtk/AccessibilityControllerGtk.cpp
+++ b/WebKitTools/DumpRenderTree/gtk/AccessibilityControllerGtk.cpp
@@ -35,6 +35,7 @@
#include webkit/webkit.h
extern C {
+extern AtkObject* webkit_web_frame_get_root_accessible_element(WebKitWebFrame*);
extern AtkObject* webkit_web_frame_get_focused_accessible_element(WebKitWebFrame*);
}
@@ -63,13 +64,11 @@ AccessibilityUIElement AccessibilityController::focusedElement()
AccessibilityUIElement AccessibilityController::rootElement()
{
-WebKitWebView* view = webkit_web_frame_get_web_view(mainFrame);
+AtkObject* accessible = webkit_web_frame_get_root_accessible_element(mainFrame);
+if (!accessible)
+return 0;
-// The presumed, desired rootElement is the parent of the web view.
-GtkWidget* webViewParent = gtk_widget_get_parent(GTK_WIDGET(view));
-AtkObject* axObject =
Re: [webkit-dev] Function Property Names
Hi,
ext Nyx wrote:
I'm in the process of writing a program to analyze traces of JavaScript code.
This involves logging events that occur in the interpreter. Currently, I'm
trying to just log function calls and property accesses. However, I'm unsure
exactly how to go about getting the names (identifiers) associated with
functions (and properties).
I wrote the following piece of code just to test things out, which I
inserted in Interpreter.cpp, in the definition of the op_call opcode, after
the vPC = newCodeBlock-instructions().begin(); line:
JSGlobalObject* globalObject = callFrame-scopeChain()-globalObject;
printf(Function call: %s\n,
asFunction(v)-name(globalObject-globalExec()).ascii());
printf(%s\n, newCodeBlock-ownerExecutable()-sourceURL().ascii());
printf(%i\n, newCodeBlock-ownerExecutable()-lineNo());
Works for me.
You can pass callFrame to name() if you want, the result is the same.
What does your JavaScript look like?
E.g., if you're using a function expression
Foo.prototype.bar = function() { ... }
Then that function isn't going to have a name, e.g. your op_call code
will print an empty string if you do f = new Foo(); f.bar();.
You could partially name it by doing
Foo.prototype.bar = function bar() { ... }
For function definitions (e.g. function foo() { ... } in global code),
the function is named accordingly.
Kent
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] MathML in Qt
My patch for MathML support in the Qt port build has just been committed. So far it looks good. There are some issues with the over all MathML rendering code on Linux that I'm looking into (e.g. stack operators have some font issues). These aren't specific to the Qt port. -- --Alex Milowski The excellence of grammar as a guide is proportional to the paucity of the inflexions, i.e. to the degree of analysis effected by the language considered. Bertrand Russell in a footnote of Principles of Mathematics ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] MathML in Qt
Nice! Thanks for letting us know. Regards, ismail On Thu, Apr 29, 2010 at 5:37 PM, Alex Milowski a...@milowski.org wrote: My patch for MathML support in the Qt port build has just been committed. So far it looks good. There are some issues with the over all MathML rendering code on Linux that I'm looking into (e.g. stack operators have some font issues). These aren't specific to the Qt port. -- --Alex Milowski The excellence of grammar as a guide is proportional to the paucity of the inflexions, i.e. to the degree of analysis effected by the language considered. Bertrand Russell in a footnote of Principles of Mathematics ___ 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] Disabling the JIT
chinmaya sn (chins) wrote: Just curious, how would you verify if JavaScript in your browser has JIT support or not? I added this in the interpreter constructor: #if ENABLE(JIT) printf(JIT enabled\n); #else printf(JIT disabled\n); #endif As an update. Building with JAVASCRIPTCORE_JIT=no works if I start from a fresh SVN checkout that hasn't been built without that option before. -- View this message in context: http://old.nabble.com/Disabling-the-JIT-tp28378562p28401613.html Sent from the Webkit mailing list archive at Nabble.com. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Function Property Names
Kent Hansen-2 wrote:
Works for me.
You can pass callFrame to name() if you want, the result is the same.
What does your JavaScript look like?
E.g., if you're using a function expression
Foo.prototype.bar = function() { ... }
Then that function isn't going to have a name, e.g. your op_call code
will print an empty string if you do f = new Foo(); f.bar();.
You could partially name it by doing
Foo.prototype.bar = function bar() { ... }
For function definitions (e.g. function foo() { ... } in global code),
the function is named accordingly.
Ah! I assumed it didn't work because there were alot of empty strings and
strange two letter names. I didn't realize the JavaScript code for
google.com is actually obfuscated.
Now I just need a way to get variable and property names...
By the way, is there some interpreter function somewhere that gets called
when a new page is loaded? I'm assuming a page load causes all the current
code to be unloaded?
--
View this message in context:
http://old.nabble.com/Function---Property-Names-tp28394250p28401921.html
Sent from the Webkit mailing list archive at Nabble.com.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On Thu, Apr 29, 2010 at 9:44 AM, Alexey Proskuryakov a...@webkit.org wrote: On 26.04.2010, at 22:06, Adam Barth wrote: If you make changes to CodeGenerator*.pm, please test your change using the following command: ~/git/webkit$ ./WebKitTools/Scripts/run-bindings-tests I don't understand the utility of such testing. When one edits CodeGenerator*.pm, that's generally to change its output, so run-bindings-tests tests will of course fail. It's great to test end-to-end behavior, and unit tests can also also useful sometimes, but why test that source code stays byte to byte identical? I should say that there's also a class of changes to CodeGenerator*.pm where we don't want to change its output at all. For example, removing some of the ridiculously deeply nested if statements. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On 29.04.2010, at 10:27, Jeremy Orlow wrote: It's great to test end-to-end behavior, and unit tests can also also useful sometimes, but why test that source code stays byte to byte identical? When you make a change to the code generator, you should make a corresponding change to the generated test code. This allows the reviewer to see how the change in CodeGenerator*.pm affects the generated code and documents the change in SVN. The hope is that it'll be obvious if your change is going to have unintended consequences on the generated code. We've found this useful when working on the V8 bindings. For these goals to be achieved, we'd need to check all generated code, not just one custom test file for each language. It would be great to have a tool that generates a diff of derived sources for inspection, but making it into a test for everyone to maintain feels like unnecessary burden. I certainly would feel bad about having to maintain a test that verifies source file content instead of behavior. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On 29.04.2010, at 10:30, Adam Barth wrote: Yeah, without seeing the consequence on the generated code, it can be very difficult to review changes to the code generator. Yes, that's a useful goal. Since an old version of code generating scripts is always in .svn directory (or its git equivalent), it should be possible to generate before and after versions of DerivedSources directory, and diff these. Such a tool would be useful and exciting. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
ext Alexey Proskuryakov wrote: On 29.04.2010, at 10:30, Adam Barth wrote: Yeah, without seeing the consequence on the generated code, it can be very difficult to review changes to the code generator. Yes, that's a useful goal. Since an old version of code generating scripts is always in .svn directory (or its git equivalent), it should be possible to generate before and after versions of DerivedSources directory, and diff these. Such a tool would be useful and exciting. Definitely. The generator for Qt Jambi (Qt bindings for Java, http://qt.gitorious.org/qt-jambi) has a --diff option that produces a colorized diff against the output of the previous run. It's a great, simple way of building confidence that the generator change only had the impact you intended (no guarantees of course, due to platform differences, feature permutations etc.). In my experience such a feature is like turning on the lights (or cranking up the generator, to use a direct analogy). Kent ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On Thu, Apr 29, 2010 at 10:54 AM, Adam Barth aba...@webkit.org wrote: On Thu, Apr 29, 2010 at 10:39 AM, Alexey Proskuryakov a...@webkit.org wrote: On 29.04.2010, at 10:27, Jeremy Orlow wrote: It's great to test end-to-end behavior, and unit tests can also also useful sometimes, but why test that source code stays byte to byte identical? When you make a change to the code generator, you should make a corresponding change to the generated test code. This allows the reviewer to see how the change in CodeGenerator*.pm affects the generated code and documents the change in SVN. The hope is that it'll be obvious if your change is going to have unintended consequences on the generated code. We've found this useful when working on the V8 bindings. For these goals to be achieved, we'd need to check all generated code, not just one custom test file for each language. The idea is that the test IDL file exercises the features of the code generator. Sure, it's not 100% coverage, but it's much better not testing it at all. It would be great to have a tool that generates a diff of derived sources for inspection, but making it into a test for everyone to maintain feels like unnecessary burden. I certainly would feel bad about having to maintain a test that verifies source file content instead of behavior. You should feel free to develop a better testing harness. This one certainly isn't best conceivable tool, but it's better than what we had previously, which was essentially the C++ compiler. The maintenance is super easy. I've been doing a lot of development work on the code generator in the past few days, and it amounts to typing a single command: ./WebKitTools/Scripts/run-bindings-tests --reset-results The harness has been super useful in working on the code generator because the tests run in a few seconds. That lets me iterate on the script much more quickly compared to rebuilding the world every time I want to try a tweak. As an additional datapoint on maintenance, it looks like you've touched CodeGeneratorJS.pm twice in the past year and neither of those change would have resulted in a large diff to the expected test results. By contrast, I've made five changes in the past two days and plan to made dozens more changes shortly to reduce the amount of custom bindings code we have in the tree, which is a serious maintenance problem because it is full of subtle, quirky behavior. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On Apr 29, 2010, at 10:54 AM, Adam Barth wrote: It would be great to have a tool that generates a diff of derived sources for inspection, but making it into a test for everyone to maintain feels like unnecessary burden. I certainly would feel bad about having to maintain a test that verifies source file content instead of behavior. You should feel free to develop a better testing harness. This one certainly isn't best conceivable tool, but it's better than what we had previously, which was essentially the C++ compiler. The maintenance is super easy. I've been doing a lot of development work on the code generator in the past few days, and it amounts to typing a single command: ./WebKitTools/Scripts/run-bindings-tests --reset-results The harness has been super useful in working on the code generator because the tests run in a few seconds. That lets me iterate on the script much more quickly compared to rebuilding the world every time I want to try a tweak. It also strikes me as odd to do testing by doing exact comparison of the generated source. But I can also see side benefits. I think the real issue here may be one of naming. If the use model is that you fully regenerate the results every time you make a change to the bindings generator, then it's not really a regression test. The purpose is not to catch unintentional changes but rather to be able to observe changes to generated code, and new kinds of generated code, while working on a change and when reviewing code. Perhaps the tool should have a name that reflects that, instead of implying that the purpose is to catch bugs accidentally introduced by changes. It doesn't seem like an efficient or effective way to do the latter. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On 29.04.2010, at 10:54, Adam Barth wrote: You should feel free to develop a better testing harness. Since you're proposing run-bindings-tests as a required tool for everyone to use, I don't think that this resolves my concerns. The maintenance is super easy. I've been doing a lot of development work on the code generator in the past few days, and it amounts to typing a single command: ./WebKitTools/Scripts/run-bindings-tests --reset-results So, isn't once supposed to extend the tests when e.g. adding new flags? - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On 29.04.2010, at 11:17, Yaar Schnitman wrote: I've been using the tool for a couple of patches in V8. It really boost the development cycle, helps reviewers understanding what a cryptic perl block of code actually does, and side effects are easy to find. Once you start using it, its becoming hard to work without it. Give it a try! 'm thinking about how this tool could have helped with the CodeGenerator changes I made in the past, and it seems that it wouldn't have detected any changes, and could require me to find creative ways to test the new behavior. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On Thu, Apr 29, 2010 at 11:39 AM, Alexey Proskuryakov a...@webkit.org wrote: On 29.04.2010, at 11:17, Yaar Schnitman wrote: I've been using the tool for a couple of patches in V8. It really boost the development cycle, helps reviewers understanding what a cryptic perl block of code actually does, and side effects are easy to find. Once you start using it, its becoming hard to work without it. Give it a try! 'm thinking about how this tool could have helped with the CodeGenerator changes I made in the past, and it seems that it wouldn't have detected any changes, and could require me to find creative ways to test the new behavior. I don't really follow the what the maintenance overhead is. How does this actually cause you more than a trivial amount of extra work? Maybe a specific example would help. Isn't this just like having a layout test with expected results? It's a small isolated test instead of testing everything. That seems like a good thing. More importantly, it lets you be sure that every feature of the code generator has some testing. In the real IDLs, a feature might stop getting used temporarily and then changes to the code generator would not be readily apparent. Ojan P.S. Sorry for the double-post some of you got. Sent from the wrong email address at first. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] High performance TCP Sockets in WebKit
Hi Everyone, I need to implement a general purpose, high performance TCP sockets inside WebCore for some of my work. By high performance I mean select/poll (multiplexing) based and by general purpose, I mean, any one (Inside WebCore) should be able to open a socket and transfer any kind of data in any format. I would prefer to implement in such a way that it works on every platform that WebKit already support (at network layer). In other words, I would like to (re)use platformSend (alike) interface in SocketStreamHandleBase. But, I reviewed interfaces in platform/network, platform/network/qt/* other, and websockets. Obviously they all have an implicit assumption of HTTP protocol, and WebSocket specification, hence I am beginning to believe I should do a new implementation. I would like hear from the community, if my understanding is correct, are there any suggestions before I proceed, or are there any work already in progress or any implementation suggestions. I would like keep this implementation in discipline with WebKit so that at some point in future, when everything makes sense, I should be able to propose this back to WebKit code base. Thanks chinmaya ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] attach/detach or insertedIntoDocument/removedFromDocument
Noone? Sergiy On Tue, Apr 27, 2010 at 12:59 PM, Sergiy Byelozyorov r...@graphics.cs.uni-sb.de wrote: Hello, I am implementing XML3D http://www.xml3d.org/ format into WebKit. I have a number of new elements that I have added to a separate namespace. Root element is xml3d and it's the only one who has a renderer. Every child that is attached to the subtree of this element should notify parent element about itself and register themselves with this renderer. I need to have a function that I can override to perform this initialization. My implementation uses third party library for rendering and requires me to have a single renderer to render the whole scene, therefore I can't split renderer into many subrenderers for each node. Moreover, 3D scenes do not follow box model and rendering of single node often requires to interact with other elements in the scene (e.g. for ray tracing) thus storing the entire scene in the single data-structure is more effective. To perform the registration with the parent renderer I have tried using insertedIntoDocument/removedFromDocument, but they result in overhead when I am attaching element to a tree which is not rendered at all (such as loaded by Ajax). Then I switched to attach/detach but these are called many times, i.e. every node gets attached, detached, attached again etc. while loading the document, which still causes the overhead. Is there any set of functions that would allow me to know when I am attached to the subtree and that tree should be rendered? Thank you in advance, Sergiy ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
As a concrete example, I found this test setup helpful for this patch: http://trac.webkit.org/changeset/58345. A nice side effect was that it revealed a bug in CodeGeneratorGObject.pm and let me fix it without having to set up build setup for whatever it is that uses the GObject bindings. I agree that golden file testing is a very high-maintenance fragile test method, but it's better than nothing. If this framework didn't exist then I would have likely made the change and relied on spot checking and our existing automated tests to catch any regressions which is less than ideal. - James On Thu, Apr 29, 2010 at 11:44 AM, Ojan Vafai o...@chromium.org wrote: On Thu, Apr 29, 2010 at 11:39 AM, Alexey Proskuryakov a...@webkit.orgwrote: On 29.04.2010, at 11:17, Yaar Schnitman wrote: I've been using the tool for a couple of patches in V8. It really boost the development cycle, helps reviewers understanding what a cryptic perl block of code actually does, and side effects are easy to find. Once you start using it, its becoming hard to work without it. Give it a try! 'm thinking about how this tool could have helped with the CodeGenerator changes I made in the past, and it seems that it wouldn't have detected any changes, and could require me to find creative ways to test the new behavior. I don't really follow the what the maintenance overhead is. How does this actually cause you more than a trivial amount of extra work? Maybe a specific example would help. Isn't this just like having a layout test with expected results? It's a small isolated test instead of testing everything. That seems like a good thing. More importantly, it lets you be sure that every feature of the code generator has some testing. In the real IDLs, a feature might stop getting used temporarily and then changes to the code generator would not be readily apparent. Ojan P.S. Sorry for the double-post some of you got. Sent from the wrong email address at first. ___ 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] Testing changes to CodeGenerator*.pm
Just curious, would it be less maintenance if the test run was integrated with run-webkit-tests?/Is the concern about having lots of different tests harness to run to verify a change? dave On Thu, Apr 29, 2010 at 12:11 PM, James Robinson jam...@google.com wrote: As a concrete example, I found this test setup helpful for this patch: http://trac.webkit.org/changeset/58345. A nice side effect was that it revealed a bug in CodeGeneratorGObject.pm and let me fix it without having to set up build setup for whatever it is that uses the GObject bindings. I agree that golden file testing is a very high-maintenance fragile test method, but it's better than nothing. If this framework didn't exist then I would have likely made the change and relied on spot checking and our existing automated tests to catch any regressions which is less than ideal. - James On Thu, Apr 29, 2010 at 11:44 AM, Ojan Vafai o...@chromium.org wrote: On Thu, Apr 29, 2010 at 11:39 AM, Alexey Proskuryakov a...@webkit.orgwrote: On 29.04.2010, at 11:17, Yaar Schnitman wrote: I've been using the tool for a couple of patches in V8. It really boost the development cycle, helps reviewers understanding what a cryptic perl block of code actually does, and side effects are easy to find. Once you start using it, its becoming hard to work without it. Give it a try! 'm thinking about how this tool could have helped with the CodeGenerator changes I made in the past, and it seems that it wouldn't have detected any changes, and could require me to find creative ways to test the new behavior. I don't really follow the what the maintenance overhead is. How does this actually cause you more than a trivial amount of extra work? Maybe a specific example would help. Isn't this just like having a layout test with expected results? It's a small isolated test instead of testing everything. That seems like a good thing. More importantly, it lets you be sure that every feature of the code generator has some testing. In the real IDLs, a feature might stop getting used temporarily and then changes to the code generator would not be readily apparent. Ojan P.S. Sorry for the double-post some of you got. Sent from the wrong email address at first. ___ 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] Testing changes to CodeGenerator*.pm
IMHO, run-webkit-tests should run all the various webkit testing scripts and we should have a run-layout-tests script that does what run-webkit-tests does today. I'd also settle for a run-tests scripts that was the ASAD testing script. Adam On Thu, Apr 29, 2010 at 12:16 PM, David Levin le...@chromium.org wrote: Just curious, would it be less maintenance if the test run was integrated with run-webkit-tests?/Is the concern about having lots of different tests harness to run to verify a change? dave On Thu, Apr 29, 2010 at 12:11 PM, James Robinson jam...@google.com wrote: As a concrete example, I found this test setup helpful for this patch: http://trac.webkit.org/changeset/58345. A nice side effect was that it revealed a bug in CodeGeneratorGObject.pm and let me fix it without having to set up build setup for whatever it is that uses the GObject bindings. I agree that golden file testing is a very high-maintenance fragile test method, but it's better than nothing. If this framework didn't exist then I would have likely made the change and relied on spot checking and our existing automated tests to catch any regressions which is less than ideal. - James On Thu, Apr 29, 2010 at 11:44 AM, Ojan Vafai o...@chromium.org wrote: On Thu, Apr 29, 2010 at 11:39 AM, Alexey Proskuryakov a...@webkit.org wrote: On 29.04.2010, at 11:17, Yaar Schnitman wrote: I've been using the tool for a couple of patches in V8. It really boost the development cycle, helps reviewers understanding what a cryptic perl block of code actually does, and side effects are easy to find. Once you start using it, its becoming hard to work without it. Give it a try! 'm thinking about how this tool could have helped with the CodeGenerator changes I made in the past, and it seems that it wouldn't have detected any changes, and could require me to find creative ways to test the new behavior. I don't really follow the what the maintenance overhead is. How does this actually cause you more than a trivial amount of extra work? Maybe a specific example would help. Isn't this just like having a layout test with expected results? It's a small isolated test instead of testing everything. That seems like a good thing. More importantly, it lets you be sure that every feature of the code generator has some testing. In the real IDLs, a feature might stop getting used temporarily and then changes to the code generator would not be readily apparent. Ojan P.S. Sorry for the double-post some of you got. Sent from the wrong email address at first. ___ 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] Testing changes to CodeGenerator*.pm
On Apr 29, 2010, at 11:38 AM, Ojan Vafai wrote: I don't really follow the argument against supporting this test. What is the maintenance overhead? Isn't this just like having a layout test with expected results? It's a small isolated test instead of testing everything. That seems like a good thing. More importantly, it lets you be sure that every feature of the code generator has some testing. In the real IDLs, a feature might stop getting used temporarily and then changes to the code generator would not be readily apparent. I don't think your comments above are responsive to my paragraph below: On Thu, Apr 29, 2010 at 11:05 AM, Maciej Stachowiak m...@apple.com wrote: It also strikes me as odd to do testing by doing exact comparison of the generated source. But I can also see side benefits. I think the real issue here may be one of naming. If the use model is that you fully regenerate the results every time you make a change to the bindings generator, then it's not really a regression test. The purpose is not to catch unintentional changes but rather to be able to observe changes to generated code, and new kinds of generated code, while working on a change and when reviewing code. Perhaps the tool should have a name that reflects that, instead of implying that the purpose is to catch bugs accidentally introduced by changes. It doesn't seem like an efficient or effective way to do the latter. To repeat, I think this is a useful tool, but not necessarily as a test. The mode of operation is that when you run this test, if the generated bindings for the text IDL file change in any way, it reports a failure. Then you must manually take the step of regenerating the golden master file. It doesn't seem like the failure report itself will result in a decision. It doesn't provide interesting information until you regenerate the test file and look at the diffs, except in the highly unusual case of changing the output of the codegen script unintentially. Most changes to the codegen script are to intentionally change the output. It seems to me a better model would be to regenerate the bindings test file automatically as part of the build. Then the changes can still be reviewed by you, or as part of a diff, but there would be no extra manual steps involved. And people making behaviorally transparent changes to codegen output would perhaps feel a little less burdened. That makes more sense to me than treating it as a regression test. It also seems like this model would still have all the benefits you cite above. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On Thu, Apr 29, 2010 at 12:43 PM, Maciej Stachowiak m...@apple.com wrote: To repeat, I think this is a useful tool, but not necessarily as a test. The mode of operation is that when you run this test, if the generated bindings for the text IDL file change in any way, it reports a failure. Then you must manually take the step of regenerating the golden master file. It doesn't seem like the failure report itself will result in a decision. It doesn't provide interesting information until you regenerate the test file and look at the diffs, except in the highly unusual case of changing the output of the codegen script unintentially. Most changes to the codegen script are to intentionally change the output. The failure report shows you the diff. In hacking on CodeGeneratorJS, I often run the script many times to see how my changes are affecting the generated code, much in the same way I would run all the LayoutTests constantly if they took 5s to run. It seems to me a better model would be to regenerate the bindings test file automatically as part of the build. Then the changes can still be reviewed by you, or as part of a diff, but there would be no extra manual steps involved. And people making behaviorally transparent changes to codegen output would perhaps feel a little less burdened. That sounds like a good improvement. I think it would be fine to regenerate the output as part of the build. However, I think we should still preserve the ability to run the script along it test mode because that's about three orders of magnitude faster than performing a build after touching CodeGeneratorJS. That makes more sense to me than treating it as a regression test. It also seems like this model would still have all the benefits you cite above. What I hear from this conversation is the following: 1) A bunch of people who've used the tool saying that they've found it useful. 2) A bunch of people who haven't used the tool suggesting improvements. This tool impacts the handful of people who work on CodeGeneratorJS.pm. Everyone else in the project can safely ignore it. I'm all for improvements, and I invite anyone interested to either improve the tool or write a new tool that does the job better. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
I see run-bindings-tests is primary a productivity tool: Faster development, easy debugging, better reviews. But it only works if the golden copies don't fall into disrepair = Hence the need to bake this tool into the general testing framework (run-webkit-tests, pre-submit checks, etc.). It will become a useful (but not perfect) regression tool as a side effect of that. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On Apr 29, 2010, at 1:05 PM, Adam Barth wrote: On Thu, Apr 29, 2010 at 12:43 PM, Maciej Stachowiak m...@apple.com wrote: It seems to me a better model would be to regenerate the bindings test file automatically as part of the build. Then the changes can still be reviewed by you, or as part of a diff, but there would be no extra manual steps involved. And people making behaviorally transparent changes to codegen output would perhaps feel a little less burdened. That sounds like a good improvement. I think it would be fine to regenerate the output as part of the build. However, I think we should still preserve the ability to run the script along it test mode because that's about three orders of magnitude faster than performing a build after touching CodeGeneratorJS. Alexey (or others who don't like the new tests), do you think this change would address your concerns? On Apr 29, 2010, at 1:05 PM, Adam Barth wrote: What I hear from this conversation is the following: 1) A bunch of people who've used the tool saying that they've found it useful. 2) A bunch of people who haven't used the tool suggesting improvements. This tool impacts the handful of people who work on CodeGeneratorJS.pm. Everyone else in the project can safely ignore it. I'm all for improvements, and I invite anyone interested to either improve the tool or write a new tool that does the job better. If everyone has to use the tool for the tool to be useful, then ideally we want a system where the people who change the bindings frequently mostly buy into. Here is the list of people with more than 5 all-time commits in the WebCore/bindings/scripts directory. Ideally I'd like to hear from more of these what they think would be helpful and not burdensome. 59 wei...@apple.com 46 e...@webkit.org 35 da...@apple.com 32 jap...@chromium.org 29 oli...@apple.com 26 gga...@apple.com 26 dglaz...@chromium.org 16 aba...@webkit.org 14 zimmerm...@webkit.org 12 a...@webkit.org 10 aro...@apple.com 8 le...@chromium.org 7 m...@apple.com 7 da...@chromium.org 6 timo...@apple.com 6 s...@chromium.org 6 jia...@chromium.org 6 ddkil...@apple.com 6 cwzwar...@webkit.org Here is the command anyone can run to see the full list (assuming you have an SVN checkout): $ svn log WebCore/bindings/scripts | grep '|.*@' | sed -e 's/^[^|]* |// g; s/ | .*$//g;' | sort | uniq -c | sort -rn The long tail of people who have made only a few bindings changes is rather large, so I suspect this tool affects more than a handful people, if it becomes a mandatory part of the process. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
On 29.04.2010, at 13:38, Maciej Stachowiak wrote: Alexey (or others who don't like the new tests), do you think this change would address your concerns? It would definitely be better if changes weren't detected as failures. Changing a checked out file as part of build seems rather magical though. I'm not sure if everyone agrees that making a diff of actual generated files would be better than looking at a single custom test. That's something I wished for many times, but it calls for a different workflow. And of course, that's not something that is currently implemented. - WBR, Alexey Proskuryakov ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Testing changes to CodeGenerator*.pm
Since I'm in the bindings hall of shame, I guess I'm supposed to reply. ;) The twice that I've used it, it was very helpful. The few reviews I've done of Adam's it was much better than what we had before. However, I agree something better could be built. I'm just not sure what better looks like yet. I expect when Adam finishes getting rid of custom bindings code he'll have a better idea. :) I'm surprised this thread so much attention. -eric On Thu, Apr 29, 2010 at 1:38 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 29, 2010, at 1:05 PM, Adam Barth wrote: On Thu, Apr 29, 2010 at 12:43 PM, Maciej Stachowiak m...@apple.com wrote: It seems to me a better model would be to regenerate the bindings test file automatically as part of the build. Then the changes can still be reviewed by you, or as part of a diff, but there would be no extra manual steps involved. And people making behaviorally transparent changes to codegen output would perhaps feel a little less burdened. That sounds like a good improvement. I think it would be fine to regenerate the output as part of the build. However, I think we should still preserve the ability to run the script along it test mode because that's about three orders of magnitude faster than performing a build after touching CodeGeneratorJS. Alexey (or others who don't like the new tests), do you think this change would address your concerns? On Apr 29, 2010, at 1:05 PM, Adam Barth wrote: What I hear from this conversation is the following: 1) A bunch of people who've used the tool saying that they've found it useful. 2) A bunch of people who haven't used the tool suggesting improvements. This tool impacts the handful of people who work on CodeGeneratorJS.pm. Everyone else in the project can safely ignore it. I'm all for improvements, and I invite anyone interested to either improve the tool or write a new tool that does the job better. If everyone has to use the tool for the tool to be useful, then ideally we want a system where the people who change the bindings frequently mostly buy into. Here is the list of people with more than 5 all-time commits in the WebCore/bindings/scripts directory. Ideally I'd like to hear from more of these what they think would be helpful and not burdensome. 59 wei...@apple.com 46 e...@webkit.org 35 da...@apple.com 32 jap...@chromium.org 29 oli...@apple.com 26 gga...@apple.com 26 dglaz...@chromium.org 16 aba...@webkit.org 14 zimmerm...@webkit.org 12 �...@webkit.org 10 aro...@apple.com 8 le...@chromium.org 7 ...@apple.com 7 da...@chromium.org 6 timo...@apple.com 6 s...@chromium.org 6 jia...@chromium.org 6 ddkil...@apple.com 6 cwzwar...@webkit.org Here is the command anyone can run to see the full list (assuming you have an SVN checkout): $ svn log WebCore/bindings/scripts | grep '|.*@' | sed -e 's/^[^|]* |//g; s/ | .*$//g;' | sort | uniq -c | sort -rn The long tail of people who have made only a few bindings changes is rather large, so I suspect this tool affects more than a handful people, if it becomes a mandatory part of the process. 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] Testing changes to CodeGenerator*.pm
On Thu, Apr 29, 2010 at 9:05 PM, Adam Barth aba...@webkit.org wrote: On Thu, Apr 29, 2010 at 12:43 PM, Maciej Stachowiak m...@apple.com wrote: It seems to me a better model would be to regenerate the bindings test file automatically as part of the build. Then the changes can still be reviewed by you, or as part of a diff, but there would be no extra manual steps involved. And people making behaviorally transparent changes to codegen output would perhaps feel a little less burdened. That sounds like a good improvement. I think it would be fine to regenerate the output as part of the build. However, I think we should still preserve the ability to run the script along it test mode because that's about three orders of magnitude faster than performing a build after touching CodeGeneratorJS. From reading this thread, this seems like the best solution and most easy to accomplish. We already have the standalone script, so all we need to do is hack it into the build. Isn't there some makefile that generates the derived sources for at least most of the platforms? Couldn't we just add it to that? People who don't care or value it could continue with their current work-flows unaffected. And those that it helps can continue with their workflows as well, as far as I can tell. And, optionally, if someone would like to create more robust testing frameworks in the future could. Am I totally missing something? I feel like I must since this discussion has continued on for a while beyond this point. J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] ENABLE(INSPECTOR) check in Console::lastWMLErrorMessage
On Apr 29, 2010, at 12:53 AM, Joseph Pecoraro wrote: Looks like it should add ENABLE(INSPECTOR) check in Console::lastWMLErrorMessage() (/trunk/WebCore/page/Console.cpp) Thanks. Ryan, I've created a bug and put up a few patches (2 desired behaviors): https://bugs.webkit.org/show_bug.cgi?id=38366 - Joe___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
