Fixes Valija typeof. Placeholder code towards non-additive monkey patching.

Partial fix for bug #814:
Changes Valija's typeof rule so that "typeof [].push" is "function" in Valija.
Contains code to attempt to fix 953 & 956 -- in order to support
non-additive monkey patching. However, some undiagnosed bug that seems
(at least partially) Rhino specific is causing an undiagnosed hang. As
a temporary kludge, we've temporarily introduced the flag
RHINO_BUG=true, so that we can leave in the code that should fix 953 &
956 and continue to try to figure out why it doesn't work.

[metaweta, 2008-12-11 22:51:51]

http://codereview.appspot.com/10273/diff/1/7
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/10273/diff/1/7#newcode220
Line 220: if (cajita.isFrozen(obj) && typeof obj.call === 'function') {
Should this recurse on obj.call?

[ihab.awad, 2008-12-12 19:57:49]

LGTM with nitpicky comments.

http://codereview.appspot.com/10273/diff/1/6
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/10273/diff/1/6#newcode262
Line 262: // NaNs are the only non-reflective value, i.e., if x !== x,
non-refle*x*ive?

http://codereview.appspot.com/10273/diff/1/6#newcode1252
Line 1252: * If the property was defined by Caja code, then yes. If it was
Complete your favor and s/Caja/Cajita/ here too?

http://codereview.appspot.com/10273/diff/1/6#newcode1268
Line 1268: switch (constr && ! isPrototypical(obj) && constr.typeTag___) {
Is it Kosher to leave a space after a unary op? (Occurs elsewhere too.)

http://codereview.appspot.com/10273/diff/1/6#newcode1282
Line 1282: if (! isRecord(optParent)) { return false; }
Rather do -- if (optParent === /*an*/ Object.prototype) { return false; }

http://codereview.appspot.com/10273/diff/1/6#newcode1333
Line 1333: function inObjective(name, obj) {
Per discussion, maybe we should use another term besides 'objective' since this
concept will be important for Cajita as well (use in scope chains during module
loading). MarkM suggested "significant".

http://codereview.appspot.com/10273/diff/1/6#newcode3101
Line 3101: hasOwnPropertyOf: hasOwnPropertyOf,
Do we need the static functions or can we just use the tamed Object.HOP?

http://codereview.appspot.com/10273/diff/1/7
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/10273/diff/1/7#newcode220
Line 220: if (cajita.isFrozen(obj) && typeof obj.call === 'function') {
At least a TODO saying we should flag these objects (tamings of xo4a) in a more
robust way. MarkM suggests trademarking.

http://codereview.appspot.com/10273/diff/1/3
File tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java (right):

http://codereview.appspot.com/10273/diff/1/3#newcode2139
Line 2139: "try { x=toString; } catch (e) {}" +
Spaces around '='.

http://codereview.appspot.com/10273/diff/1/2
File tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java
(right):

http://codereview.appspot.com/10273/diff/1/2#newcode414
Line 414: assertConsistent("[ (typeof noSuchGlobal), (typeof 's')," +
According to MarkM: the 1st 2 entries are a copy/paste error.

[MarkM, 2008-12-13 02:55:05]

Not yet ready to commit, since this also causes an undiagnosed performance
regression.

http://codereview.appspot.com/10273/diff/1/6
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/10273/diff/1/6#newcode262
Line 262: // NaNs are the only non-reflective value, i.e., if x !== x,
On 2008/12/12 19:57:49, ihab.awad wrote:
> non-refle*x*ive?

Done.

http://codereview.appspot.com/10273/diff/1/6#newcode1252
Line 1252: * If the property was defined by Caja code, then yes. If it was
On 2008/12/12 19:57:49, ihab.awad wrote:
> Complete your favor and s/Caja/Cajita/ here too?

Done.

http://codereview.appspot.com/10273/diff/1/6#newcode1268
Line 1268: switch (constr && ! isPrototypical(obj) && constr.typeTag___) {
On 2008/12/12 19:57:49, ihab.awad wrote:
> Is it Kosher to leave a space after a unary op? (Occurs elsewhere too.)

I was wrong. Our convention is no space after unary operator. I replaced all
suitable occurrences of "! " with "!".

Done

http://codereview.appspot.com/10273/diff/1/6#newcode1282
Line 1282: if (! isRecord(optParent)) { return false; }
On 2008/12/12 19:57:49, ihab.awad wrote:
> Rather do -- if (optParent === /*an*/ Object.prototype) { return false; }

In this and similar contexts, "if (constr.prototype === optParent)" is a fast
test for whether it is an Object.prototype.

Done

http://codereview.appspot.com/10273/diff/1/6#newcode1333
Line 1333: function inObjective(name, obj) {
On 2008/12/12 19:57:49, ihab.awad wrote:
> Per discussion, maybe we should use another term besides 'objective' since
this
> concept will be important for Cajita as well (use in scope chains during
module
> loading). MarkM suggested "significant".

After trying it on for size, we reverted to "objective".

Done (though a better word would still be welcome).

http://codereview.appspot.com/10273/diff/1/6#newcode3101
Line 3101: hasOwnPropertyOf: hasOwnPropertyOf,
On 2008/12/12 19:57:49, ihab.awad wrote:
> Do we need the static functions or can we just use the tamed Object.HOP?

Cajita no longer exports the static hasOwnPropertyOf.
Done.

http://codereview.appspot.com/10273/diff/1/7
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/10273/diff/1/7#newcode220
Line 220: if (cajita.isFrozen(obj) && typeof obj.call === 'function') {
On 2008/12/11 22:51:51, metaweta wrote:
> Should this recurse on obj.call?

Perhaps. But for now I just clarified the comment that the 'call' property must
be a cajita function.

http://codereview.appspot.com/10273/diff/1/7#newcode220
Line 220: if (cajita.isFrozen(obj) && typeof obj.call === 'function') {
On 2008/12/12 19:57:49, ihab.awad wrote:
> At least a TODO saying we should flag these objects (tamings of xo4a) in a
more
> robust way. MarkM suggests trademarking.

Done.

http://codereview.appspot.com/10273/diff/1/3
File tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java (right):

http://codereview.appspot.com/10273/diff/1/3#newcode2139
Line 2139: "try { x=toString; } catch (e) {}" +
On 2008/12/12 19:57:49, ihab.awad wrote:
> Spaces around '='.

Done.

http://codereview.appspot.com/10273/diff/1/2
File tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java
(right):

http://codereview.appspot.com/10273/diff/1/2#newcode414
Line 414: assertConsistent("[ (typeof noSuchGlobal), (typeof 's')," +
On 2008/12/12 19:57:49, ihab.awad wrote:
> According to MarkM: the 1st 2 entries are a copy/paste error.

Done.