Merge pull request #1051 from herby/bootjs-dnu-refactor

boot.js DNU handling redesign

src/Kernel-Infrastructure.js

@@ -1111,11 +1111,11 @@ fn: function (aString,anArray){
 var self=this;
 return smalltalk.withContext(function($ctx1) { 
 
-		return smalltalk.send(self._jsObject(), aString, anArray);
+		return smalltalk.accessJavaScript(self._jsObject(), aString, anArray);
 	;
 return self}, function($ctx1) {$ctx1.fill(self,"forwardMessage:withArguments:",{aString:aString,anArray:anArray},globals.JSObjectProxy)})},
 args: ["aString", "anArray"],
-source: "forwardMessage: aString withArguments: anArray\x0a\x09<\x0a\x09\x09return smalltalk.send(self._jsObject(), aString, anArray);\x0a\x09>",
+source: "forwardMessage: aString withArguments: anArray\x0a\x09<\x0a\x09\x09return smalltalk.accessJavaScript(self._jsObject(), aString, anArray);\x0a\x09>",
 messageSends: [],
 referencedClasses: []
 }),

src/Kernel-Infrastructure.st

@@ -424,7 +424,7 @@ doesNotUnderstand: aMessage
 
 forwardMessage: aString withArguments: anArray
 	<
-		return smalltalk.send(self._jsObject(), aString, anArray);
+		return smalltalk.accessJavaScript(self._jsObject(), aString, anArray);
 	>
 !
 

src/Kernel-Tests.js

@@ -6087,6 +6087,74 @@ referencedClasses: ["MessageNotUnderstood"]
 }),
 globals.JSObjectProxyTest);
 
