From b2c387f1f438f43270c33d6fe9f691b5b51db3f4 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 12 Jul 2008 01:24:51 +0300 Subject: [PATCH 01/32] Bug 1029: Use JS_CallFunctionValue in load_uri.c JS_CallFunction does not support closures in SpiderMonkey versions earlier than 1.8. Test case: elinks.keymaps.main["!"] = function() { elinks.load_uri("http://www.eldar.org/cgi-bin/fortune.pl?text_format=yes", function (cached) { elinks.alert(cached.content); }); } --- src/scripting/smjs/load_uri.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/scripting/smjs/load_uri.c b/src/scripting/smjs/load_uri.c index f9c7e09f..576cf1a1 100644 --- a/src/scripting/smjs/load_uri.c +++ b/src/scripting/smjs/load_uri.c @@ -17,7 +17,11 @@ struct smjs_load_uri_hop { struct session *ses; - JSFunction *callback; + + /* SpiderMonkey versions earlier than 1.8 cannot properly call + * a closure if given just a JSFunction pointer. They need a + * jsval that points to the corresponding JSObject. */ + jsval callback; }; static void @@ -39,16 +43,13 @@ smjs_loading_callback(struct download *download, void *data) * the script is using it. */ object_lock(download->cached); - assert(hop->callback); - smjs_ses = hop->ses; cache_entry_object = smjs_get_cache_entry_object(download->cached); if (!cache_entry_object) goto end; args[0] = OBJECT_TO_JSVAL(cache_entry_object); - - JS_CallFunction(smjs_ctx, NULL, hop->callback, 1, args, &rval); + JS_CallFunctionValue(smjs_ctx, NULL, hop->callback, 1, args, &rval); end: if (download->cached) @@ -85,12 +86,12 @@ smjs_load_uri(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, hop = mem_alloc(sizeof(*hop)); if (!hop) { - done_uri(uri); mem_free(download); + done_uri(uri); return JS_FALSE; } - hop->callback = JS_ValueToFunction(smjs_ctx, argv[1]); + hop->callback = argv[1]; hop->ses = smjs_ses; download->data = hop; From 079b97d21b05fefc5d284ebbc78c34c8bdf5ca2f Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 12 Jul 2008 01:37:51 +0300 Subject: [PATCH 02/32] Bug 1026: Protect callback of elinks.load_uri from GC --- NEWS | 3 +++ src/scripting/smjs/load_uri.c | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index f117d602..8c699fc5 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,9 @@ generally also includes the bug fixes made in ELinks 0.11.4.GIT. Bugs that should be removed from NEWS before the 0.12.0 release: +* major bug 1026 in user SMJS: Protect the callback of elinks.load_uri + from the garbage collector. The elinks.load_uri method was added in + ELinks 0.12pre1. * bug 955: Reset buttons no longer run FORM/@onsubmit, and ``harmless'' buttons no longer submit the form. ELinks 0.12pre1 was the first release that had these bugs. diff --git a/src/scripting/smjs/load_uri.c b/src/scripting/smjs/load_uri.c index 576cf1a1..b8adcc2c 100644 --- a/src/scripting/smjs/load_uri.c +++ b/src/scripting/smjs/load_uri.c @@ -20,7 +20,9 @@ struct smjs_load_uri_hop { /* SpiderMonkey versions earlier than 1.8 cannot properly call * a closure if given just a JSFunction pointer. They need a - * jsval that points to the corresponding JSObject. */ + * jsval that points to the corresponding JSObject. Besides, + * JS_AddNamedRoot is not documented to support JSFunction + * pointers. */ jsval callback; }; @@ -54,6 +56,7 @@ smjs_loading_callback(struct download *download, void *data) end: if (download->cached) object_unlock(download->cached); + JS_RemoveRoot(smjs_ctx, &hop->callback); mem_free(download->data); mem_free(download); @@ -93,6 +96,13 @@ smjs_load_uri(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, hop->callback = argv[1]; hop->ses = smjs_ses; + if (!JS_AddNamedRoot(smjs_ctx, &hop->callback, + "smjs_load_uri_hop.callback")) { + mem_free(hop); + mem_free(download); + done_uri(uri); + return JS_FALSE; + } download->data = hop; download->callback = (download_callback_T *) smjs_loading_callback; From e3830cfd676af9e60f4a860c2a858abdb40b0af3 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 12 Jul 2008 01:49:08 +0300 Subject: [PATCH 03/32] Bug 1029: Use JS_CallFunctionValue in elinks_object.c JS_CallFunction does not support closures in SpiderMonkey versions earlier than 1.8. Test case: function set_suffix(suffix) { elinks.preformat_html = function(cached, vs) { cached.content += suffix; } } set_suffix("hello"); --- src/scripting/smjs/elinks_object.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/scripting/smjs/elinks_object.c b/src/scripting/smjs/elinks_object.c index 0a78fac6..09455db6 100644 --- a/src/scripting/smjs/elinks_object.c +++ b/src/scripting/smjs/elinks_object.c @@ -176,8 +176,6 @@ JSBool smjs_invoke_elinks_object_method(unsigned char *method, jsval argv[], int argc, jsval *rval) { - JSFunction *func; - assert(smjs_ctx); assert(smjs_elinks_object); assert(rval); @@ -190,9 +188,6 @@ smjs_invoke_elinks_object_method(unsigned char *method, jsval argv[], int argc, if (JSVAL_VOID == *rval) return JS_FALSE; - func = JS_ValueToFunction(smjs_ctx, *rval); - if (!func) return JS_FALSE; - - return JS_CallFunction(smjs_ctx, smjs_elinks_object, - func, argc, argv, rval); + return JS_CallFunctionValue(smjs_ctx, smjs_elinks_object, + *rval, argc, argv, rval); } From c5a012eca5a06a660f3a66b62da9e1a4fb40ac0d Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 12 Jul 2008 01:54:49 +0300 Subject: [PATCH 04/32] Bug 1029: Use JS_CallFunctionValue in keybinding.c JS_CallFunction does not support closures in SpiderMonkey versions earlier than 1.8. Test case: elinks.keymaps.main["\""] = function() { elinks.keymaps.main["e"] = function() { elinks.alert("hello!"); }; } --- src/scripting/smjs/keybinding.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/scripting/smjs/keybinding.c b/src/scripting/smjs/keybinding.c index 8c2f697b..61bffb27 100644 --- a/src/scripting/smjs/keybinding.c +++ b/src/scripting/smjs/keybinding.c @@ -55,15 +55,13 @@ smjs_keybinding_action_callback(va_list ap, void *data) jsval rval; struct session *ses = va_arg(ap, struct session *); JSObject *jsobj = data; - JSFunction *func = JS_ValueToFunction(smjs_ctx, OBJECT_TO_JSVAL(jsobj)); evhook_use_params(ses); - assert(func); - smjs_ses = ses; - JS_CallFunction(smjs_ctx, NULL, func, 0, NULL, &rval); + JS_CallFunctionValue(smjs_ctx, NULL, OBJECT_TO_JSVAL(jsobj), + 0, NULL, &rval); smjs_ses = NULL; From ddd36cafb430ba551b533f829fdb16befc9f2dfd Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 12 Jul 2008 01:59:18 +0300 Subject: [PATCH 05/32] Bug 1029: List in NEWS. --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 8c699fc5..05659126 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,8 @@ ELinks 0.12pre1.GIT now: To be released as 0.12pre2, 0.12rc1, or even 0.12.0. This branch generally also includes the bug fixes made in ELinks 0.11.4.GIT. +* critical bug 1029 in user SMJS: Prefer JS_CallFunctionValue over + JS_CallFunction, which can crash if given a closure. * minor bug 951: SpiderMonkey scripting objects used to prevent ELinks from removing files from the memory cache From 46e76e0688542c296c50952d9240fb1c2167fb3e Mon Sep 17 00:00:00 2001 From: Jonas Fonseca Date: Sun, 13 Jul 2008 15:25:41 +0200 Subject: [PATCH 06/32] Minor documentation improvements --- doc/ecmascript.txt | 2 +- doc/small.txt | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/doc/ecmascript.txt b/doc/ecmascript.txt index 73f5ba00..7070abac 100644 --- a/doc/ecmascript.txt +++ b/doc/ecmascript.txt @@ -131,7 +131,7 @@ yet if it does not work in the Mozilla browsers neither ;-). Now, I would still like NJS or a new JS engine from scratch... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -...and you don't fear some coding? That's fine then! ELinks is in no way tied +\...and you don't fear some coding? That's fine then! ELinks is in no way tied to SpiderMonkey, in fact the ECMAScript support was carefully implemented so that there are no SpiderMonkey references outside of `src/ecmascript/spidermonkey.*`. If you want to implement an alternative diff --git a/doc/small.txt b/doc/small.txt index 6ee41705..9d526e6f 100644 --- a/doc/small.txt +++ b/doc/small.txt @@ -20,8 +20,7 @@ Note that if you don't use dietlibc, you definitively want to add `-Os` or `-O2` to `CFLAGS`; GCC 2.95 does not know `-Os`, and some say `-O2` gives smaller executables even for GCC 3.x. -[NOTE] -.Warning +[TIP] =============================================================================== If you use these `CFLAGS` on Cygwin and you get unresolved symbols (`htons` and suite in particular), try removing `-fno-inline` parameter. @@ -38,8 +37,8 @@ $ ./configure --disable-ipv6 --disable-backtrace --disable-nls \ You can disable bookmarks, globhist and more, too, if you want to. -[NOTE] -.Notes +[TIP] +.Other configure options that can reduce the size =============================================================================== - \--disable-backtrace disables internal backtrace code. - \--disable-nls disables i18n support. From e83f76b79e431d24256260acfbf2aaf935255189 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sat, 12 Jul 2008 18:36:24 +0200 Subject: [PATCH 07/32] 1030: Fixed issue with undefined HAVE_REGEX_H. (cherry picked from commit 442b0d83b021217539d5a00409f97ade2f248baa) --- src/viewer/text/search.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/viewer/text/search.c b/src/viewer/text/search.c index 9d034d23..cde45828 100644 --- a/src/viewer/text/search.c +++ b/src/viewer/text/search.c @@ -1576,14 +1576,17 @@ search_typeahead(struct session *ses, struct document_view *doc_view, * a nice cleanup target ;-). --pasky */ enum search_option { +#ifdef HAVE_REGEX_H SEARCH_OPT_REGEX, +#endif SEARCH_OPT_CASE, - SEARCH_OPTIONS, }; static struct option_resolver resolvers[] = { +#ifdef HAVE_REGEX_H { SEARCH_OPT_REGEX, "regex" }, +#endif { SEARCH_OPT_CASE, "case" }, }; @@ -1648,7 +1651,11 @@ search_dlg_do(struct terminal *term, struct memory_list *ml, hop->values, SEARCH_OPTIONS); hop->data = data; +#ifdef HAVE_REGEX_H #define SEARCH_WIDGETS_COUNT 8 +#else +#define SEARCH_WIDGETS_COUNT 5 +#endif dlg = calloc_dialog(SEARCH_WIDGETS_COUNT, MAX_STR_LEN); if (!dlg) { mem_free(hop); @@ -1668,9 +1675,11 @@ search_dlg_do(struct terminal *term, struct memory_list *ml, field = get_dialog_offset(dlg, SEARCH_WIDGETS_COUNT); add_dlg_field(dlg, text, 0, 0, NULL, MAX_STR_LEN, field, history); +#ifdef HAVE_REGEX_H add_dlg_radio(dlg, _("Normal search", term), 1, 0, &hop->values[SEARCH_OPT_REGEX].number); add_dlg_radio(dlg, _("Regexp search", term), 1, 1, &hop->values[SEARCH_OPT_REGEX].number); add_dlg_radio(dlg, _("Extended regexp search", term), 1, 2, &hop->values[SEARCH_OPT_REGEX].number); +#endif add_dlg_radio(dlg, _("Case sensitive", term), 2, 1, &hop->values[SEARCH_OPT_CASE].number); add_dlg_radio(dlg, _("Case insensitive", term), 2, 0, &hop->values[SEARCH_OPT_CASE].number); From e287ca9265fdcdf32409446d02a95579c0da8c1e Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Mon, 14 Jul 2008 22:26:38 +0300 Subject: [PATCH 08/32] 1030: Wrap get_search_region_from_search_nodes in #ifdef HAVE_REGEX_H This change avoids the following error: gcc -DHAVE_CONFIG_H -I../../.. -I/home/Kalle/src/elinks-0.11/src -I/home/Kalle/prefix/include -I/usr/include/smjs -I/usr/include -I/usr/include/lua50 -I/usr/include -I/usr/include -O0 -ggdb -Wall -Wall -Werror -fno-strict-aliasing -Wno-pointer-sign -Wno-address -fno-strict-overflow -o search.o -c /home/Kalle/src/elinks-0.11/src/viewer/text/search.c cc1: warnings being treated as errors /home/Kalle/src/elinks-0.11/src/viewer/text/search.c:257: warning: 'get_search_region_from_search_nodes' defined but not used make[3]: *** [search.o] Error 1 make[3]: Leaving directory `/home/Kalle/build/i686-pc-linux-gnu/elinks-0.11/src/viewer/text' get_search_region_from_search_nodes is called only from search_for_pattern, which already was inside #ifdef HAVE_REGEX_H. (cherry picked from commit 2aec302d4741ac27ce8e37de573030ca8bba727b) --- src/viewer/text/search.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viewer/text/search.c b/src/viewer/text/search.c index cde45828..4c0201e0 100644 --- a/src/viewer/text/search.c +++ b/src/viewer/text/search.c @@ -262,6 +262,7 @@ get_range(struct document *document, int y, int height, int l, return 0; } +#ifdef HAVE_REGEX_H /** Returns a string @c doc that is a copy of the text in the search * nodes from @a s1 to (@a s1 + @a doclen - 1) with the space at the * end of each line converted to a new-line character (LF). */ @@ -293,7 +294,6 @@ get_search_region_from_search_nodes(struct search *s1, struct search *s2, return doc; } -#ifdef HAVE_REGEX_H struct regex_match_context { struct search *s1; struct search *s2; From 6b05cdb3a0a12e8cf8bae3860b1a59e86d3076a1 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Mon, 14 Jul 2008 22:34:43 +0300 Subject: [PATCH 09/32] 1030: List the bug in NEWS. (cherry picked from commit 295c7760f7d31263caad5d111440157325fdea48) --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 05659126..b01a90a7 100644 --- a/NEWS +++ b/NEWS @@ -189,6 +189,8 @@ To be released as 0.11.5. * critical bug 1027 in user SMJS: make elinks.keymaps treat null and "none" as equivalent actions, avoiding a segfault +* critical bug 1030: an assertion used to fail in the search dialog + on systems that lack a usable * major bug 503: various fixes in parsing and updating of elinks.conf * build bug 1021: fixed uninitialized variable in http_got_header From e9d4d3aef23b6871d73e880221e0612f07503f62 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Tue, 15 Jul 2008 00:09:27 +0300 Subject: [PATCH 10/32] Fix crash after a tab was opened during reload. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 0b99fa70ca9d0f976655e61adee1a5eebacc0734 "Bug 620: Reset form fields to default values on reload" made render_document() decrement vs->form_info_len to 0 while vs->form_info remained non-NULL. copy_vs() then copied the whole structure with copy_struct and did not change form_info because form_info_len was 0. Both view_state structures had form_info pointing to the same memory block, causing a segfault when destroy_vs() tried to free that block a second time. Reported by أحمد المحمودي. --- NEWS | 2 ++ src/viewer/text/vs.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/NEWS b/NEWS index b01a90a7..7108a8ef 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ generally also includes the bug fixes made in ELinks 0.11.4.GIT. Bugs that should be removed from NEWS before the 0.12.0 release: +* critical: Fix crash after a tab was opened during reload. This was + triggered by the bug 620 fix in ELinks 0.12pre1. * major bug 1026 in user SMJS: Protect the callback of elinks.load_uri from the garbage collector. The elinks.load_uri method was added in ELinks 0.12pre1. diff --git a/src/viewer/text/vs.c b/src/viewer/text/vs.c index d0bbdf52..a7978dbe 100644 --- a/src/viewer/text/vs.c +++ b/src/viewer/text/vs.c @@ -79,6 +79,12 @@ copy_vs(struct view_state *dst, struct view_state *src) dst->ecmascript_fragile = 1; #endif + /* destroy_vs(vs) does mem_free_if(vs->form_info), so each + * view_state must have its own form_info. Normally we make a + * copy below, but not if src->form_info_len is 0, which it + * can be even if src->form_info is not NULL. */ + dst->form_info = NULL; + /* Clean as a baby. */ dst->doc_view = NULL; From 37ce1ca65896e12d53434def836cbbaeddf4ab16 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sat, 12 Jul 2008 18:04:40 +0200 Subject: [PATCH 11/32] Here is a patch, which adds support for tcc's alloca. --- configure.in | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/configure.in b/configure.in index c4bc8198..880c1680 100644 --- a/configure.in +++ b/configure.in @@ -1402,6 +1402,16 @@ AC_SUBST(LIBDIR) EL_LOG_CONFIG(CONFDIR, [System configuration directory], []) EL_LOG_CONFIG(LOCALEDIR, [Locale catalogs directory], []) + +AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include +]], [[#if !defined(__TINYC__) || !defined(alloca) + #error not tcc + #endif ]])],[cf_result=yes],[cf_result=no]) + +if test "$cf_result" = "yes"; then + AC_DEFINE([HAVE_ALLOCA]) + AC_DEFINE([_ALLOCA_H], [1], [alloca of tcc]) +fi # =================================================================== # A little fine tuning of gcc specific options (continued) # =================================================================== From f479d6e82a0837244e80b86cb24436f0578e6643 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Tue, 15 Jul 2008 22:53:26 +0300 Subject: [PATCH 12/32] Explain in config.h.in what a hack _ALLOCA_H is --- configure.in | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/configure.in b/configure.in index 880c1680..0572e7d7 100644 --- a/configure.in +++ b/configure.in @@ -1410,7 +1410,16 @@ AC_LINK_IFELSE([AC_LANG_PROGRAM([[#include if test "$cf_result" = "yes"; then AC_DEFINE([HAVE_ALLOCA]) - AC_DEFINE([_ALLOCA_H], [1], [alloca of tcc]) + AC_DEFINE([_ALLOCA_H], [1], [Define as 1 if you are using Tiny C Compiler + with the GNU C Library, and of glibc would otherwise + override the alloca macro defined in of TCC. + If of glibc sees the _ALLOCA_H macro, it assumes + it has already been included, and does not redefine alloca. + This might not work in future glibc versions though, because + the names of the #include guard macros are not documented. + The incompatibility has been reported to the tinycc-devel + mailing list on 2008-07-14. If a future version of TCC provides + an of its own, this hack won't be needed.]) fi # =================================================================== # A little fine tuning of gcc specific options (continued) From 32889bf90887d5117a60e53890a5eeaa84af33a6 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Wed, 16 Jul 2008 12:32:24 +0300 Subject: [PATCH 13/32] 1031: Add spidermonkey-shared.c used for both web and user scripts Rename src/ecmascript/spidermonkey/util.c to src/ecmascript/spidermonkey-shared.c and compile it also when CONFIG_SCRIPTING_SMJS is enabled but CONFIG_ECMASCRIPT_SPIDERMONKEY is not. Then use its functions from src/scripting/smjs/ too. Move the corresponding declarations, as well as the inline functions needed by src/scripting/smjs/, from src/ecmascript/spidermonkey/util.h to src/ecmascript/spidermonkey-shared.h. ELinks is nowadays using two JSRuntimes and SpiderMonkey has bugs that make it crash in such use. To work around them, ELinks will need to be changed to use only one JSRuntime. I am planning to define and initialize that JSRuntime in src/ecmascript/spidermonkey-shared.c, now that it's compiled whenever either of the modules is enabled. --- src/ecmascript/Makefile | 10 +++ .../util.c => spidermonkey-shared.c} | 5 +- src/ecmascript/spidermonkey-shared.h | 75 +++++++++++++++++++ src/ecmascript/spidermonkey/Makefile | 2 +- src/ecmascript/spidermonkey/util.h | 61 +-------------- src/scripting/smjs/action_object.c | 2 +- src/scripting/smjs/bookmarks.c | 2 +- src/scripting/smjs/bookmarks.h | 2 +- src/scripting/smjs/cache_object.c | 2 +- src/scripting/smjs/core.c | 2 +- src/scripting/smjs/core.h | 2 +- src/scripting/smjs/elinks_object.c | 33 +++----- src/scripting/smjs/elinks_object.h | 2 +- src/scripting/smjs/global_object.c | 2 +- src/scripting/smjs/global_object.h | 2 +- src/scripting/smjs/globhist.c | 2 +- src/scripting/smjs/hooks.c | 2 +- src/scripting/smjs/keybinding.c | 2 +- src/scripting/smjs/keybinding.h | 2 +- src/scripting/smjs/load_uri.c | 2 +- src/scripting/smjs/view_state_object.c | 2 +- 21 files changed, 117 insertions(+), 99 deletions(-) rename src/ecmascript/{spidermonkey/util.c => spidermonkey-shared.c} (91%) create mode 100644 src/ecmascript/spidermonkey-shared.h diff --git a/src/ecmascript/Makefile b/src/ecmascript/Makefile index c5ca6ea0..ab936a02 100644 --- a/src/ecmascript/Makefile +++ b/src/ecmascript/Makefile @@ -8,6 +8,16 @@ SUBDIRS-$(CONFIG_ECMASCRIPT_SMJS) += spidermonkey OBJS-$(CONFIG_ECMASCRIPT_SEE) += see.o OBJS-$(CONFIG_ECMASCRIPT_SMJS) += spidermonkey.o +ifeq ($(CONFIG_ECMASCRIPT_SMJS), yes) +CONFIG_ANY_SPIDERMONKEY = yes +else ifeq ($(CONFIG_SCRIPTING_SPIDERMONKEY), yes) +CONFIG_ANY_SPIDERMONKEY = yes +else +CONFIG_ANY_SPIDERMONKEY = no +endif + +OBJS-$(CONFIG_ANY_SPIDERMONKEY) += spidermonkey-shared.o + OBJS = ecmascript.o include $(top_srcdir)/Makefile.lib diff --git a/src/ecmascript/spidermonkey/util.c b/src/ecmascript/spidermonkey-shared.c similarity index 91% rename from src/ecmascript/spidermonkey/util.c rename to src/ecmascript/spidermonkey-shared.c index ccddbcd3..769a35b3 100644 --- a/src/ecmascript/spidermonkey/util.c +++ b/src/ecmascript/spidermonkey-shared.c @@ -1,4 +1,5 @@ -/* Better compatibility across versions of SpiderMonkey. */ +/** SpiderMonkey support for both user scripts and web scripts. + * @file */ #ifdef HAVE_CONFIG_H #include "config.h" @@ -6,7 +7,7 @@ #include "elinks.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" /** An ELinks-specific replacement for JS_DefineFunctions(). * diff --git a/src/ecmascript/spidermonkey-shared.h b/src/ecmascript/spidermonkey-shared.h new file mode 100644 index 00000000..3b2602b6 --- /dev/null +++ b/src/ecmascript/spidermonkey-shared.h @@ -0,0 +1,75 @@ +#ifndef EL__ECMASCRIPT_SPIDERMONKEY_SHARED_H +#define EL__ECMASCRIPT_SPIDERMONKEY_SHARED_H + +/* Inconsistently applied conventions for prefixes in identifiers: + * - "smjs" for user scripts (scripting/smjs/). + * - "js" for web scripts (ecmascript/spidermonkey/). + * - "spidermonkey" for common stuff, especially any that replace + * similarly named things defined in SpiderMonkey itself. */ + +/* For wild SpiderMonkey installations. */ +#ifdef CONFIG_OS_BEOS +#define XP_BEOS +#elif CONFIG_OS_OS2 +#define XP_OS2 +#elif CONFIG_OS_RISCOS +#error Out of luck, buddy! +#elif CONFIG_OS_UNIX +#define XP_UNIX +#elif CONFIG_OS_WIN32 +#define XP_WIN +#endif + +#include + +#include "util/string.h" + +/** An ELinks-specific replacement for JSFunctionSpec. + * + * Bug 1016: In SpiderMonkey 1.7 bundled with XULRunner 1.8, jsapi.h + * defines JSFunctionSpec in different ways depending on whether + * MOZILLA_1_8_BRANCH is defined, and there is no obvious way for + * ELinks to check whether MOZILLA_1_8_BRANCH was defined when the + * library was built. Avoid the unstable JSFunctionSpec definitions + * and use this ELinks-specific structure instead. */ +typedef struct spidermonkeyFunctionSpec { + const char *name; + JSNative call; + uint8 nargs; + /* ELinks does not use "flags" and "extra" so omit them here. */ +} spidermonkeyFunctionSpec; + +JSBool spidermonkey_DefineFunctions(JSContext *cx, JSObject *obj, + const spidermonkeyFunctionSpec *fs); +JSObject *spidermonkey_InitClass(JSContext *cx, JSObject *obj, + JSObject *parent_proto, JSClass *clasp, + JSNative constructor, uintN nargs, + JSPropertySpec *ps, + const spidermonkeyFunctionSpec *fs, + JSPropertySpec *static_ps, + const spidermonkeyFunctionSpec *static_fs); + +static void undef_to_jsval(JSContext *ctx, jsval *vp); +static unsigned char *jsval_to_string(JSContext *ctx, jsval *vp); + +/* Inline functions */ + +static inline void +undef_to_jsval(JSContext *ctx, jsval *vp) +{ + *vp = JSVAL_NULL; +} + +static inline unsigned char * +jsval_to_string(JSContext *ctx, jsval *vp) +{ + jsval val; + + if (JS_ConvertValue(ctx, *vp, JSTYPE_STRING, &val) == JS_FALSE) { + return ""; + } + + return empty_string_or_(JS_GetStringBytes(JS_ValueToString(ctx, val))); +} + +#endif diff --git a/src/ecmascript/spidermonkey/Makefile b/src/ecmascript/spidermonkey/Makefile index 4f07d9bf..f1c0fef3 100644 --- a/src/ecmascript/spidermonkey/Makefile +++ b/src/ecmascript/spidermonkey/Makefile @@ -2,6 +2,6 @@ top_builddir=../../.. include $(top_builddir)/Makefile.config INCLUDES += $(SPIDERMONKEY_CFLAGS) -OBJS = document.o form.o location.o navigator.o unibar.o util.o window.o +OBJS = document.o form.o location.o navigator.o unibar.o window.o include $(top_srcdir)/Makefile.lib diff --git a/src/ecmascript/spidermonkey/util.h b/src/ecmascript/spidermonkey/util.h index a9f955eb..84da9a6c 100644 --- a/src/ecmascript/spidermonkey/util.h +++ b/src/ecmascript/spidermonkey/util.h @@ -2,32 +2,16 @@ #ifndef EL__ECMASCRIPT_SPIDERMONKEY_UTIL_H #define EL__ECMASCRIPT_SPIDERMONKEY_UTIL_H -/* For wild SpiderMonkey installations. */ -#ifdef CONFIG_OS_BEOS -#define XP_BEOS -#elif CONFIG_OS_OS2 -#define XP_OS2 -#elif CONFIG_OS_RISCOS -#error Out of luck, buddy! -#elif CONFIG_OS_UNIX -#define XP_UNIX -#elif CONFIG_OS_WIN32 -#define XP_WIN -#endif - -#include +#include "ecmascript/spidermonkey-shared.h" #include "util/memory.h" -#include "util/string.h" static void string_to_jsval(JSContext *ctx, jsval *vp, unsigned char *string); static void astring_to_jsval(JSContext *ctx, jsval *vp, unsigned char *string); static void int_to_jsval(JSContext *ctx, jsval *vp, int number); static void object_to_jsval(JSContext *ctx, jsval *vp, JSObject *object); static void boolean_to_jsval(JSContext *ctx, jsval *vp, int boolean); -static void undef_to_jsval(JSContext *ctx, jsval *vp); static int jsval_to_boolean(JSContext *ctx, jsval *vp); -static unsigned char *jsval_to_string(JSContext *ctx, jsval *vp); @@ -68,12 +52,6 @@ boolean_to_jsval(JSContext *ctx, jsval *vp, int boolean) *vp = BOOLEAN_TO_JSVAL(boolean); } -static inline void -undef_to_jsval(JSContext *ctx, jsval *vp) -{ - *vp = JSVAL_NULL; -} - static inline int jsval_to_boolean(JSContext *ctx, jsval *vp) @@ -87,41 +65,4 @@ jsval_to_boolean(JSContext *ctx, jsval *vp) return JSVAL_TO_BOOLEAN(val); } -static inline unsigned char * -jsval_to_string(JSContext *ctx, jsval *vp) -{ - jsval val; - - if (JS_ConvertValue(ctx, *vp, JSTYPE_STRING, &val) == JS_FALSE) { - return ""; - } - - return empty_string_or_(JS_GetStringBytes(JS_ValueToString(ctx, val))); -} - -/** An ELinks-specific replacement for JSFunctionSpec. - * - * Bug 1016: In SpiderMonkey 1.7 bundled with XULRunner 1.8, jsapi.h - * defines JSFunctionSpec in different ways depending on whether - * MOZILLA_1_8_BRANCH is defined, and there is no obvious way for - * ELinks to check whether MOZILLA_1_8_BRANCH was defined when the - * library was built. Avoid the unstable JSFunctionSpec definitions - * and use this ELinks-specific structure instead. */ -typedef struct spidermonkeyFunctionSpec { - const char *name; - JSNative call; - uint8 nargs; - /* ELinks does not use "flags" and "extra" so omit them here. */ -} spidermonkeyFunctionSpec; - -JSBool spidermonkey_DefineFunctions(JSContext *cx, JSObject *obj, - const spidermonkeyFunctionSpec *fs); -JSObject *spidermonkey_InitClass(JSContext *cx, JSObject *obj, - JSObject *parent_proto, JSClass *clasp, - JSNative constructor, uintN nargs, - JSPropertySpec *ps, - const spidermonkeyFunctionSpec *fs, - JSPropertySpec *static_ps, - const spidermonkeyFunctionSpec *static_fs); - #endif diff --git a/src/scripting/smjs/action_object.c b/src/scripting/smjs/action_object.c index b3c38dce..d95564d2 100644 --- a/src/scripting/smjs/action_object.c +++ b/src/scripting/smjs/action_object.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "config/kbdbind.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "scripting/smjs/core.h" #include "scripting/smjs/elinks_object.h" #include "session/session.h" diff --git a/src/scripting/smjs/bookmarks.c b/src/scripting/smjs/bookmarks.c index f591bd8f..fd6610d2 100644 --- a/src/scripting/smjs/bookmarks.c +++ b/src/scripting/smjs/bookmarks.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "bookmarks/bookmarks.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "main/event.h" #include "scripting/smjs/core.h" #include "scripting/smjs/elinks_object.h" diff --git a/src/scripting/smjs/bookmarks.h b/src/scripting/smjs/bookmarks.h index bdf93852..16f1ec4d 100644 --- a/src/scripting/smjs/bookmarks.h +++ b/src/scripting/smjs/bookmarks.h @@ -1,7 +1,7 @@ #ifndef EL__SCRIPTING_SMJS_BOOKMARKS_H #define EL__SCRIPTING_SMJS_BOOKMARKS_H -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" void smjs_init_bookmarks_interface(void); diff --git a/src/scripting/smjs/cache_object.c b/src/scripting/smjs/cache_object.c index 59708fe8..cd23842c 100644 --- a/src/scripting/smjs/cache_object.c +++ b/src/scripting/smjs/cache_object.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "cache/cache.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "protocol/uri.h" #include "scripting/smjs/cache_object.h" #include "scripting/smjs/core.h" diff --git a/src/scripting/smjs/core.c b/src/scripting/smjs/core.c index 9d51a915..60fe7ba0 100644 --- a/src/scripting/smjs/core.c +++ b/src/scripting/smjs/core.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "config/home.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "main/module.h" #include "osdep/osdep.h" #include "scripting/scripting.h" diff --git a/src/scripting/smjs/core.h b/src/scripting/smjs/core.h index dcc8a74b..0542f5cb 100644 --- a/src/scripting/smjs/core.h +++ b/src/scripting/smjs/core.h @@ -1,7 +1,7 @@ #ifndef EL__SCRIPTING_SMJS_CORE_H #define EL__SCRIPTING_SMJS_CORE_H -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" struct module; struct session; diff --git a/src/scripting/smjs/elinks_object.c b/src/scripting/smjs/elinks_object.c index 09455db6..727bfda6 100644 --- a/src/scripting/smjs/elinks_object.c +++ b/src/scripting/smjs/elinks_object.c @@ -8,7 +8,7 @@ #include "bfu/msgbox.h" #include "config/home.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "intl/gettext/libintl.h" #include "protocol/uri.h" #include "scripting/scripting.h" @@ -69,7 +69,7 @@ elinks_set_location(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) return JS_TRUE; } -/* function "alert" in the object returned by smjs_get_elinks_object() */ +/* @elinks_funcs{"alert"} */ static JSBool elinks_alert(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) { @@ -90,7 +90,7 @@ elinks_alert(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval return JS_TRUE; } -/* function "execute" in the object returned by smjs_get_elinks_object() */ +/* @elinks_funcs{"execute"} */ static JSBool elinks_execute(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) { @@ -117,6 +117,12 @@ static const JSClass elinks_class = { JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, JS_FinalizeStub }; +static const spidermonkeyFunctionSpec elinks_funcs[] = { + { "alert", elinks_alert, 1 }, + { "execute", elinks_execute, 1 }, + { NULL } +}; + static JSObject * smjs_get_elinks_object(void) { @@ -125,24 +131,9 @@ smjs_get_elinks_object(void) assert(smjs_ctx); assert(smjs_global_object); - jsobj = JS_InitClass(smjs_ctx, smjs_global_object, NULL, - (JSClass *) &elinks_class, NULL, 0, NULL, - (JSFunctionSpec *) NULL, NULL, NULL); - - /* Bug 1016: In SpiderMonkey 1.7 bundled with XULRunner 1.8, - * jsapi.h defines JSFunctionSpec in different ways depending - * on whether MOZILLA_1_8_BRANCH is defined, and there is no - * obvious way for ELinks to check whether MOZILLA_1_8_BRANCH - * was defined when the library was built. Avoid the unstable - * JSFunctionSpec definitions and instead use JS_DefineFunction - * directly. - * - * In elinks/src/ecmascript/spidermonkey/, there is an - * ELinks-specific replacement for JSFunctionSpec; however, to - * keep the modules independent, elinks/src/scripting/smjs/ - * does not use that. */ - JS_DefineFunction(smjs_ctx, jsobj, "alert", elinks_alert, 1, 0); - JS_DefineFunction(smjs_ctx, jsobj, "execute", elinks_execute, 1, 0); + jsobj = spidermonkey_InitClass(smjs_ctx, smjs_global_object, NULL, + (JSClass *) &elinks_class, NULL, 0, NULL, + elinks_funcs, NULL, NULL); JS_DefineProperty(smjs_ctx, jsobj, "location", JSVAL_NULL, elinks_get_location, elinks_set_location, diff --git a/src/scripting/smjs/elinks_object.h b/src/scripting/smjs/elinks_object.h index aa8f57d3..0395d892 100644 --- a/src/scripting/smjs/elinks_object.h +++ b/src/scripting/smjs/elinks_object.h @@ -1,7 +1,7 @@ #ifndef EL__SCRIPTING_SMJS_ELINKS_OBJECT_H #define EL__SCRIPTING_SMJS_ELINKS_OBJECT_H -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" /* This is the all-powerful elinks object through which all client scripts * will interface with ELinks. */ diff --git a/src/scripting/smjs/global_object.c b/src/scripting/smjs/global_object.c index f2c51102..3a597823 100644 --- a/src/scripting/smjs/global_object.c +++ b/src/scripting/smjs/global_object.c @@ -6,7 +6,7 @@ #include "elinks.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "scripting/scripting.h" #include "scripting/smjs/core.h" #include "scripting/smjs/global_object.h" diff --git a/src/scripting/smjs/global_object.h b/src/scripting/smjs/global_object.h index c7ff510a..1d2b6344 100644 --- a/src/scripting/smjs/global_object.h +++ b/src/scripting/smjs/global_object.h @@ -1,7 +1,7 @@ #ifndef EL__SCRIPTING_SMJS_GLOBAL_OBJECT_H #define EL__SCRIPTING_SMJS_GLOBAL_OBJECT_H -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" /* The root of the object hierarchy. If object 'foo' has this as its parent, * you can use foo's method and properties with 'foo.'. */ diff --git a/src/scripting/smjs/globhist.c b/src/scripting/smjs/globhist.c index 094f2e9d..0ab69386 100644 --- a/src/scripting/smjs/globhist.c +++ b/src/scripting/smjs/globhist.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "globhist/globhist.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "scripting/smjs/core.h" #include "scripting/smjs/elinks_object.h" #include "util/memory.h" diff --git a/src/scripting/smjs/hooks.c b/src/scripting/smjs/hooks.c index 647d2887..291039cf 100644 --- a/src/scripting/smjs/hooks.c +++ b/src/scripting/smjs/hooks.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "cache/cache.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "protocol/uri.h" #include "main/event.h" #include "main/module.h" diff --git a/src/scripting/smjs/keybinding.c b/src/scripting/smjs/keybinding.c index 61bffb27..2b354776 100644 --- a/src/scripting/smjs/keybinding.c +++ b/src/scripting/smjs/keybinding.c @@ -7,7 +7,7 @@ #include "elinks.h" #include "config/kbdbind.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "main/event.h" #include "scripting/smjs/core.h" #include "scripting/smjs/elinks_object.h" diff --git a/src/scripting/smjs/keybinding.h b/src/scripting/smjs/keybinding.h index b309420f..2f2893a2 100644 --- a/src/scripting/smjs/keybinding.h +++ b/src/scripting/smjs/keybinding.h @@ -1,7 +1,7 @@ #ifndef EL__SCRIPTING_SMJS_KEYBINDING_H #define EL__SCRIPTING_SMJS_KEYBINDING_H -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" void smjs_init_keybinding_interface(void); diff --git a/src/scripting/smjs/load_uri.c b/src/scripting/smjs/load_uri.c index b8adcc2c..4f2dcf26 100644 --- a/src/scripting/smjs/load_uri.c +++ b/src/scripting/smjs/load_uri.c @@ -6,7 +6,7 @@ #include "elinks.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "network/connection.h" #include "protocol/uri.h" #include "scripting/smjs/core.h" diff --git a/src/scripting/smjs/view_state_object.c b/src/scripting/smjs/view_state_object.c index b5f1ca0c..64f43caa 100644 --- a/src/scripting/smjs/view_state_object.c +++ b/src/scripting/smjs/view_state_object.c @@ -8,7 +8,7 @@ #include "elinks.h" -#include "ecmascript/spidermonkey/util.h" +#include "ecmascript/spidermonkey-shared.h" #include "protocol/uri.h" #include "scripting/smjs/elinks_object.h" #include "scripting/smjs/view_state_object.h" From 2024ea610b33db9322be11b9e144f6d2dac3abda Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Wed, 16 Jul 2008 14:23:19 +0300 Subject: [PATCH 14/32] 1031: Replace smjs_rt with spidermonkey_runtime src/scripting/smjs/ now uses a JSRuntime managed by spidermonkey-shared.c. --- src/ecmascript/spidermonkey-shared.c | 48 ++++++++++++++++++++++++++++ src/ecmascript/spidermonkey-shared.h | 4 +++ src/scripting/smjs/core.c | 24 +++++++------- 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/ecmascript/spidermonkey-shared.c b/src/ecmascript/spidermonkey-shared.c index 769a35b3..161b9900 100644 --- a/src/ecmascript/spidermonkey-shared.c +++ b/src/ecmascript/spidermonkey-shared.c @@ -9,6 +9,54 @@ #include "ecmascript/spidermonkey-shared.h" +/** A shared runtime used for both user scripts (scripting/smjs/) and + * scripts on web pages (ecmascript/spidermonkey/). + * + * SpiderMonkey has bugs that corrupt memory when multiple JSRuntimes + * are used: https://bugzilla.mozilla.org/show_bug.cgi?id=378918 and + * perhaps others. */ +JSRuntime *spidermonkey_runtime; + +/** A reference count for ::spidermonkey_runtime so that modules using + * it can be initialized and shut down in arbitrary order. */ +static int spidermonkey_runtime_refcount; + +/** Add a reference to ::spidermonkey_runtime, and initialize it if + * that is the first reference. + * @return 1 if successful or 0 on error. If this succeeds, the + * caller must eventually call spidermonkey_runtime_release(). */ +int +spidermonkey_runtime_addref(void) +{ + if (!spidermonkey_runtime) { + assert(spidermonkey_runtime_refcount == 0); + if_assert_failed return 0; + spidermonkey_runtime = JS_NewRuntime(1L * 1024L * 1024L); + if (!spidermonkey_runtime) return 0; + } + spidermonkey_runtime_refcount++; + assert(spidermonkey_runtime_refcount > 0); + if_assert_failed { spidermonkey_runtime_refcount--; return 0; } + return 1; +} + +/** Release a reference to ::spidermonkey_runtime, and destroy it if + * that was the last reference. If spidermonkey_runtime_addref() + * failed, then this must not be called. */ +void +spidermonkey_runtime_release(void) +{ + assert(spidermonkey_runtime_refcount > 0); + assert(spidermonkey_runtime); + if_assert_failed return; + + --spidermonkey_runtime_refcount; + if (spidermonkey_runtime_refcount == 0) { + JS_DestroyRuntime(spidermonkey_runtime); + spidermonkey_runtime = NULL; + } +} + /** An ELinks-specific replacement for JS_DefineFunctions(). * * @relates spidermonkeyFunctionSpec */ diff --git a/src/ecmascript/spidermonkey-shared.h b/src/ecmascript/spidermonkey-shared.h index 3b2602b6..225c5f46 100644 --- a/src/ecmascript/spidermonkey-shared.h +++ b/src/ecmascript/spidermonkey-shared.h @@ -24,6 +24,10 @@ #include "util/string.h" +extern JSRuntime *spidermonkey_runtime; +int spidermonkey_runtime_addref(void); +void spidermonkey_runtime_release(void); + /** An ELinks-specific replacement for JSFunctionSpec. * * Bug 1016: In SpiderMonkey 1.7 bundled with XULRunner 1.8, jsapi.h diff --git a/src/scripting/smjs/core.c b/src/scripting/smjs/core.c index 60fe7ba0..a5235c1b 100644 --- a/src/scripting/smjs/core.c +++ b/src/scripting/smjs/core.c @@ -68,8 +68,6 @@ reported: JS_ClearPendingException(ctx); } -static JSRuntime *smjs_rt; - static int smjs_do_file(unsigned char *path) { @@ -127,13 +125,11 @@ smjs_load_hooks(void) void init_smjs(struct module *module) { - smjs_rt = JS_NewRuntime(1L * 1024L * 1024L); - if (!smjs_rt) return; + if (!spidermonkey_runtime_addref()) return; - smjs_ctx = JS_NewContext(smjs_rt, 8192); + smjs_ctx = JS_NewContext(spidermonkey_runtime, 8192); if (!smjs_ctx) { - JS_DestroyRuntime(smjs_rt); - smjs_rt = NULL; + spidermonkey_runtime_release(); return; } @@ -154,9 +150,15 @@ cleanup_smjs(struct module *module) { if (!smjs_ctx) return; - /* These calls also finalize all JSObjects that have been - * allocated in the JSRuntime, so cache_entry_finalize gets - * called and resets each cache_entry.jsobject = NULL. */ + /* JS_DestroyContext also collects garbage in the JSRuntime. + * Because the JSObjects created in smjs_ctx have not been + * made visible to any other JSContext, and the garbage + * collector of SpiderMonkey is precise, SpiderMonkey + * finalizes all of those objects, so cache_entry_finalize + * gets called and resets each cache_entry.jsobject = NULL. + * If the garbage collector were conservative, ELinks would + * have to call smjs_detach_cache_entry_object on each cache + * entry before it releases the runtime here. */ JS_DestroyContext(smjs_ctx); - JS_DestroyRuntime(smjs_rt); + spidermonkey_runtime_release(); } From 031c1e6143093d731477fb4f266f466477975544 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Wed, 16 Jul 2008 14:50:41 +0300 Subject: [PATCH 15/32] 1031: Replace jsrt with spidermonkey_runtime src/ecmascript/spidermonkey/ now uses a JSRuntime managed by spidermonkey-shared.c. --- src/ecmascript/spidermonkey-shared.c | 16 +++++++++++++++- src/ecmascript/spidermonkey.c | 21 +++++++++++---------- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/ecmascript/spidermonkey-shared.c b/src/ecmascript/spidermonkey-shared.c index 161b9900..6d887551 100644 --- a/src/ecmascript/spidermonkey-shared.c +++ b/src/ecmascript/spidermonkey-shared.c @@ -29,11 +29,24 @@ int spidermonkey_runtime_addref(void) { if (!spidermonkey_runtime) { + JSContext *dummy; + assert(spidermonkey_runtime_refcount == 0); if_assert_failed return 0; - spidermonkey_runtime = JS_NewRuntime(1L * 1024L * 1024L); + + spidermonkey_runtime = JS_NewRuntime(4L * 1024L * 1024L); if (!spidermonkey_runtime) return 0; + + /* XXX: This is a hack to avoid a crash on exit. + * SMJS will crash on JS_DestroyRuntime if the given + * runtime has never had any context created, which + * will be the case if one closes ELinks without + * having loaded any documents. */ + dummy = JS_NewContext(spidermonkey_runtime, 0); + if (dummy) JS_DestroyContext(dummy); + /* Else hope it works anyway. */ } + spidermonkey_runtime_refcount++; assert(spidermonkey_runtime_refcount > 0); if_assert_failed { spidermonkey_runtime_refcount--; return 0; } @@ -54,6 +67,7 @@ spidermonkey_runtime_release(void) if (spidermonkey_runtime_refcount == 0) { JS_DestroyRuntime(spidermonkey_runtime); spidermonkey_runtime = NULL; + JS_ShutDown(); } } diff --git a/src/ecmascript/spidermonkey.c b/src/ecmascript/spidermonkey.c index 6dae8009..267f81c5 100644 --- a/src/ecmascript/spidermonkey.c +++ b/src/ecmascript/spidermonkey.c @@ -55,7 +55,7 @@ /* TODO? Are there any which need to be implemented? */ -static JSRuntime *jsrt; +static int js_module_init_ok; static void error_reporter(JSContext *ctx, const char *message, JSErrorReport *report) @@ -137,19 +137,14 @@ setup_safeguard(struct ecmascript_interpreter *interpreter, static void spidermonkey_init(struct module *xxx) { - jsrt = JS_NewRuntime(0x400000UL); - /* XXX: This is a hack to avoid a crash on exit. SMJS will crash - * on JS_DestroyRuntime if the given runtime has never had any context - * created, which will be the case if one closes ELinks without having - * loaded any documents. */ - JS_DestroyContext(JS_NewContext(jsrt, 0)); + js_module_init_ok = spidermonkey_runtime_addref(); } static void spidermonkey_done(struct module *xxx) { - JS_DestroyRuntime(jsrt); - JS_ShutDown(); + if (js_module_init_ok) + spidermonkey_runtime_release(); } @@ -161,8 +156,10 @@ spidermonkey_get_interpreter(struct ecmascript_interpreter *interpreter) *statusbar_obj, *menubar_obj, *navigator_obj; assert(interpreter); + if (!js_module_init_ok) return NULL; - ctx = JS_NewContext(jsrt, 8192 /* Stack allocation chunk size */); + ctx = JS_NewContext(spidermonkey_runtime, + 8192 /* Stack allocation chunk size */); if (!ctx) return NULL; interpreter->backend_data = ctx; @@ -236,6 +233,7 @@ spidermonkey_put_interpreter(struct ecmascript_interpreter *interpreter) JSContext *ctx; assert(interpreter); + if (!js_module_init_ok) return; ctx = interpreter->backend_data; JS_DestroyContext(ctx); interpreter->backend_data = NULL; @@ -250,6 +248,7 @@ spidermonkey_eval(struct ecmascript_interpreter *interpreter, jsval rval; assert(interpreter); + if (!js_module_init_ok) return; ctx = interpreter->backend_data; setup_safeguard(interpreter, ctx); interpreter->ret = ret; @@ -266,6 +265,7 @@ spidermonkey_eval_stringback(struct ecmascript_interpreter *interpreter, jsval rval; assert(interpreter); + if (!js_module_init_ok) return NULL; ctx = interpreter->backend_data; setup_safeguard(interpreter, ctx); interpreter->ret = NULL; @@ -293,6 +293,7 @@ spidermonkey_eval_boolback(struct ecmascript_interpreter *interpreter, int ret; assert(interpreter); + if (!js_module_init_ok) return 0; ctx = interpreter->backend_data; setup_safeguard(interpreter, ctx); interpreter->ret = NULL; From 2668602edc77720b021c5ce6fc74fabb68e16f59 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Wed, 16 Jul 2008 14:55:31 +0300 Subject: [PATCH 16/32] 1031: Mention in NEWS. --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 7108a8ef..5f52c010 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ generally also includes the bug fixes made in ELinks 0.11.4.GIT. * critical bug 1029 in user SMJS: Prefer JS_CallFunctionValue over JS_CallFunction, which can crash if given a closure. +* critical bug 1031: Use the same JSRuntime for both user SMJS and + scripts on web pages, to work around SpiderMonkey bug 378918. * minor bug 951: SpiderMonkey scripting objects used to prevent ELinks from removing files from the memory cache From e9f3a4a9d3890d03320ae918926b327197aa31e5 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Wed, 16 Jul 2008 15:19:34 +0200 Subject: [PATCH 17/32] 1033: Fixed memory leak in open(...). --- src/ecmascript/see/window.c | 9 +++++---- src/ecmascript/spidermonkey/window.c | 11 +++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ecmascript/see/window.c b/src/ecmascript/see/window.c index 5b3d3a55..513b962e 100644 --- a/src/ecmascript/see/window.c +++ b/src/ecmascript/see/window.c @@ -240,7 +240,7 @@ js_window_open(struct SEE_interpreter *interp, struct SEE_object *self, struct document_view *doc_view = vs->doc_view; struct session *ses = doc_view->session; unsigned char *frame = ""; - unsigned char *url; + unsigned char *url, *url2; struct uri *uri; struct SEE_value url_value; #if 0 @@ -291,10 +291,11 @@ js_window_open(struct SEE_interpreter *interp, struct SEE_object *self, } /* TODO: Support for window naming and perhaps some window features? */ - url = join_urls(doc_view->document->uri, url); - if (!url) return; - uri = get_uri(url, 0); + url2 = join_urls(doc_view->document->uri, url); mem_free(url); + if (!url2) return; + uri = get_uri(url2, 0); + mem_free(url2); if (!uri) return; if (*frame && strcasecmp(frame, "_blank")) { diff --git a/src/ecmascript/spidermonkey/window.c b/src/ecmascript/spidermonkey/window.c index a08b9e8e..ea8fe82c 100644 --- a/src/ecmascript/spidermonkey/window.c +++ b/src/ecmascript/spidermonkey/window.c @@ -342,7 +342,7 @@ window_open(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) struct document_view *doc_view; struct session *ses; unsigned char *frame = ""; - unsigned char *url; + unsigned char *url, *url2; struct uri *uri; static time_t ratelimit_start; static int ratelimit_count; @@ -387,10 +387,13 @@ window_open(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) /* TODO: Support for window naming and perhaps some window features? */ - url = join_urls(doc_view->document->uri, url); - if (!url) return JS_TRUE; - uri = get_uri(url, 0); + url2 = join_urls(doc_view->document->uri, url); mem_free(url); + if (!url2) { + return JS_TRUE; + } + uri = get_uri(url2, 0); + mem_free(url2); if (!uri) return JS_TRUE; From 887a751441a0cab4ecfc30a5929eb7dcddfd2fc3 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Wed, 16 Jul 2008 17:15:12 +0300 Subject: [PATCH 18/32] 1033: Mention in NEWS. --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 5f52c010..c18b265f 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,8 @@ Bugs that should be removed from NEWS before the 0.12.0 release: * bug 955: Reset buttons no longer run FORM/@onsubmit, and ``harmless'' buttons no longer submit the form. ELinks 0.12pre1 was the first release that had these bugs. +* bug 1033: Fix memory leak in ECMAScript window.open. ELinks 0.12pre1 + was the first release that had this bug. ELinks 0.12pre1: ---------------- From 5ad675e2449402184bfc7c1bbdc24cd8d67add9e Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 19:01:48 +0300 Subject: [PATCH 19/32] Remove my comment about prefixes used with SpiderMonkey --- src/ecmascript/spidermonkey-shared.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/ecmascript/spidermonkey-shared.h b/src/ecmascript/spidermonkey-shared.h index 225c5f46..a35f00ec 100644 --- a/src/ecmascript/spidermonkey-shared.h +++ b/src/ecmascript/spidermonkey-shared.h @@ -1,12 +1,6 @@ #ifndef EL__ECMASCRIPT_SPIDERMONKEY_SHARED_H #define EL__ECMASCRIPT_SPIDERMONKEY_SHARED_H -/* Inconsistently applied conventions for prefixes in identifiers: - * - "smjs" for user scripts (scripting/smjs/). - * - "js" for web scripts (ecmascript/spidermonkey/). - * - "spidermonkey" for common stuff, especially any that replace - * similarly named things defined in SpiderMonkey itself. */ - /* For wild SpiderMonkey installations. */ #ifdef CONFIG_OS_BEOS #define XP_BEOS From 8f2f9e7265787305060e59a6c971c99f85300d4b Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Mon, 7 Jul 2008 15:57:47 +0300 Subject: [PATCH 20/32] 952, 954: Add spidermonkey_empty_context --- src/ecmascript/spidermonkey-shared.c | 47 +++++++++++++++++++--------- src/ecmascript/spidermonkey-shared.h | 1 + 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/ecmascript/spidermonkey-shared.c b/src/ecmascript/spidermonkey-shared.c index 6d887551..c4cc60c2 100644 --- a/src/ecmascript/spidermonkey-shared.c +++ b/src/ecmascript/spidermonkey-shared.c @@ -17,36 +17,50 @@ * perhaps others. */ JSRuntime *spidermonkey_runtime; +/** A JSContext that can be used in JS_SetPrivate and JS_GetPrivate + * when no better one is available. This context has no global + * object, so scripts cannot be evaluated in it. + * + * XXX: This also works around a crash on exit. SMJS will crash on + * JS_DestroyRuntime if the given runtime has never had any context + * created, which will be the case if one closes ELinks without having + * loaded any documents. */ +JSContext *spidermonkey_empty_context; + /** A reference count for ::spidermonkey_runtime so that modules using * it can be initialized and shut down in arbitrary order. */ static int spidermonkey_runtime_refcount; -/** Add a reference to ::spidermonkey_runtime, and initialize it if - * that is the first reference. +/** Initialize ::spidermonkey_runtime and ::spidermonkey_empty_context. + * If already initialized, just increment the reference count. + * * @return 1 if successful or 0 on error. If this succeeds, the * caller must eventually call spidermonkey_runtime_release(). */ int spidermonkey_runtime_addref(void) { - if (!spidermonkey_runtime) { - JSContext *dummy; - - assert(spidermonkey_runtime_refcount == 0); + if (spidermonkey_runtime_refcount == 0) { + assert(spidermonkey_runtime == NULL); + assert(spidermonkey_empty_context == NULL); if_assert_failed return 0; spidermonkey_runtime = JS_NewRuntime(4L * 1024L * 1024L); if (!spidermonkey_runtime) return 0; - - /* XXX: This is a hack to avoid a crash on exit. - * SMJS will crash on JS_DestroyRuntime if the given - * runtime has never had any context created, which - * will be the case if one closes ELinks without - * having loaded any documents. */ - dummy = JS_NewContext(spidermonkey_runtime, 0); - if (dummy) JS_DestroyContext(dummy); - /* Else hope it works anyway. */ + + spidermonkey_empty_context = JS_NewContext(spidermonkey_runtime, + 0); + if (!spidermonkey_empty_context) { + /* Perhaps JS_DestroyRuntime will now crash + * because no context was created, but there's + * not much else to do. */ + JS_DestroyRuntime(spidermonkey_runtime); + spidermonkey_runtime = NULL; + JS_ShutDown(); + } } + assert(spidermonkey_runtime); + assert(spidermonkey_empty_context); spidermonkey_runtime_refcount++; assert(spidermonkey_runtime_refcount > 0); if_assert_failed { spidermonkey_runtime_refcount--; return 0; } @@ -61,10 +75,13 @@ spidermonkey_runtime_release(void) { assert(spidermonkey_runtime_refcount > 0); assert(spidermonkey_runtime); + assert(spidermonkey_empty_context); if_assert_failed return; --spidermonkey_runtime_refcount; if (spidermonkey_runtime_refcount == 0) { + JS_DestroyContext(spidermonkey_empty_context); + spidermonkey_empty_context = NULL; JS_DestroyRuntime(spidermonkey_runtime); spidermonkey_runtime = NULL; JS_ShutDown(); diff --git a/src/ecmascript/spidermonkey-shared.h b/src/ecmascript/spidermonkey-shared.h index a35f00ec..4cc0eebc 100644 --- a/src/ecmascript/spidermonkey-shared.h +++ b/src/ecmascript/spidermonkey-shared.h @@ -19,6 +19,7 @@ #include "util/string.h" extern JSRuntime *spidermonkey_runtime; +extern JSContext *spidermonkey_empty_context; int spidermonkey_runtime_addref(void); void spidermonkey_runtime_release(void); From bbadb99dd159dcc93959cd12a42473c91f6d2108 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 19:16:34 +0300 Subject: [PATCH 21/32] 952, 954: Add ecmascript_{detach,moved}_form_state stubs Anything that frees or reallocates struct form_state must now call the new functions ecmascript_detach_form_state or ecmascript_moved_form_state. These functions should then clear out any dangling pointers, but that has not yet been implemented. --- src/document/renderer.c | 4 ++-- src/ecmascript/ecmascript.c | 26 ++++++++++++++++++++++++-- src/ecmascript/ecmascript.h | 4 ++++ src/ecmascript/see.h | 3 +++ src/ecmascript/spidermonkey.h | 3 +++ src/viewer/text/form.c | 31 +++++++++++++++++++++++++++++++ src/viewer/text/form.h | 2 ++ src/viewer/text/vs.c | 13 ++++++++----- 8 files changed, 77 insertions(+), 9 deletions(-) diff --git a/src/document/renderer.c b/src/document/renderer.c index 4708430c..e5534ed2 100644 --- a/src/document/renderer.c +++ b/src/document/renderer.c @@ -337,8 +337,8 @@ render_document(struct view_state *vs, struct document_view *doc_view, if (doc_view->session && doc_view->session->reloadlevel > CACHE_MODE_NORMAL) - while (vs->form_info_len) - mem_free_if(vs->form_info[--vs->form_info_len].value); + for (; vs->form_info_len > 0; vs->form_info_len--) + done_form_state(&vs->form_info[vs->form_info_len - 1]); shrink_memory(0); diff --git a/src/ecmascript/ecmascript.c b/src/ecmascript/ecmascript.c index c5f255b1..c822b509 100644 --- a/src/ecmascript/ecmascript.c +++ b/src/ecmascript/ecmascript.c @@ -214,6 +214,23 @@ ecmascript_eval_boolback(struct ecmascript_interpreter *interpreter, return result; } +void ecmascript_detach_form_state(struct form_state *fs) +{ +#ifdef CONFIG_ECMASCRIPT_SEE + see_detach_form_state(fs); +#else + spidermonkey_detach_form_state(fs); +#endif +} + +void ecmascript_moved_form_state(struct form_state *fs) +{ +#ifdef CONFIG_ECMASCRIPT_SEE + see_moved_form_state(fs); +#else + spidermonkey_moved_form_state(fs); +#endif +} void ecmascript_reset_state(struct view_state *vs) @@ -221,14 +238,19 @@ ecmascript_reset_state(struct view_state *vs) struct form_view *fv; int i; + /* Normally, if vs->ecmascript == NULL, the associated + * ecmascript_obj pointers are also NULL. However, they might + * be non-NULL if the ECMAScript objects have been lazily + * created because of scripts running in sibling HTML frames. */ + for (i = 0; i < vs->form_info_len; i++) + ecmascript_detach_form_state(&vs->form_info[i]); + vs->ecmascript_fragile = 0; if (vs->ecmascript) ecmascript_put_interpreter(vs->ecmascript); foreach (fv, vs->forms) fv->ecmascript_obj = NULL; - for (i = 0; i < vs->form_info_len; i++) - vs->form_info[i].ecmascript_obj = NULL; vs->ecmascript = ecmascript_get_interpreter(vs); if (!vs->ecmascript) diff --git a/src/ecmascript/ecmascript.h b/src/ecmascript/ecmascript.h index e39bf4e1..ff93737f 100644 --- a/src/ecmascript/ecmascript.h +++ b/src/ecmascript/ecmascript.h @@ -8,6 +8,7 @@ #include "main/module.h" #include "util/time.h" +struct form_state; struct string; struct terminal; struct uri; @@ -70,6 +71,9 @@ void ecmascript_free_urls(struct module *module); struct ecmascript_interpreter *ecmascript_get_interpreter(struct view_state*vs); void ecmascript_put_interpreter(struct ecmascript_interpreter *interpreter); +void ecmascript_detach_form_state(struct form_state *fs); +void ecmascript_moved_form_state(struct form_state *fs); + void ecmascript_reset_state(struct view_state *vs); void ecmascript_eval(struct ecmascript_interpreter *interpreter, struct string *code, struct string *ret); diff --git a/src/ecmascript/see.h b/src/ecmascript/see.h index 1c643912..f22fe32e 100644 --- a/src/ecmascript/see.h +++ b/src/ecmascript/see.h @@ -7,6 +7,9 @@ struct string; void *see_get_interpreter(struct ecmascript_interpreter *interpreter); void see_put_interpreter(struct ecmascript_interpreter *interpreter); +#define see_detach_form_state(fs) ((fs)->ecmascript_obj = NULL) +#define see_moved_form_state(fs) ((void) (fs)) + void see_eval(struct ecmascript_interpreter *interpreter, struct string *code, struct string *ret); unsigned char *see_eval_stringback(struct ecmascript_interpreter *interpreter, struct string *code); int see_eval_boolback(struct ecmascript_interpreter *interpreter, struct string *code); diff --git a/src/ecmascript/spidermonkey.h b/src/ecmascript/spidermonkey.h index 5f4f9441..8dfbb7b9 100644 --- a/src/ecmascript/spidermonkey.h +++ b/src/ecmascript/spidermonkey.h @@ -7,6 +7,9 @@ struct string; void *spidermonkey_get_interpreter(struct ecmascript_interpreter *interpreter); void spidermonkey_put_interpreter(struct ecmascript_interpreter *interpreter); +#define spidermonkey_detach_form_state(fs) ((fs)->ecmascript_obj = NULL) +#define spidermonkey_moved_form_state(fs) ((void) (fs)) + void spidermonkey_eval(struct ecmascript_interpreter *interpreter, struct string *code, struct string *ret); unsigned char *spidermonkey_eval_stringback(struct ecmascript_interpreter *interpreter, struct string *code); int spidermonkey_eval_boolback(struct ecmascript_interpreter *interpreter, struct string *code); diff --git a/src/viewer/text/form.c b/src/viewer/text/form.c index bc8ef212..4437bd64 100644 --- a/src/viewer/text/form.c +++ b/src/viewer/text/form.c @@ -31,6 +31,7 @@ #include "document/forms.h" #include "document/html/parser.h" #include "document/view.h" +#include "ecmascript/ecmascript.h" #include "intl/gettext/libintl.h" #include "formhist/formhist.h" #include "mime/mime.h" @@ -239,11 +240,30 @@ find_form_state(struct document_view *doc_view, struct form_control *fc) if (n >= vs->form_info_len) { int nn = n + 1; +#ifdef CONFIG_ECMASCRIPT + const struct form_state *const old_form_info = vs->form_info; +#endif fs = mem_align_alloc(&vs->form_info, vs->form_info_len, nn, 0); if (!fs) return NULL; vs->form_info = fs; vs->form_info_len = nn; + +#ifdef CONFIG_ECMASCRIPT + /* TODO: Standard C does not allow this comparison; + * if the memory to which old_form_info pointed has + * been freed, then the value of the pointer itself is + * indeterminate. Fixing this would require changing + * mem_align_alloc to tell the caller whether it did + * realloc or not. */ + if (vs->form_info != old_form_info) { + /* vs->form_info[] was moved to a different address. + * Update all the ECMAScript objects that have + * pointers to its elements. */ + for (nn = 0; nn < vs->form_info_len; nn++) + ecmascript_moved_form_state(&vs->form_info[nn]); + } +#endif /* CONFIG_ECMASCRIPT */ } fs = &vs->form_info[n]; @@ -315,6 +335,17 @@ find_form_by_form_view(struct document *document, struct form_view *fv) return NULL; } +/** Free any data owned by @a fs, but not the struct form_state + * itself, because that is normally allocated as part of an array. + * @relates form_state */ +void +done_form_state(struct form_state *fs) +{ +#ifdef CONFIG_ECMASCRIPT + ecmascript_detach_form_state(fs); +#endif + mem_free_if(fs->value); +} int get_current_state(struct session *ses) diff --git a/src/viewer/text/form.h b/src/viewer/text/form.h index 79542b0e..bc3bba1f 100644 --- a/src/viewer/text/form.h +++ b/src/viewer/text/form.h @@ -113,6 +113,8 @@ struct form_view *find_form_view_in_vs(struct view_state *vs, int form_num); struct form_view *find_form_view(struct document_view *doc_view, struct form *form); struct form *find_form_by_form_view(struct document *document, struct form_view *fv); +void done_form_state(struct form_state *); + enum frame_event_status field_op(struct session *ses, struct document_view *doc_view, struct link *link, struct term_event *ev); void draw_form_entry(struct terminal *term, struct document_view *doc_view, struct link *link); diff --git a/src/viewer/text/vs.c b/src/viewer/text/vs.c index a7978dbe..46a68336 100644 --- a/src/viewer/text/vs.c +++ b/src/viewer/text/vs.c @@ -45,13 +45,13 @@ init_vs(struct view_state *vs, struct uri *uri, int plain) void destroy_vs(struct view_state *vs, int blast_ecmascript) { - int i; - - for (i = 0; i < vs->form_info_len; i++) - mem_free_if(vs->form_info[i].value); + /* form_state contains a pointer to form_view, so it's safest + * to delete the form_state first. */ + for (; vs->form_info_len > 0; vs->form_info_len--) + done_form_state(&vs->form_info[vs->form_info_len - 1]); + mem_free_set(&vs->form_info, NULL); if (vs->uri) done_uri(vs->uri); - mem_free_if(vs->form_info); free_list(vs->forms); #ifdef CONFIG_ECMASCRIPT if (blast_ecmascript && vs->ecmascript) @@ -114,6 +114,9 @@ copy_vs(struct view_state *dst, struct view_state *src) struct form_state *srcfs = &src->form_info[i]; struct form_state *dstfs = &dst->form_info[i]; +#ifdef CONFIG_ECMASCRIPT + dstfs->ecmascript_obj = NULL; +#endif if (srcfs->value) dstfs->value = stracpy(srcfs->value); /* XXX: This makes it O(nm). */ From 759fbb1142f12c1e1676205e50ac0eb66494c10d Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 19:43:30 +0300 Subject: [PATCH 22/32] 952, 954: Add ecmascript_detach_form_view stub Anything that frees struct form_view must now call the new function ecmascript_detach_form_view. This function should then clear out any dangling pointers, but that has not yet been implemented. --- src/ecmascript/ecmascript.c | 15 ++++++++++++--- src/ecmascript/ecmascript.h | 2 ++ src/ecmascript/see.h | 1 + src/ecmascript/spidermonkey.h | 1 + src/viewer/text/form.c | 12 ++++++++++++ src/viewer/text/form.h | 1 + src/viewer/text/vs.c | 8 +++++++- 7 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/ecmascript/ecmascript.c b/src/ecmascript/ecmascript.c index c822b509..1d8ebe3d 100644 --- a/src/ecmascript/ecmascript.c +++ b/src/ecmascript/ecmascript.c @@ -214,6 +214,16 @@ ecmascript_eval_boolback(struct ecmascript_interpreter *interpreter, return result; } +void +ecmascript_detach_form_view(struct form_view *fv) +{ +#ifdef CONFIG_ECMASCRIPT_SEE + see_detach_form_view(fv); +#else + spidermonkey_detach_form_view(fv); +#endif +} + void ecmascript_detach_form_state(struct form_state *fs) { #ifdef CONFIG_ECMASCRIPT_SEE @@ -242,6 +252,8 @@ ecmascript_reset_state(struct view_state *vs) * ecmascript_obj pointers are also NULL. However, they might * be non-NULL if the ECMAScript objects have been lazily * created because of scripts running in sibling HTML frames. */ + foreach (fv, vs->forms) + ecmascript_detach_form_view(fv); for (i = 0; i < vs->form_info_len; i++) ecmascript_detach_form_state(&vs->form_info[i]); @@ -249,9 +261,6 @@ ecmascript_reset_state(struct view_state *vs) if (vs->ecmascript) ecmascript_put_interpreter(vs->ecmascript); - foreach (fv, vs->forms) - fv->ecmascript_obj = NULL; - vs->ecmascript = ecmascript_get_interpreter(vs); if (!vs->ecmascript) vs->ecmascript_fragile = 1; diff --git a/src/ecmascript/ecmascript.h b/src/ecmascript/ecmascript.h index ff93737f..d24937b0 100644 --- a/src/ecmascript/ecmascript.h +++ b/src/ecmascript/ecmascript.h @@ -9,6 +9,7 @@ #include "util/time.h" struct form_state; +struct form_view; struct string; struct terminal; struct uri; @@ -71,6 +72,7 @@ void ecmascript_free_urls(struct module *module); struct ecmascript_interpreter *ecmascript_get_interpreter(struct view_state*vs); void ecmascript_put_interpreter(struct ecmascript_interpreter *interpreter); +void ecmascript_detach_form_view(struct form_view *fv); void ecmascript_detach_form_state(struct form_state *fs); void ecmascript_moved_form_state(struct form_state *fs); diff --git a/src/ecmascript/see.h b/src/ecmascript/see.h index f22fe32e..26f1c6fa 100644 --- a/src/ecmascript/see.h +++ b/src/ecmascript/see.h @@ -7,6 +7,7 @@ struct string; void *see_get_interpreter(struct ecmascript_interpreter *interpreter); void see_put_interpreter(struct ecmascript_interpreter *interpreter); +#define see_detach_form_view(fv) ((fv)->ecmascript_obj = NULL) #define see_detach_form_state(fs) ((fs)->ecmascript_obj = NULL) #define see_moved_form_state(fs) ((void) (fs)) diff --git a/src/ecmascript/spidermonkey.h b/src/ecmascript/spidermonkey.h index 8dfbb7b9..51f5078d 100644 --- a/src/ecmascript/spidermonkey.h +++ b/src/ecmascript/spidermonkey.h @@ -7,6 +7,7 @@ struct string; void *spidermonkey_get_interpreter(struct ecmascript_interpreter *interpreter); void spidermonkey_put_interpreter(struct ecmascript_interpreter *interpreter); +#define spidermonkey_detach_form_view(fv) ((fv)->ecmascript_obj = NULL) #define spidermonkey_detach_form_state(fs) ((fs)->ecmascript_obj = NULL) #define spidermonkey_moved_form_state(fs) ((void) (fs)) diff --git a/src/viewer/text/form.c b/src/viewer/text/form.c index 4437bd64..3d5738e0 100644 --- a/src/viewer/text/form.c +++ b/src/viewer/text/form.c @@ -347,6 +347,18 @@ done_form_state(struct form_state *fs) mem_free_if(fs->value); } +/** Free @a fv and any data owned by it. This does not call + * del_from_list(fv), so the caller must usually do that first. + * @relates form_view */ +void +done_form_view(struct form_view *fv) +{ +#ifdef CONFIG_ECMASCRIPT + ecmascript_detach_form_view(fv); +#endif + mem_free(fv); +} + int get_current_state(struct session *ses) { diff --git a/src/viewer/text/form.h b/src/viewer/text/form.h index bc3bba1f..74a99915 100644 --- a/src/viewer/text/form.h +++ b/src/viewer/text/form.h @@ -114,6 +114,7 @@ struct form_view *find_form_view(struct document_view *doc_view, struct form *fo struct form *find_form_by_form_view(struct document *document, struct form_view *fv); void done_form_state(struct form_state *); +void done_form_view(struct form_view *); enum frame_event_status field_op(struct session *ses, struct document_view *doc_view, struct link *link, struct term_event *ev); diff --git a/src/viewer/text/vs.c b/src/viewer/text/vs.c index 46a68336..e8450a22 100644 --- a/src/viewer/text/vs.c +++ b/src/viewer/text/vs.c @@ -45,14 +45,20 @@ init_vs(struct view_state *vs, struct uri *uri, int plain) void destroy_vs(struct view_state *vs, int blast_ecmascript) { + struct form_view *fv, *next; + /* form_state contains a pointer to form_view, so it's safest * to delete the form_state first. */ for (; vs->form_info_len > 0; vs->form_info_len--) done_form_state(&vs->form_info[vs->form_info_len - 1]); mem_free_set(&vs->form_info, NULL); + foreachsafe (fv, next, vs->forms) { + del_from_list(fv); + done_form_view(fv); + } + if (vs->uri) done_uri(vs->uri); - free_list(vs->forms); #ifdef CONFIG_ECMASCRIPT if (blast_ecmascript && vs->ecmascript) ecmascript_put_interpreter(vs->ecmascript); From 2d49f6e9cdf7ae0921c8e2aa9f05c829b4999230 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 19:46:12 +0300 Subject: [PATCH 23/32] 952, 954: Finalize form_state.ecmascript_obj for SpiderMonkey JSObject instances of input_class now again contain a private pointer directly to struct form_state. This pointer is cleared or updated when appropriate. --- src/ecmascript/spidermonkey.h | 4 +- src/ecmascript/spidermonkey/form.c | 154 ++++++++++++++++++++--------- 2 files changed, 109 insertions(+), 49 deletions(-) diff --git a/src/ecmascript/spidermonkey.h b/src/ecmascript/spidermonkey.h index 51f5078d..3742d49e 100644 --- a/src/ecmascript/spidermonkey.h +++ b/src/ecmascript/spidermonkey.h @@ -8,8 +8,8 @@ void *spidermonkey_get_interpreter(struct ecmascript_interpreter *interpreter); void spidermonkey_put_interpreter(struct ecmascript_interpreter *interpreter); #define spidermonkey_detach_form_view(fv) ((fv)->ecmascript_obj = NULL) -#define spidermonkey_detach_form_state(fs) ((fs)->ecmascript_obj = NULL) -#define spidermonkey_moved_form_state(fs) ((void) (fs)) +void spidermonkey_detach_form_state(struct form_state *fs); +void spidermonkey_moved_form_state(struct form_state *fs); void spidermonkey_eval(struct ecmascript_interpreter *interpreter, struct string *code, struct string *ret); unsigned char *spidermonkey_eval_stringback(struct ecmascript_interpreter *interpreter, struct string *code); diff --git a/src/ecmascript/spidermonkey/form.c b/src/ecmascript/spidermonkey/form.c index 1e2eded1..7592a28d 100644 --- a/src/ecmascript/spidermonkey/form.c +++ b/src/ecmascript/spidermonkey/form.c @@ -22,6 +22,7 @@ #include "document/forms.h" #include "document/view.h" #include "ecmascript/ecmascript.h" +#include "ecmascript/spidermonkey.h" #include "ecmascript/spidermonkey/document.h" #include "ecmascript/spidermonkey/form.h" #include "ecmascript/spidermonkey/window.h" @@ -56,28 +57,15 @@ static const JSClass form_class; /* defined below */ static JSBool input_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp); static JSBool input_set_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp); - -/* Indexes of reserved slots in instances of @input_class. */ -enum { - /* The slot contains an integer used as an index to - * view_state.form_info[]. This allows ELinks to reallocate - * form_info[] without keeping track of SMJS objects that - * refer to its elements. We do not use JSCLASS_HAS_PRIVATE - * for that because SMJS expects the private data to be an - * aligned pointer. */ - JSRS_INPUT_FSINDEX, - - /* Number of reserved slots. */ - JSRS_INPUT_COUNT -}; +static void input_finalize(JSContext *ctx, JSObject *obj); /* Each @input_class object must have a @form_class parent. */ static const JSClass input_class = { "input", /* here, we unleash ourselves */ - JSCLASS_HAS_RESERVED_SLOTS(JSRS_INPUT_COUNT), + JSCLASS_HAS_PRIVATE, /* struct form_state *, or NULL if detached */ JS_PropertyStub, JS_PropertyStub, input_get_property, input_set_property, - JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, JS_FinalizeStub + JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, input_finalize }; /* Tinyids of properties. Use negative values to distinguish these @@ -146,23 +134,18 @@ static unicode_val_T jsval_to_accesskey(JSContext *ctx, jsval *vp); static struct form_state * -input_get_form_state(JSContext *ctx, JSObject *obj, struct view_state *vs) +input_get_form_state(JSContext *ctx, JSObject *jsinput) { - jsval val; - int n; - JSBool ok; + struct form_state *fs = JS_GetInstancePrivate(ctx, jsinput, + (JSClass *) &input_class, + NULL); - ok = JS_GetReservedSlot(ctx, obj, JSRS_INPUT_FSINDEX, &val); - assert(ok); - assert(JSVAL_IS_INT(val)); + if (!fs) return NULL; /* detached */ + + assert(fs->ecmascript_obj == jsinput); if_assert_failed return NULL; - n = JSVAL_TO_INT(val); - assert(n >= 0); - assert(n < vs->form_info_len); - if_assert_failed return NULL; - - return &vs->form_info[n]; + return fs; } /* @input_class.getProperty */ @@ -199,7 +182,8 @@ input_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) (JSClass *) &window_class, NULL); doc_view = vs->doc_view; document = doc_view->document; - fs = input_get_form_state(ctx, obj, vs); + fs = input_get_form_state(ctx, obj); + if (!fs) return JS_FALSE; /* detached */ fc = find_form_control(document, fs); assert(fc); @@ -350,7 +334,8 @@ input_set_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) (JSClass *) &window_class, NULL); doc_view = vs->doc_view; document = doc_view->document; - fs = input_get_form_state(ctx, obj, vs); + fs = input_get_form_state(ctx, obj); + if (!fs) return JS_FALSE; /* detached */ fc = find_form_control(document, fs); assert(fc); @@ -475,7 +460,8 @@ input_click(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) doc_view = vs->doc_view; document = doc_view->document; ses = doc_view->session; - fs = input_get_form_state(ctx, obj, vs); + fs = input_get_form_state(ctx, obj); + if (!fs) return JS_FALSE; /* detached */ assert(fs); fc = find_form_control(document, fs); @@ -528,7 +514,8 @@ input_focus(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) doc_view = vs->doc_view; document = doc_view->document; ses = doc_view->session; - fs = input_get_form_state(ctx, obj, vs); + fs = input_get_form_state(ctx, obj); + if (!fs) return JS_FALSE; /* detached */ assert(fs); fc = find_form_control(document, fs); @@ -555,26 +542,99 @@ input_select(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval } static JSObject * -get_input_object(JSContext *ctx, JSObject *jsform, long number) +get_input_object(JSContext *ctx, JSObject *jsform, struct form_state *fs) { -#if 0 - if (fs->ecmascript_obj) - return fs->ecmascript_obj; -#endif + JSObject *jsinput = fs->ecmascript_obj; + + if (jsinput) { + /* This assumes JS_GetInstancePrivate cannot GC. */ + assert(JS_GetInstancePrivate(ctx, jsinput, + (JSClass *) &input_class, NULL) + == fs); + if_assert_failed return NULL; + + return jsinput; + } + /* jsform ('form') is input's parent */ /* FIXME: That is NOT correct since the real containing element * should be its parent, but gimme DOM first. --pasky */ - JSObject *jsinput = JS_NewObject(ctx, (JSClass *) &input_class, NULL, jsform); - + jsinput = JS_NewObject(ctx, (JSClass *) &input_class, NULL, jsform); + if (!jsinput) + return NULL; JS_DefineProperties(ctx, jsinput, (JSPropertySpec *) input_props); spidermonkey_DefineFunctions(ctx, jsinput, input_funcs); - JS_SetReservedSlot(ctx, jsinput, JSRS_INPUT_FSINDEX, INT_TO_JSVAL(number)); - return jsinput;; + + if (!JS_SetPrivate(ctx, jsinput, fs)) /* to @input_class */ + return NULL; + fs->ecmascript_obj = jsinput; + return jsinput; +} + +static void +input_finalize(JSContext *ctx, JSObject *jsinput) +{ + struct form_state *fs = JS_GetInstancePrivate(ctx, jsinput, + (JSClass *) &input_class, + NULL); + + if (fs) { + /* If this assertion fails, leave fs->ecmascript_obj + * unchanged, because it may point to a different + * JSObject whose private pointer will later have to + * be updated to avoid crashes. */ + assert(fs->ecmascript_obj == jsinput); + if_assert_failed return; + + fs->ecmascript_obj = NULL; + /* No need to JS_SetPrivate, because jsinput is being + * destroyed. */ + } +} + +void +spidermonkey_detach_form_state(struct form_state *fs) +{ + JSObject *jsinput = fs->ecmascript_obj; + + if (jsinput) { + /* This assumes JS_GetInstancePrivate and JS_SetPrivate + * cannot GC. */ + + /* If this assertion fails, it is not clear whether + * the private pointer of jsinput should be reset; + * crashes seem possible either way. Resetting it is + * easiest. */ + assert(JS_GetInstancePrivate(spidermonkey_empty_context, + jsinput, + (JSClass *) &input_class, NULL) + == fs); + if_assert_failed {} + + JS_SetPrivate(spidermonkey_empty_context, jsinput, NULL); + fs->ecmascript_obj = NULL; + } +} + +void +spidermonkey_moved_form_state(struct form_state *fs) +{ + JSObject *jsinput = fs->ecmascript_obj; + + if (jsinput) { + /* This assumes JS_SetPrivate cannot GC. If it could, + * then the GC might call input_finalize for some + * other object whose struct form_state has also been + * reallocated, and an assertion would fail in + * input_finalize. */ + JS_SetPrivate(spidermonkey_empty_context, jsinput, fs); + } } static JSObject * -get_form_control_object(JSContext *ctx, JSObject *jsform, enum form_type type, int number) +get_form_control_object(JSContext *ctx, JSObject *jsform, + enum form_type type, struct form_state *fs) { switch (type) { case FC_TEXT: @@ -588,7 +648,7 @@ get_form_control_object(JSContext *ctx, JSObject *jsform, enum form_type type, i case FC_BUTTON: case FC_HIDDEN: case FC_SELECT: - return get_input_object(ctx, jsform, (long)number); + return get_input_object(ctx, jsform, fs); case FC_TEXTAREA: /* TODO */ @@ -741,7 +801,7 @@ form_elements_item(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval struct form_state *fs = find_form_state(doc_view, fc); if (fs) { - JSObject *fcobj = get_form_control_object(ctx, parent_form, fc->type, fc->g_ctrl_num); + JSObject *fcobj = get_form_control_object(ctx, parent_form, fc->type, fs); if (fcobj) object_to_jsval(ctx, rval, fcobj); @@ -801,7 +861,7 @@ form_elements_namedItem(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, struct form_state *fs = find_form_state(doc_view, fc); if (fs) { - JSObject *fcobj = get_form_control_object(ctx, parent_form, fc->type, fc->g_ctrl_num); + JSObject *fcobj = get_form_control_object(ctx, parent_form, fc->type, fs); if (fcobj) object_to_jsval(ctx, rval, fcobj); @@ -908,7 +968,7 @@ form_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) undef_to_jsval(ctx, vp); fs = find_form_state(doc_view, fc); if (fs) { - fcobj = get_form_control_object(ctx, obj, fc->type, fc->g_ctrl_num); + fcobj = get_form_control_object(ctx, obj, fc->type, fs); if (fcobj) object_to_jsval(ctx, vp, fcobj); } From f4213ac35017226c43d003582d7c1e96ec78eb03 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 20:16:17 +0300 Subject: [PATCH 24/32] 952, 954: Finalize form_view.ecmascript_obj for SpiderMonkey --- src/ecmascript/spidermonkey.h | 2 +- src/ecmascript/spidermonkey/form.c | 120 ++++++++++++++++++++++++----- 2 files changed, 100 insertions(+), 22 deletions(-) diff --git a/src/ecmascript/spidermonkey.h b/src/ecmascript/spidermonkey.h index 3742d49e..119ff802 100644 --- a/src/ecmascript/spidermonkey.h +++ b/src/ecmascript/spidermonkey.h @@ -7,7 +7,7 @@ struct string; void *spidermonkey_get_interpreter(struct ecmascript_interpreter *interpreter); void spidermonkey_put_interpreter(struct ecmascript_interpreter *interpreter); -#define spidermonkey_detach_form_view(fv) ((fv)->ecmascript_obj = NULL) +void spidermonkey_detach_form_view(struct form_view *fv); void spidermonkey_detach_form_state(struct form_state *fs); void spidermonkey_moved_form_state(struct form_state *fs); diff --git a/src/ecmascript/spidermonkey/form.c b/src/ecmascript/spidermonkey/form.c index 7592a28d..49d26440 100644 --- a/src/ecmascript/spidermonkey/form.c +++ b/src/ecmascript/spidermonkey/form.c @@ -661,7 +661,7 @@ get_form_control_object(JSContext *ctx, JSObject *jsform, } - +static struct form_view *form_get_form_view(JSContext *ctx, JSObject *jsform, jsval *argv); static JSBool form_elements_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp); /* Each @form_elements_class object must have a @form_class parent. */ @@ -726,8 +726,8 @@ form_elements_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) (JSClass *) &window_class, NULL); doc_view = vs->doc_view; document = doc_view->document; - form_view = JS_GetInstancePrivate(ctx, parent_form, - (JSClass *) &form_class, NULL); + form_view = form_get_form_view(ctx, parent_form, NULL); + if (!form_view) return JS_FALSE; /* detached */ form = find_form_by_form_view(document, form_view); if (JSVAL_IS_STRING(id)) { @@ -784,8 +784,8 @@ form_elements_item(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval (JSClass *) &window_class, NULL); doc_view = vs->doc_view; document = doc_view->document; - form_view = JS_GetInstancePrivate(ctx, parent_form, - (JSClass *) &form_class, NULL); + form_view = form_get_form_view(ctx, parent_form, NULL); + if (!form_view) return JS_FALSE; /* detached */ form = find_form_by_form_view(document, form_view); if (argc != 1) @@ -843,8 +843,8 @@ form_elements_namedItem(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, (JSClass *) &window_class, NULL); doc_view = vs->doc_view; document = doc_view->document; - form_view = JS_GetInstancePrivate(ctx, parent_form, - (JSClass *) &form_class, NULL); + form_view = form_get_form_view(ctx, parent_form, NULL); + if (!form_view) return JS_FALSE; /* detached */ form = find_form_by_form_view(document, form_view); if (argc != 1) @@ -877,14 +877,15 @@ form_elements_namedItem(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, static JSBool form_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp); static JSBool form_set_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp); +static void form_finalize(JSContext *ctx, JSObject *obj); /* Each @form_class object must have a @document_class parent. */ static const JSClass form_class = { "form", - JSCLASS_HAS_PRIVATE, /* struct form_view * */ + JSCLASS_HAS_PRIVATE, /* struct form_view *, or NULL if detached */ JS_PropertyStub, JS_PropertyStub, form_get_property, form_set_property, - JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, JS_FinalizeStub + JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, form_finalize }; /* Tinyids of properties. Use negative values to distinguish these @@ -921,6 +922,21 @@ static const spidermonkeyFunctionSpec form_funcs[] = { { NULL } }; +static struct form_view * +form_get_form_view(JSContext *ctx, JSObject *jsform, jsval *argv) +{ + struct form_view *fv = JS_GetInstancePrivate(ctx, jsform, + (JSClass *) &form_class, + argv); + + if (!fv) return NULL; /* detached */ + + assert(fv->ecmascript_obj == jsform); + if_assert_failed return NULL; + + return fv; +} + /* @form_class.getProperty */ static JSBool form_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) @@ -948,7 +964,8 @@ form_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) vs = JS_GetInstancePrivate(ctx, parent_win, (JSClass *) &window_class, NULL); doc_view = vs->doc_view; - fv = JS_GetInstancePrivate(ctx, obj, (JSClass *) &form_class, NULL); + fv = form_get_form_view(ctx, obj, NULL); + if (!fv) return JS_FALSE; /* detached */ form = find_form_by_form_view(doc_view->document, fv); assert(form); @@ -1082,7 +1099,8 @@ form_set_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp) vs = JS_GetInstancePrivate(ctx, parent_win, (JSClass *) &window_class, NULL); doc_view = vs->doc_view; - fv = JS_GetInstancePrivate(ctx, obj, (JSClass *) &form_class, NULL); + fv = form_get_form_view(ctx, obj, NULL); + if (!fv) return JS_FALSE; /* detached */ form = find_form_by_form_view(doc_view->document, fv); assert(form); @@ -1162,7 +1180,8 @@ form_reset(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) vs = JS_GetInstancePrivate(ctx, parent_win, (JSClass *) &window_class, NULL); doc_view = vs->doc_view; - fv = JS_GetInstancePrivate(ctx, obj, (JSClass *) &form_class, argv); + fv = form_get_form_view(ctx, obj, argv); + if (!fv) return JS_FALSE; /* detached */ form = find_form_by_form_view(doc_view->document, fv); assert(form); @@ -1199,7 +1218,8 @@ form_submit(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) (JSClass *) &window_class, NULL); doc_view = vs->doc_view; ses = doc_view->session; - fv = JS_GetInstancePrivate(ctx, obj, (JSClass *) &form_class, argv); + fv = form_get_form_view(ctx, obj, argv); + if (!fv) return JS_FALSE; /* detached */ form = find_form_by_form_view(doc_view->document, fv); assert(form); @@ -1213,21 +1233,79 @@ form_submit(JSContext *ctx, JSObject *obj, uintN argc, jsval *argv, jsval *rval) JSObject * get_form_object(JSContext *ctx, JSObject *jsdoc, struct form_view *fv) { -#if 0 - if (fv->ecmascript_obj) - return fv->ecmascript_obj; -#endif + JSObject *jsform = fv->ecmascript_obj; + + if (jsform) { + /* This assumes JS_GetInstancePrivate cannot GC. */ + assert(JS_GetInstancePrivate(ctx, jsform, + (JSClass *) &form_class, NULL) + == fv); + if_assert_failed return NULL; + + return jsform; + } + /* jsdoc ('document') is fv's parent */ /* FIXME: That is NOT correct since the real containing element * should be its parent, but gimme DOM first. --pasky */ - JSObject *jsform = JS_NewObject(ctx, (JSClass *) &form_class, NULL, jsdoc); - + jsform = JS_NewObject(ctx, (JSClass *) &form_class, NULL, jsdoc); + if (jsform == NULL) + return NULL; JS_DefineProperties(ctx, jsform, (JSPropertySpec *) form_props); spidermonkey_DefineFunctions(ctx, jsform, form_funcs); - JS_SetPrivate(ctx, jsform, fv); /* to @form_class */ + + if (!JS_SetPrivate(ctx, jsform, fv)) /* to @form_class */ + return NULL; fv->ecmascript_obj = jsform; - return fv->ecmascript_obj; + return jsform; } + +static void +form_finalize(JSContext *ctx, JSObject *jsform) +{ + struct form_view *fv = JS_GetInstancePrivate(ctx, jsform, + (JSClass *) &form_class, + NULL); + + if (fv) { + /* If this assertion fails, leave fv->ecmascript_obj + * unchanged, because it may point to a different + * JSObject whose private pointer will later have to + * be updated to avoid crashes. */ + assert(fv->ecmascript_obj == jsform); + if_assert_failed return; + + fv->ecmascript_obj = NULL; + /* No need to JS_SetPrivate, because the object is + * being destroyed. */ + } +} + +void +spidermonkey_detach_form_view(struct form_view *fv) +{ + JSObject *jsform = fv->ecmascript_obj; + + if (jsform) { + /* This assumes JS_GetInstancePrivate and JS_SetPrivate + * cannot GC. */ + + /* If this assertion fails, it is not clear whether + * the private pointer of jsform should be reset; + * crashes seem possible either way. Resetting it is + * easiest. */ + assert(JS_GetInstancePrivate(spidermonkey_empty_context, + jsform, + (JSClass *) &form_class, NULL) + == fv); + if_assert_failed {} + + JS_SetPrivate(spidermonkey_empty_context, jsform, NULL); + fv->ecmascript_obj = NULL; + } +} + + static JSBool forms_get_property(JSContext *ctx, JSObject *obj, jsval id, jsval *vp); /* Each @forms_class object must have a @document_class parent. */ From c81e2051f8c6af2bd7f8b11a85125fef5cf77627 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 20:22:23 +0300 Subject: [PATCH 25/32] 952, 954: Finalize form_state.ecmascript_obj for SEE --- src/ecmascript/see.h | 4 +- src/ecmascript/see/form.c | 130 ++++++++++++++++++++++++++++++-------- 2 files changed, 107 insertions(+), 27 deletions(-) diff --git a/src/ecmascript/see.h b/src/ecmascript/see.h index 26f1c6fa..db87286e 100644 --- a/src/ecmascript/see.h +++ b/src/ecmascript/see.h @@ -8,8 +8,8 @@ void *see_get_interpreter(struct ecmascript_interpreter *interpreter); void see_put_interpreter(struct ecmascript_interpreter *interpreter); #define see_detach_form_view(fv) ((fv)->ecmascript_obj = NULL) -#define see_detach_form_state(fs) ((fs)->ecmascript_obj = NULL) -#define see_moved_form_state(fs) ((void) (fs)) +void see_detach_form_state(struct form_state *fs); +void see_moved_form_state(struct form_state *fs); void see_eval(struct ecmascript_interpreter *interpreter, struct string *code, struct string *ret); unsigned char *see_eval_stringback(struct ecmascript_interpreter *interpreter, struct string *code); diff --git a/src/ecmascript/see/form.c b/src/ecmascript/see/form.c index fb8746d9..c82e8cd4 100644 --- a/src/ecmascript/see/form.c +++ b/src/ecmascript/see/form.c @@ -22,6 +22,7 @@ #include "document/forms.h" #include "document/view.h" #include "ecmascript/ecmascript.h" +#include "ecmascript/see.h" #include "ecmascript/see/checktype.h" #include "ecmascript/see/document.h" #include "ecmascript/see/form.h" @@ -56,8 +57,9 @@ static void js_input_focus(struct SEE_interpreter *, struct SEE_object *, struct static void js_input_select(struct SEE_interpreter *, struct SEE_object *, struct SEE_object *, int, struct SEE_value **, struct SEE_value *); static int input_canput(struct SEE_interpreter *, struct SEE_object *, struct SEE_string *); static int input_hasproperty(struct SEE_interpreter *, struct SEE_object *, struct SEE_string *); -static struct js_input *js_get_input_object(struct SEE_interpreter *, struct js_form *, int); -static struct js_input *js_get_form_control_object(struct SEE_interpreter *, struct js_form *, enum form_type, int); +static struct js_input *js_get_input_object(struct SEE_interpreter *, struct js_form *, struct form_state *); +static void input_finalize(struct SEE_interpreter *, void *, void *); +static struct js_input *js_get_form_control_object(struct SEE_interpreter *, struct js_form *, enum form_type, struct form_state *); static void js_form_elems_item(struct SEE_interpreter *, struct SEE_object *, struct SEE_object *, int, struct SEE_value **, struct SEE_value *); static void js_form_elems_namedItem(struct SEE_interpreter *, struct SEE_object *, struct SEE_object *, int, struct SEE_value **, struct SEE_value *); @@ -140,7 +142,7 @@ struct js_input { struct SEE_object *click; struct SEE_object *focus; struct SEE_object *select; - int form_number; + struct form_state *fs; }; struct js_forms_object { @@ -159,12 +161,21 @@ struct js_form_elems { static inline struct form_state * -form_state_of_js_input(struct view_state *vs, const struct js_input *input) +form_state_of_js_input(struct SEE_interpreter *interp, + const struct js_input *input) { - assert(input->form_number >= 0); - assert(input->form_number < vs->form_info_len); - if_assert_failed return NULL; - return &vs->form_info[input->form_number]; + struct form_state *fs = input->fs; + + if (!fs) + SEE_error_throw(interp, interp->Error, + "Input field has been destroyed"); + + assert(fs->ecmascript_obj == input); + if_assert_failed + SEE_error_throw(interp, interp->Error, + "Internal corruption"); + + return fs; } static void @@ -177,7 +188,7 @@ input_get(struct SEE_interpreter *interp, struct SEE_object *o, struct document *document = doc_view->document; struct js_input *input = (struct js_input *)o; struct js_form *parent = input->parent; - struct form_state *fs = form_state_of_js_input(vs, input); + struct form_state *fs = form_state_of_js_input(interp, input); struct form_control *fc = find_form_control(document, fs); int linknum; struct link *link = NULL; @@ -280,7 +291,7 @@ input_put(struct SEE_interpreter *interp, struct SEE_object *o, struct document_view *doc_view = vs->doc_view; struct document *document = doc_view->document; struct js_input *input = (struct js_input *)o; - struct form_state *fs = form_state_of_js_input(vs, input); + struct form_state *fs = form_state_of_js_input(interp, input); struct form_control *fc = find_form_control(document, fs); int linknum; struct link *link = NULL; @@ -380,7 +391,7 @@ js_input_click(struct SEE_interpreter *interp, struct SEE_object *self, struct js_input *input = ( see_check_class(interp, thisobj, &js_input_object_class), (struct js_input *)thisobj); - struct form_state *fs = form_state_of_js_input(vs, input); + struct form_state *fs = form_state_of_js_input(interp, input); struct form_control *fc; int linknum; @@ -415,7 +426,7 @@ js_input_focus(struct SEE_interpreter *interp, struct SEE_object *self, struct js_input *input = ( see_check_class(interp, thisobj, &js_input_object_class), (struct js_input *)thisobj); - struct form_state *fs = form_state_of_js_input(vs, input); + struct form_state *fs = form_state_of_js_input(interp, input); struct form_control *fc; int linknum; @@ -460,19 +471,23 @@ input_hasproperty(struct SEE_interpreter *interp, struct SEE_object *o, } static struct js_input * -js_get_input_object(struct SEE_interpreter *interp, struct js_form *jsform, int num) +js_get_input_object(struct SEE_interpreter *interp, struct js_form *jsform, + struct form_state *fs) { - struct js_input *jsinput; + struct js_input *jsinput = fs->ecmascript_obj; + if (jsinput) { + assert(jsinput->fs == fs); + if_assert_failed return NULL; + + return jsinput; + } -#if 0 - if (fs->ecmascript_obj) - return fs->ecmascript_obj; -#endif /* jsform ('form') is input's parent */ /* FIXME: That is NOT correct since the real containing element * should be its parent, but gimme DOM first. --pasky */ - jsinput = SEE_NEW(interp, struct js_input); + jsinput = SEE_NEW_FINALIZE(interp, struct js_input, + input_finalize, NULL); jsinput->object.objectclass = &js_input_object_class; jsinput->object.Prototype = NULL; @@ -482,14 +497,15 @@ js_get_input_object(struct SEE_interpreter *interp, struct js_form *jsform, int jsinput->focus = SEE_cfunction_make(interp, js_input_focus, s_focus, 0); jsinput->select = SEE_cfunction_make(interp, js_input_select, s_select, 0); - jsinput->form_number = num; jsinput->parent = jsform; + jsinput->fs = fs; + fs->ecmascript_obj = jsinput; return jsinput; } static struct js_input * js_get_form_control_object(struct SEE_interpreter *interp, struct js_form *jsform, - enum form_type type, int num) + enum form_type type, struct form_state *fs) { switch (type) { case FC_TEXT: @@ -503,7 +519,7 @@ js_get_form_control_object(struct SEE_interpreter *interp, struct js_form *jsfor case FC_BUTTON: case FC_HIDDEN: case FC_SELECT: - return js_get_input_object(interp, jsform, num); + return js_get_input_object(interp, jsform, fs); case FC_TEXTAREA: /* TODO */ @@ -515,7 +531,71 @@ js_get_form_control_object(struct SEE_interpreter *interp, struct js_form *jsfor } } +static void +input_finalize(struct SEE_interpreter *interp, void *jsinput_void, void *dummy) +{ + struct js_input *jsinput = jsinput_void; + struct form_state *fs = jsinput->fs; + if (fs) { + /* Reset jsinput->fs in case some ELinks code uses it + * even after this finalizer has run. Such use should + * not be possible but the explanation is somewhat + * complex so it seems safest to do this. + * + * Unlike SpiderMonkey, Boehm's GC allows a finalizer + * to resurrect the object by making something point + * to it. And it's a conservative GC so it can see + * such a pointer where none actually exists. However + * it also implicitly unregisters the finalizer before + * calling it, so the object becomes just a chunk of + * memory that nothing really uses, even if the GC + * never realizes it is garbage. */ + jsinput->fs = NULL; + + /* If this assertion fails, leave fs->ecmascript_obj + * unchanged, because it may point to a different + * structure whose js_input.fs pointer will later have + * to be updated to avoid crashes. + * + * If the assertion fails and we leave jsinput->fs + * unchanged, and something then deletes fs, + * jsinput->fs becomes a dangling pointer because fs + * does not know about jsinput. So that's why the + * assertion comes after the jsinput->fs assignment + * above. */ + assert(fs->ecmascript_obj == jsinput); + if_assert_failed return; + + fs->ecmascript_obj = NULL; + } +} + +void +see_detach_form_state(struct form_state *fs) +{ + struct js_input *jsinput = fs->ecmascript_obj; + + if (jsinput) { + /* If this assertion fails, it is not clear whether + * jsinput->fs should be reset; crashes seem possible + * either way. Resetting it is easiest. */ + assert(jsinput->fs == fs); + if_assert_failed {} + + jsinput->fs = NULL; + fs->ecmascript_obj = NULL; + } +} + +void +see_moved_form_state(struct form_state *fs) +{ + struct js_input *jsinput = fs->ecmascript_obj; + + if (jsinput) + jsinput->fs = fs; +} static void @@ -553,7 +633,7 @@ js_form_elems_item(struct SEE_interpreter *interp, struct SEE_object *self, struct form_state *fs = find_form_state(doc_view, fc); if (fs) { - struct js_input *fcobj = js_get_form_control_object(interp, parent_form, fc->type, fc->g_ctrl_num); + struct js_input *fcobj = js_get_form_control_object(interp, parent_form, fc->type, fs); if (fcobj) SEE_SET_OBJECT(res, (struct SEE_object *)fcobj); @@ -594,7 +674,7 @@ js_form_elems_namedItem(struct SEE_interpreter *interp, struct SEE_object *self, struct form_state *fs = find_form_state(doc_view, fc); if (fs) { - struct js_input *fcobj = js_get_form_control_object(interp, parent_form, fc->type, fc->g_ctrl_num); + struct js_input *fcobj = js_get_form_control_object(interp, parent_form, fc->type, fs); if (fcobj) SEE_SET_OBJECT(res, (struct SEE_object *)fcobj); @@ -857,7 +937,7 @@ form_get(struct SEE_interpreter *interp, struct SEE_object *o, continue; fs = find_form_state(doc_view, fc); if (fs) { - fcobj = js_get_form_control_object(interp, js_form, fc->type, fc->g_ctrl_num); + fcobj = js_get_form_control_object(interp, js_form, fc->type, fs); if (fcobj) SEE_SET_OBJECT(res, (struct SEE_object *)fcobj); From ae8080b172340cf4c15d55e0321874e0339aa021 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 20:43:23 +0300 Subject: [PATCH 26/32] 952, 954: Finalize form_view.ecmascript_obj for SEE --- src/ecmascript/see.h | 2 +- src/ecmascript/see/form.c | 109 +++++++++++++++++++++++++++++++++----- 2 files changed, 96 insertions(+), 15 deletions(-) diff --git a/src/ecmascript/see.h b/src/ecmascript/see.h index db87286e..fa211c7d 100644 --- a/src/ecmascript/see.h +++ b/src/ecmascript/see.h @@ -7,7 +7,7 @@ struct string; void *see_get_interpreter(struct ecmascript_interpreter *interpreter); void see_put_interpreter(struct ecmascript_interpreter *interpreter); -#define see_detach_form_view(fv) ((fv)->ecmascript_obj = NULL) +void see_detach_form_view(struct form_view *fv); void see_detach_form_state(struct form_state *fs); void see_moved_form_state(struct form_state *fs); diff --git a/src/ecmascript/see/form.c b/src/ecmascript/see/form.c index c82e8cd4..40f3e1ae 100644 --- a/src/ecmascript/see/form.c +++ b/src/ecmascript/see/form.c @@ -77,6 +77,7 @@ static int form_canput(struct SEE_interpreter *, struct SEE_object *, struct SEE static int form_hasproperty(struct SEE_interpreter *, struct SEE_object *, struct SEE_string *); static void js_form_reset(struct SEE_interpreter *, struct SEE_object *, struct SEE_object *, int, struct SEE_value **, struct SEE_value *); static void js_form_submit(struct SEE_interpreter *, struct SEE_object *, struct SEE_object *, int, struct SEE_value **, struct SEE_value *); +static void form_finalize(struct SEE_interpreter *, void *, void *); struct SEE_objectclass js_input_object_class = { @@ -598,6 +599,24 @@ see_moved_form_state(struct form_state *fs) } +static inline struct form_view * +form_view_of_js_form(struct SEE_interpreter *interp, + const struct js_form *jsform) +{ + struct form_view *fv = jsform->fv; + + if (!fv) + SEE_error_throw(interp, interp->Error, + "Form has been destroyed"); + + assert(fv->ecmascript_obj == jsform); + if_assert_failed + SEE_error_throw(interp, interp->Error, + "Internal corruption"); + + return fv; +} + static void js_form_elems_item(struct SEE_interpreter *interp, struct SEE_object *self, struct SEE_object *thisobj, int argc, struct SEE_value **argv, @@ -611,7 +630,7 @@ js_form_elems_item(struct SEE_interpreter *interp, struct SEE_object *self, see_check_class(interp, thisobj, &js_form_elems_class), (struct js_form_elems *)thisobj); struct js_form *parent_form = jsfe->parent; - struct form_view *fv = parent_form->fv; + struct form_view *fv = form_view_of_js_form(interp, parent_form); struct form *form = find_form_by_form_view(document, fv); struct form_control *fc; unsigned char *string; @@ -657,7 +676,7 @@ js_form_elems_namedItem(struct SEE_interpreter *interp, struct SEE_object *self, see_check_class(interp, thisobj, &js_form_elems_class), (struct js_form_elems *)thisobj); struct js_form *parent_form = jsfe->parent; - struct form_view *fv = parent_form->fv; + struct form_view *fv = form_view_of_js_form(interp, parent_form); struct form *form = find_form_by_form_view(document, fv); struct form_control *fc; unsigned char *string; @@ -695,7 +714,7 @@ form_elems_get(struct SEE_interpreter *interp, struct SEE_object *o, struct document *document = doc_view->document; struct js_form_elems *jsfe = (struct js_form_elems *)o; struct js_form *parent_form = jsfe->parent; - struct form_view *fv = parent_form->fv; + struct form_view *fv = form_view_of_js_form(interp, parent_form); struct form *form = find_form_by_form_view(document, fv); if (p == s_length) { @@ -863,7 +882,7 @@ form_get(struct SEE_interpreter *interp, struct SEE_object *o, struct view_state *vs = g->win->vs; struct document_view *doc_view = vs->doc_view; struct js_form *js_form = (struct js_form *)o; - struct form_view *fv = js_form->fv; + struct form_view *fv = form_view_of_js_form(interp, js_form); struct form *form = find_form_by_form_view(doc_view->document, fv); struct SEE_string *str; @@ -956,7 +975,7 @@ form_put(struct SEE_interpreter *interp, struct SEE_object *o, struct view_state *vs = g->win->vs; struct document_view *doc_view = vs->doc_view; struct js_form *js_form = (struct js_form *)o; - struct form_view *fv = js_form->fv; + struct form_view *fv = form_view_of_js_form(interp, js_form); struct form *form = find_form_by_form_view(doc_view->document, fv); unsigned char *string = see_value_to_unsigned_char(interp, val); @@ -1018,7 +1037,7 @@ js_form_reset(struct SEE_interpreter *interp, struct SEE_object *self, struct js_form *js_form = ( see_check_class(interp, thisobj, &js_form_class), (struct js_form *)thisobj); - struct form_view *fv = js_form->fv; + struct form_view *fv = form_view_of_js_form(interp, js_form); struct form *form = find_form_by_form_view(doc_view->document, fv); assert(form); @@ -1040,7 +1059,7 @@ js_form_submit(struct SEE_interpreter *interp, struct SEE_object *self, struct js_form *js_form = ( see_check_class(interp, thisobj, &js_form_class), (struct js_form *)thisobj); - struct form_view *fv = js_form->fv; + struct form_view *fv = form_view_of_js_form(interp, js_form); struct form *form = find_form_by_form_view(doc_view->document, fv); assert(form); @@ -1051,27 +1070,89 @@ js_form_submit(struct SEE_interpreter *interp, struct SEE_object *self, struct js_form *js_get_form_object(struct SEE_interpreter *interp, struct js_document_object *doc, struct form_view *fv) { - struct js_form *js_form; + struct js_form *js_form = fv->ecmascript_obj; + + if (js_form) { + assert(js_form->fv == fv); + if_assert_failed return NULL; + + return js_form; + } -#if 0 - if (fv->ecmascript_obj) - return fv->ecmascript_obj; -#endif /* jsdoc ('document') is fv's parent */ /* FIXME: That is NOT correct since the real containing element * should be its parent, but gimme DOM first. --pasky */ - js_form = SEE_NEW(interp, struct js_form); + js_form = SEE_NEW_FINALIZE(interp, struct js_form, + form_finalize, NULL); js_form->object.objectclass = &js_form_class; js_form->object.Prototype = NULL; /* TODO: use prototype for form */ js_form->parent = doc; js_form->reset = SEE_cfunction_make(interp, js_form_reset, s_reset, 0); js_form->submit = SEE_cfunction_make(interp, js_form_submit, s_submit, 0); - js_form->fv = fv; + js_form->fv = fv; fv->ecmascript_obj = js_form; return js_form; } +static void +form_finalize(struct SEE_interpreter *interp, void *jsform_void, void *dummy) +{ + struct js_form *jsform = jsform_void; + struct form_view *fv = jsform->fv; + + if (fv) { + /* Reset jsform->fv in case some ELinks code uses it + * even after this finalizer has run. Such use should + * not be possible but the explanation is somewhat + * complex so it seems safest to do this. + * + * Unlike SpiderMonkey, Boehm's GC allows a finalizer + * to resurrect the object by making something point + * to it. And it's a conservative GC so it can see + * such a pointer where none actually exists. However + * it also implicitly unregisters the finalizer before + * calling it, so the object becomes just a chunk of + * memory that nothing really uses, even if the GC + * never realizes it is garbage. */ + jsform->fv = NULL; + + /* If this assertion fails, leave fv->ecmascript_obj + * unchanged, because it may point to a different + * structure whose js_form.fv pointer will later have + * to be updated to avoid crashes. + * + * If the assertion fails and we leave jsform->fv + * unchanged, and something then deletes fv, + * jsform->fv becomes a dangling pointer because fv + * does not know about jsform. So that's why the + * assertion comes after the jsform->fv assignment + * above. */ + assert(fv->ecmascript_obj == jsform); + if_assert_failed return; + + fv->ecmascript_obj = NULL; + } +} + +void +see_detach_form_view(struct form_view *fv) +{ + struct js_form *jsform = fv->ecmascript_obj; + + if (jsform) { + /* If this assertion fails, it is not clear whether + * jsform->fv should be reset; crashes seem possible + * either way. Resetting it is easiest. */ + assert(jsform->fv == fv); + if_assert_failed {} + + jsform->fv = NULL; + fv->ecmascript_obj = NULL; + } +} + + void init_js_forms_object(struct ecmascript_interpreter *interpreter) { From 39a5d684472649abbc3903676af7e1808a4eb57a Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Fri, 18 Jul 2008 20:54:05 +0300 Subject: [PATCH 27/32] 952, 954: Describe in NEWS. --- NEWS | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS b/NEWS index c18b265f..7ee6608d 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,12 @@ ELinks 0.12pre1.GIT now: To be released as 0.12pre2, 0.12rc1, or even 0.12.0. This branch generally also includes the bug fixes made in ELinks 0.11.4.GIT. +* bug 954, enhancement 952: Keep track of ECMAScript form and input + objects instead of constructing new ones on every access. When the + corresponding ELinks internal objects are destroyed, detach the + ECMAScript objects from them, to prevent crashes. (Bug 954 was + first added in ELinks 0.11.4, and the bug 620 fix in ELinks 0.12pre1 + made crashes more likely.) * critical bug 1029 in user SMJS: Prefer JS_CallFunctionValue over JS_CallFunction, which can crash if given a closure. * critical bug 1031: Use the same JSRuntime for both user SMJS and From 85bfba4530c6a96fc3b07ca7da18f48456b1bfe8 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 20 Jul 2008 09:26:52 +0300 Subject: [PATCH 28/32] SEE: Do not check thisobj->objectclass in window functions. init_js_window_object() copies the alert, open, and setTimeout methods from the window object to the global object. My fix for bug 846 on 2006-12-10 incorrectly made the corresponding C functions refuse to work if they were not called as methods of the window object. --- NEWS | 3 +++ src/ecmascript/see/window.c | 12 +++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 7ee6608d..328a02a3 100644 --- a/NEWS +++ b/NEWS @@ -36,6 +36,9 @@ Bugs that should be removed from NEWS before the 0.12.0 release: was the first release that had these bugs. * bug 1033: Fix memory leak in ECMAScript window.open. ELinks 0.12pre1 was the first release that had this bug. +* Global ECMAScript functions alert, open, and setTimeout again work + with SEE. ELinks 0.12pre1 was the first release that supported SEE + at all. ELinks 0.12pre1: ---------------- diff --git a/src/ecmascript/see/window.c b/src/ecmascript/see/window.c index 513b962e..5df35c7d 100644 --- a/src/ecmascript/see/window.c +++ b/src/ecmascript/see/window.c @@ -211,7 +211,9 @@ js_window_alert(struct SEE_interpreter *interp, struct SEE_object *self, struct view_state *vs = win->vs; unsigned char *string; - see_check_class(interp, thisobj, &js_window_object_class); + /* Do not check thisobj->objectclass. ELinks sets this + * function as a property of both the window object and the + * global object, so thisobj may validly refer to either. */ SEE_SET_BOOLEAN(res, 1); if (argc < 1) @@ -248,7 +250,9 @@ js_window_open(struct SEE_interpreter *interp, struct SEE_object *self, static int ratelimit_count; #endif - see_check_class(interp, thisobj, &js_window_object_class); + /* Do not check thisobj->objectclass. ELinks sets this + * function as a property of both the window object and the + * global object, so thisobj may validly refer to either. */ SEE_SET_OBJECT(res, (struct SEE_object *)win); if (get_opt_bool("ecmascript.block_window_opening")) { @@ -342,7 +346,9 @@ js_setTimeout(struct SEE_interpreter *interp, struct SEE_object *self, unsigned char *code; int timeout; - see_check_class(interp, thisobj, &js_window_object_class); + /* Do not check thisobj->objectclass. ELinks sets this + * function as a property of both the window object and the + * global object, so thisobj may validly refer to either. */ if (argc != 2) return; ei = ((struct global_object *)interp)->interpreter; From 83ccaa367316e944b3c5d5ab672526e0de55b670 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Mon, 14 Jul 2008 15:02:06 +0300 Subject: [PATCH 29/32] Bug 698: Keep forms contiguous and non-overlapping and start from 0. In document.forms, each struct form has form_num and form_end members that reserve a subrange of [0, INT_MAX] to that form. Previously, multiple forms in the list could have form_end == INT_MAX and thus overlap each other. Prevent that by adjusting form_end of each form newly added to the list. Revert 438f039bda7d424f79502b3f83e99cedcb7f7ae9, "check_html_form_hierarchy: Old code was buggy.", which made check_html_form_hierarchy attach controls to the wrong forms. Instead, construct the dummy form ("for those Flying Dutchmans") at form_num == 0 always before adding any real forms to the list. This prevents the assertion failure by ensuring that every possible form_control.position is covered by some form, if there are any forms. Add a function assert_forms_list_ok, which checks that the set of forms actually covers the [0, INT_MAX] range without overlapping, as intended. Call that from check_html_form_hierarchy to detect any corruption. I have tested this code (before any cherry-picking) with: - bug 613 attachment 210: didn't crash - bug 714 attachment 471: didn't crash - bug 961 attachment 382: didn't crash - bug 698 attachment 239: all the submit buttons showed the right URLs - bug 698 attachment 470: the submit button showed the right URL (cherry picked from commit 386a5d517b5996112bc5a75b1a1b917c7adffe88) --- NEWS | 2 + src/document/html/renderer.c | 148 +++++++++++++++++++++++++---------- 2 files changed, 107 insertions(+), 43 deletions(-) diff --git a/NEWS b/NEWS index 328a02a3..8f0000ff 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,8 @@ generally also includes the bug fixes made in ELinks 0.11.4.GIT. JS_CallFunction, which can crash if given a closure. * critical bug 1031: Use the same JSRuntime for both user SMJS and scripts on web pages, to work around SpiderMonkey bug 378918. +* bug 698: Attach controls to the intended form even if it is + incorrectly nested in a table. (Was broken in 0.11.4.) * minor bug 951: SpiderMonkey scripting objects used to prevent ELinks from removing files from the memory cache diff --git a/src/document/html/renderer.c b/src/document/html/renderer.c index 486607c6..d7e5cf62 100644 --- a/src/document/html/renderer.c +++ b/src/document/html/renderer.c @@ -1746,7 +1746,11 @@ end: static void html_special_form(struct part *part, struct form *form) { + struct form *nform; + assert(part && form); + assert(form->form_num > 0); + assert(form->form_end == INT_MAX); if_assert_failed return; if (!part->document) { @@ -1754,50 +1758,60 @@ html_special_form(struct part *part, struct form *form) return; } - if (!list_empty(part->document->forms)) { - struct form *nform; - - /* Make sure the new form ``claims'' its slice of the form range - * maintained in the form_num and form_end variables. */ - foreach (nform, part->document->forms) { - if (form->form_num < nform->form_num - || nform->form_end < form->form_num) - continue; - - /* First check if the form has identical form numbers. - * That should only be the case when the form being - * added is in fact the same form in which case it - * should be dropped. The fact that this can happen - * suggests that the table renderering can be confused. - * See bug 647 for a test case. */ - if (nform->form_num == form->form_num - && nform->form_end == form->form_end) { - done_form(form); - return; - } - - /* The form start is inside an already added form, so - * partition the space of the existing form and get - * |old|new|. */ - nform->form_end = form->form_num - 1; - assertm(nform->form_num <= nform->form_end, - "[%d:%d] [%d:%d]", nform->form_num, nform->form_end, - form->form_num, form->form_end); - break; + /* Make a fake form with form_num == 0 so that there is + * something to use if form controls appear above the first + * actual FORM element. There can never be a real form with + * form_num == 0 because the form_num is the position after the + * "document->forms)) { + nform = init_form(); + if (!nform) { + done_form(form); + return; } - } else { - /* If it is the first form make sure it eats the whole form - * range. */ -#if 0 - /* Disabled because in tables the parse order may lead to a - * later form being parsed before a preceeding one causing the - * wrong order if we set it to zero. Let's hope it doesn't break - * anything else. */ - form->form_num = 0; -#endif + nform->form_num = 0; + add_to_list(part->document->forms, nform); } - add_to_list(part->document->forms, form); + /* Make sure the new form ``claims'' its slice of the form range + * maintained in the form_num and form_end variables. */ + foreach (nform, part->document->forms) { + if (form->form_num < nform->form_num + || nform->form_end < form->form_num) + continue; + + /* First check if the form has identical form numbers. + * That should only be the case when the form being + * added is in fact the same form in which case it + * should be dropped. The fact that this can happen + * suggests that the table renderering can be confused. + * See bug 647 for a test case. + * Do not compare form->form_end here because it is + * normally set by this function and that has obviously + * not yet been done. */ + if (nform->form_num == form->form_num) { + done_form(form); + return; + } + + /* The form start is inside an already added form, so + * partition the space of the existing form and get + * |old|new|. */ + form->form_end = nform->form_end; + nform->form_end = form->form_num - 1; + assertm(nform->form_num <= nform->form_end, + "[%d:%d] [%d:%d]", nform->form_num, nform->form_end, + form->form_num, form->form_end); + add_to_list(part->document->forms, form); + return; + } + + ERROR("hole between forms"); + done_form(form); + return; } static void @@ -1830,6 +1844,51 @@ html_special_form_control(struct part *part, struct form_control *fc) add_to_list(form->items, fc); } +#ifdef CONFIG_DEBUG +/** Assert that each form in the list has a different form.form_num + * ... form.form_end range and that the ranges are contiguous and + * together cover all numbers from 0 to INT_MAX. Alternatively, the + * whole list may be empty. This function can be called from a + * debugger, or automatically from some places. + * + * This function may leave assert_failed = 1; the caller must use + * if_assert_failed. */ +static void +assert_forms_list_ok(LIST_OF(struct form) *forms) +{ + int saw_form_num_0 = 0; + struct form *outer; + + if (list_empty(*forms)) return; + + /* O(n^2) algorithm, but it's only for debugging. */ + foreach (outer, *forms) { + int followers = 0; + struct form *inner; + + if (outer->form_num == 0) + saw_form_num_0++; + + foreach (inner, *forms) { + assert(inner == outer + || inner->form_num > outer->form_end + || outer->form_num > inner->form_end); + if (outer->form_end == inner->form_num - 1) + followers++; + } + + if (outer->form_end == INT_MAX) + assert(followers == 0); + else + assert(followers == 1); + } + + assert(saw_form_num_0 == 1); +} +#else /* !CONFIG_DEBUG */ +# define assert_forms_list_ok(forms) ((void) 0) +#endif /* !CONFIG_DEBUG */ + /* Reparents form items based on position in the source. */ void check_html_form_hierarchy(struct part *part) @@ -1842,6 +1901,9 @@ check_html_form_hierarchy(struct part *part) if (list_empty(document->forms)) return; + assert_forms_list_ok(&document->forms); + if_assert_failed {} + /* Take out all badly placed form items. */ foreach (form, document->forms) { @@ -1863,8 +1925,8 @@ check_html_form_hierarchy(struct part *part) foreachsafe (fc, next, form_controls) { foreach (form, document->forms) { - if (form->form_num <= fc->position - && fc->position <= form->form_end) + if (fc->position < form->form_num + || form->form_end < fc->position) continue; fc->form = form; From e213a91badb154b4ed169e1b1f65752d0514ee37 Mon Sep 17 00:00:00 2001 From: Witold Filipczyk Date: Sun, 20 Jul 2008 12:01:49 +0200 Subject: [PATCH 30/32] 1034: Fixed deflate decompression. First try decompress in zlib format. If this fails, restart with the raw deflate. Works for both blogs.msdn.com and for URL of the bug 1034. --- src/encoding/deflate.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/encoding/deflate.c b/src/encoding/deflate.c index 4d76ed50..c166b269 100644 --- a/src/encoding/deflate.c +++ b/src/encoding/deflate.c @@ -30,6 +30,7 @@ struct deflate_enc_data { int fdread; unsigned int last_read:1; + unsigned int after_first_read:1; /* A buffer for data that has been read from the file but not * yet decompressed. z_stream.next_in and z_stream.avail_in @@ -59,6 +60,7 @@ deflate_open(int window_size, struct stream_encoded *stream, int fd) copy_struct(&data->deflate_stream, &null_z_stream); data->fdread = fd; data->last_read = 0; + data->after_first_read = 0; err = inflateInit2(&data->deflate_stream, window_size); if (err != Z_OK) { @@ -70,12 +72,14 @@ deflate_open(int window_size, struct stream_encoded *stream, int fd) return 0; } +#if 0 static int deflate_raw_open(struct stream_encoded *stream, int fd) { /* raw DEFLATE with neither zlib nor gzip header */ return deflate_open(-MAX_WBITS, stream, fd); } +#endif static int deflate_gzip_open(struct stream_encoded *stream, int fd) @@ -89,6 +93,7 @@ deflate_read(struct stream_encoded *stream, unsigned char *buf, int len) { struct deflate_enc_data *data = (struct deflate_enc_data *) stream->data; int err = 0; + int l; if (!data) return -1; @@ -101,7 +106,7 @@ deflate_read(struct stream_encoded *stream, unsigned char *buf, int len) do { if (data->deflate_stream.avail_in == 0) { - int l = safe_read(data->fdread, data->buf, + l = safe_read(data->fdread, data->buf, ELINKS_DEFLATE_BUFFER_LENGTH); if (l == -1) { @@ -117,7 +122,19 @@ deflate_read(struct stream_encoded *stream, unsigned char *buf, int len) data->deflate_stream.next_in = data->buf; data->deflate_stream.avail_in = l; } +restart: err = inflate(&data->deflate_stream, Z_SYNC_FLUSH); + if (err == Z_DATA_ERROR && !data->after_first_read) { + data->after_first_read = 1; + inflateEnd(&data->deflate_stream); + data->deflate_stream.avail_out = len; + data->deflate_stream.next_out = buf; + data->deflate_stream.next_in = data->buf; + data->deflate_stream.avail_in = l; + err = inflateInit2(&data->deflate_stream, -MAX_WBITS); + if (err == Z_OK) goto restart; + } + data->after_first_read = 1; if (err == Z_STREAM_END) { data->last_read = 1; break; @@ -211,7 +228,7 @@ static const unsigned char *const deflate_extensions[] = { NULL }; const struct decoding_backend deflate_decoding_backend = { "deflate", deflate_extensions, - deflate_raw_open, + deflate_gzip_open, deflate_read, deflate_raw_decode_buffer, deflate_close, From b9d48ad7e8c3d59d3eb07a5c6cd9ece829506145 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 20 Jul 2008 14:32:56 +0300 Subject: [PATCH 31/32] 1034: Initialize l in deflate_read to shut up GCC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid this warning: [CC] src/encoding/deflate.o cc1: warnings being treated as errors /home/Kalle/src/elinks-0.12/src/encoding/deflate.c: In function ‘deflate_read’: /home/Kalle/src/elinks-0.12/src/encoding/deflate.c:96: warning: ‘l’ may be used uninitialized in this function --- src/encoding/deflate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/encoding/deflate.c b/src/encoding/deflate.c index c166b269..f4b97f2a 100644 --- a/src/encoding/deflate.c +++ b/src/encoding/deflate.c @@ -93,7 +93,7 @@ deflate_read(struct stream_encoded *stream, unsigned char *buf, int len) { struct deflate_enc_data *data = (struct deflate_enc_data *) stream->data; int err = 0; - int l; + int l = 0; if (!data) return -1; From 327fc1e46ec2d1e515ab82ae2e05117734be68dc Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sun, 20 Jul 2008 14:30:18 +0300 Subject: [PATCH 32/32] 1034: NEWS, comments, and tests --- NEWS | 2 ++ src/encoding/deflate.c | 20 ++++++++++++++++++++ test/cgi/chunked_deflate.py | 8 +++----- test/cgi/chunked_raw_deflate.py | 22 ++++++++++++++++++++++ 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100755 test/cgi/chunked_raw_deflate.py diff --git a/NEWS b/NEWS index 8f0000ff..d326dcef 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,8 @@ Bugs that should be removed from NEWS before the 0.12.0 release: was the first release that had these bugs. * bug 1033: Fix memory leak in ECMAScript window.open. ELinks 0.12pre1 was the first release that had this bug. +* bug 1034: ``Content-Encoding: deflate'' allows a zlib header as + specified in RFC 2616. * Global ECMAScript functions alert, open, and setTimeout again work with SEE. ELinks 0.12pre1 was the first release that supported SEE at all. diff --git a/src/encoding/deflate.c b/src/encoding/deflate.c index f4b97f2a..aae2e299 100644 --- a/src/encoding/deflate.c +++ b/src/encoding/deflate.c @@ -125,6 +125,26 @@ deflate_read(struct stream_encoded *stream, unsigned char *buf, int len) restart: err = inflate(&data->deflate_stream, Z_SYNC_FLUSH); if (err == Z_DATA_ERROR && !data->after_first_read) { + /* RFC 2616 requires a zlib header for + * "Content-Encoding: deflate", but some HTTP + * servers (Microsoft-IIS/6.0 at blogs.msdn.com, + * and reportedly Apache with mod_deflate) omit + * that, causing Z_DATA_ERROR. Clarification of + * the term "deflate" has been requested for the + * next version of HTTP: + * http://www3.tools.ietf.org/wg/httpbis/trac/ticket/73 + * + * Try to recover by telling zlib not to expect + * the header. If the error does not happen on + * the first inflate() call, then it is too late + * to recover because ELinks may already have + * discarded part of the input data. + * + * TODO: This fallback to raw DEFLATE is currently + * enabled for "Content-Encoding: gzip" too. It + * might be better to fall back to no compression + * at all, because Apache can send that header for + * uncompressed *.gz.md5 files. */ data->after_first_read = 1; inflateEnd(&data->deflate_stream); data->deflate_stream.avail_out = len; diff --git a/test/cgi/chunked_deflate.py b/test/cgi/chunked_deflate.py index 5300c575..45f06828 100755 --- a/test/cgi/chunked_deflate.py +++ b/test/cgi/chunked_deflate.py @@ -1,11 +1,9 @@ #!/usr/bin/env python -import os, time -from zlib import * +import os, time, zlib data1 = 'Two lines should be visible.
The second line.' -ob = compressobj(Z_DEFAULT_COMPRESSION, DEFLATED, -MAX_WBITS) -cd1 = ob.compress(data1) -cd1 += ob.flush() +cd1 = zlib.compress(data1) + length = len(cd1) next_chunk = hex(length - 10)[2:] diff --git a/test/cgi/chunked_raw_deflate.py b/test/cgi/chunked_raw_deflate.py new file mode 100755 index 00000000..6a762636 --- /dev/null +++ b/test/cgi/chunked_raw_deflate.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python +import os, time +from zlib import * + +# According to section 3.5 of RFC 2616, "Content-Encoding: deflate" +# requires a ZLIB header. However, Microsoft-IIS/6.0 sends a raw +# DEFLATE stream instead. This CGI tests how ELinks handles that. + +data1 = 'Two lines should be visible.
The second line.' +ob = compressobj(Z_DEFAULT_COMPRESSION, DEFLATED, -MAX_WBITS) +cd1 = ob.compress(data1) +cd1 += ob.flush() +length = len(cd1) +next_chunk = hex(length - 10)[2:] + +os.write(1, "Date: Sun, 20 Jan 2008 15:24:00 GMT\r\nServer: ddd\r\nTransfer-Encoding: chunked\r\nContent-Encoding: deflate\r\nConnection: close\r\nContent-Type: text/html; charset=ISO-8859-1\r\n") +os.write(1, "\r\na\r\n") +os.write(1, cd1[:10]) +time.sleep(2) +os.write(1, "\r\n%s\r\n" % next_chunk) +os.write(1, cd1[10:]) +os.write(1, "\r\n0\r\n")