+smalltalk.addMethod(
+smalltalk.method({
+selector: "testDNURegression1057",
+protocol: 'tests',
+fn: function (){
+var self=this;
+var jsObject;
+function $Error(){return globals.Error||(typeof Error=="undefined"?nil:Error)}
+return smalltalk.withContext(function($ctx1) { 
+var $1;
+jsObject=[];
+_st(jsObject)._basicAt_put_("allowJavaScriptCalls",true);
+$ctx1.sendIdx["basicAt:put:"]=1;
+_st(jsObject)._basicAt_put_("foo",(3));
+self._shouldnt_raise_((function(){
+return smalltalk.withContext(function($ctx2) {
+return _st(jsObject)._foo();
+$ctx2.sendIdx["foo"]=1;
+}, function($ctx2) {$ctx2.fillBlock({},$ctx1,1)})}),$Error());
+$ctx1.sendIdx["shouldnt:raise:"]=1;
+$1=_st(jsObject)._foo();
+$ctx1.sendIdx["foo"]=2;
+self._assert_equals_($1,(3));
+$ctx1.sendIdx["assert:equals:"]=1;
+self._shouldnt_raise_((function(){
+return smalltalk.withContext(function($ctx2) {
+return _st(jsObject)._foo_((4));
+}, function($ctx2) {$ctx2.fillBlock({},$ctx1,2)})}),$Error());
+self._assert_equals_(_st(jsObject)._foo(),(4));
+return self}, function($ctx1) {$ctx1.fill(self,"testDNURegression1057",{jsObject:jsObject},globals.JSObjectProxyTest)})},
+args: [],
+source: "testDNURegression1057\x0a\x09| jsObject |\x0a\x09jsObject := #().\x0a\x09jsObject basicAt: 'allowJavaScriptCalls' put: true.\x0a\x09jsObject basicAt: 'foo' put: 3.\x0a\x09self shouldnt: [ jsObject foo ] raise: Error.\x0a\x09self assert: jsObject foo equals: 3.\x0a\x09self shouldnt: [ jsObject foo: 4 ] raise: Error.\x0a\x09self assert: jsObject foo equals: 4",
+messageSends: ["basicAt:put:", "shouldnt:raise:", "foo", "assert:equals:", "foo:"],
+referencedClasses: ["Error"]
+}),
+globals.JSObjectProxyTest);
+
+smalltalk.addMethod(
+smalltalk.method({
+selector: "testDNURegression1059",
+protocol: 'tests',
+fn: function (){
+var self=this;
+var jsObject;
+function $Error(){return globals.Error||(typeof Error=="undefined"?nil:Error)}
+return smalltalk.withContext(function($ctx1) { 
+jsObject=[];
+_st(jsObject)._basicAt_put_("allowJavaScriptCalls",true);
+$ctx1.sendIdx["basicAt:put:"]=1;
+_st(jsObject)._basicAt_put_("x",(3));
+$ctx1.sendIdx["basicAt:put:"]=2;
+_st(jsObject)._basicAt_put_("x:",(function(){
+return smalltalk.withContext(function($ctx2) {
+return self._error();
+}, function($ctx2) {$ctx2.fillBlock({},$ctx1,1)})}));
+self._shouldnt_raise_((function(){
+return smalltalk.withContext(function($ctx2) {
+return _st(jsObject)._x_((4));
+}, function($ctx2) {$ctx2.fillBlock({},$ctx1,2)})}),$Error());
+self._assert_equals_(_st(jsObject)._x(),(4));
+return self}, function($ctx1) {$ctx1.fill(self,"testDNURegression1059",{jsObject:jsObject},globals.JSObjectProxyTest)})},
+args: [],
+source: "testDNURegression1059\x0a\x09| jsObject |\x0a\x09jsObject := #().\x0a\x09jsObject basicAt: 'allowJavaScriptCalls' put: true.\x0a\x09jsObject basicAt: 'x' put: 3.\x0a\x09jsObject basicAt: 'x:' put: [ self error ].\x0a\x09self shouldnt: [ jsObject x: 4 ] raise: Error.\x0a\x09self assert: jsObject x equals: 4\x0a\x09",
+messageSends: ["basicAt:put:", "error", "shouldnt:raise:", "x:", "assert:equals:", "x"],
+referencedClasses: ["Error"]
+}),
+globals.JSObjectProxyTest);
+
 smalltalk.addMethod(
 smalltalk.method({
 selector: "testDNURegression1062",

src/Kernel-Tests.st

@@ -1746,6 +1746,27 @@ testDNU
 	self should: [ self jsObject foo ] raise: MessageNotUnderstood
 !
 
+testDNURegression1057
+	| jsObject |
+	jsObject := #().
+	jsObject basicAt: 'allowJavaScriptCalls' put: true.
+	jsObject basicAt: 'foo' put: 3.
+	self shouldnt: [ jsObject foo ] raise: Error.
+	self assert: jsObject foo equals: 3.
+	self shouldnt: [ jsObject foo: 4 ] raise: Error.
+	self assert: jsObject foo equals: 4
+!
+
+testDNURegression1059
+	| jsObject |
+	jsObject := #().
+	jsObject basicAt: 'allowJavaScriptCalls' put: true.
+	jsObject basicAt: 'x' put: 3.
+	jsObject basicAt: 'x:' put: [ self error ].
+	self shouldnt: [ jsObject x: 4 ] raise: Error.
+	self assert: jsObject x equals: 4
+!
+
 testDNURegression1062
 	| jsObject stored |
 	jsObject := #().

support/boot.js

@@ -191,29 +191,29 @@ define("amber/boot", [ 'require', './browser-compatibility' ], function (require
 		var methods = [], checker = Object.create(null);
 		this.selectors = [];
 
-		this.get = function (string) {
-			var index = this.selectors.indexOf(string);
+		this.get = function (stSelector) {
+			var index = this.selectors.indexOf(stSelector);
 			if(index !== -1) {
 				return methods[index];
 			}
-			this.selectors.push(string);
-			var selector = st.selector(string);
-			checker[selector] = true;
-			var method = {jsSelector: selector, fn: createHandler(selector)};
+			this.selectors.push(stSelector);
+			var jsSelector = st.selector(stSelector);
+			checker[jsSelector] = true;
+			var method = {jsSelector: jsSelector, fn: createHandler(stSelector)};
 			methods.push(method);
 			manip.installMethod(method, rootAsClass);
 			return method;
 		};
 
-		this.isSelector = function (selector) {
-			return checker[selector];
+		this.isSelector = function (jsSelector) {
+			return checker[jsSelector];
 		};
 
 		/* Dnu handler method */
 
-		function createHandler(selector) {
+		function createHandler(stSelector) {
 			return function() {
-				return brikz.messageSend.messageNotUnderstood(this, selector, arguments);
+				return brikz.messageSend.messageNotUnderstood(this, stSelector, arguments);
 			};
 		}
 
@@ -987,43 +987,44 @@ define("amber/boot", [ 'require', './browser-compatibility' ], function (require
 		/* Handles unhandled errors during message sends */
 		// simply send the message and handle #dnu:
 
-		st.send = function(receiver, selector, args, klass) {
+		st.send = function(receiver, jsSelector, args, klass) {
 			var method;
 			if(receiver === null) {
 				receiver = nil;
 			}
-			method = klass ? klass.fn.prototype[selector] : receiver.klass && receiver[selector];
+			method = klass ? klass.fn.prototype[jsSelector] : receiver.klass && receiver[jsSelector];
 			if(method) {
 				return method.apply(receiver, args);
 			} else {
-				return messageNotUnderstood(receiver, selector, args);
+				return messageNotUnderstood(receiver, st.convertSelector(jsSelector), args);
 			}
 		};
 
-		/* Handles #dnu: *and* JavaScript method calls.
-		 if the receiver has no klass, we consider it a JS object (outside of the
-		 Amber system). Else assume that the receiver understands #doesNotUnderstand: */
-
-		function messageNotUnderstood(receiver, selector, args) {
-			/* Handles JS method calls. */
-			if(receiver.klass === undefined || receiver.allowJavaScriptCalls) {
-				return callJavaScriptMethod(receiver, selector, args);
-			}
-
-			/* Handles not understood messages. Also see the Amber counter-part
-			 Object>>doesNotUnderstand: */
-
+		function invokeDnuMethod(receiver, stSelector, args) {
 			return receiver._doesNotUnderstand_(
 				globals.Message._new()
-					._selector_(st.convertSelector(selector))
+					._selector_(stSelector)
 					._arguments_([].slice.call(args))
 			);
 		}
 
-		/* Call a method of a JS object, or answer a property if it exists.
-		 Else try wrapping a JSObjectProxy around the receiver.
+		/* Handles #dnu: *and* JavaScript method calls.
+		 if the receiver has no klass, we consider it a JS object (outside of the
+		 Amber system). Else assume that the receiver understands #doesNotUnderstand: */
+		function messageNotUnderstood(receiver, stSelector, args) {
+			if (receiver.klass !== undefined && !receiver.allowJavaScriptCalls) {
+				return invokeDnuMethod(receiver, stSelector, args);
+			}
+			/* Call a method of a JS object, or answer a property if it exists.
+			 Else try wrapping a JSObjectProxy around the receiver. */
+			var propertyName = st.st2prop(stSelector);
+			if (!(propertyName in receiver)) {
+				return invokeDnuMethod(globals.JSObjectProxy._on_(receiver), stSelector, args);
+			}
+			return accessJavaScript(receiver, propertyName, args);
+		}
 
-		 If the object property is a function, then call it, except if it starts with
+		/* If the object property is a function, then call it, except if it starts with
 		 an uppercase character (we probably want to answer the function itself in this
 		 case and send it #new from Amber).
 
@@ -1032,24 +1033,19 @@ define("amber/boot", [ 'require', './browser-compatibility' ], function (require
 
 		 Example:
 		 "self do: aBlock with: anObject" -> "self.do(aBlock, anObject)" */
-
-		function callJavaScriptMethod(receiver, selector, args) {
-			var jsSelector = st.st2prop(selector);
-			if (jsSelector in receiver) {
-				var jsProperty = receiver[jsSelector];
-				if (typeof jsProperty === "function" && !/^[A-Z]/.test(jsSelector)) {
-					return jsProperty.apply(receiver, args);
-				} else if (args.length > 0) {
-					receiver[jsSelector] = args[0];
-					return nil;
-				} else {
-					return jsProperty;
-				}
+		function accessJavaScript(receiver, propertyName, args) {
+			var propertyValue = receiver[propertyName];
+			if (typeof propertyValue === "function" && !/^[A-Z]/.test(propertyName)) {
+				return propertyValue.apply(receiver, args);
+			} else if (args.length > 0) {
+				receiver[propertyName] = args[0];
+				return nil;
+			} else {
+				return propertyValue;
 			}
-
-			return st.send(globals.JSObjectProxy._on_(receiver), selector, args);
 		}
 
+		st.accessJavaScript = accessJavaScript;
 		this.messageNotUnderstood = messageNotUnderstood;
 	}
 
@@ -1110,7 +1106,6 @@ define("amber/boot", [ 'require', './browser-compatibility' ], function (require
 		}
 
         st.st2prop = function (stSelector) {
-            if (stSelector[0] === '_') return ''; // TODO revert when #1062 root cause fixed
             var colonPosition = stSelector.indexOf(':');
             return colonPosition === -1 ? stSelector : stSelector.slice(0, colonPosition);
         };