Possibly buggy detection of "undefined property" by JS Engine?

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
(This has been discussed in bugzilla
Bug 936007 - JavaScript strict warning:
chrome://global/content/bindings/tree.xml, line 50: reference to
undefined property this.treeBoxObject.columns
https://bugzilla.mozilla.org/show_bug.cgi?id=936007
)


It seems that JS Engine's idea of the situation of printing of
"reference to undefined property" is not quite what typical methods
which many documents suggest can catch the "undefined-ness".

if (typeof( var.property ) === 'undefined') ...

It is possible that there is a straight-foward bug in the handling of
"undefined property" and the printing of the message in JS Engine.

Or, maybe, a property that is defined/used elsewhere sometimes may
trigger different printing behavior (still buggy behavior).

I am posting this to solicit feedback from JS Engine developers.
(If there is a better forum to solicit good feedback on this issue,
please let me know. TIA)

BACKGROUND/BUGGY BEHAVIOR(?)

The case in point is the printed warning from full debug version of TB
(comm-central) after the startup during the run of test suite, |make
mozmill| for TB.

Quote from the log:

    ++DOMWINDOW == 14 (0x4709970) [pid = 29700] [serial = 14] [outer =
(nil)]
    ++DOMWINDOW == 15 (0x519e0e0) [pid = 29700] [serial = 15] [outer =
0x46f5d70]
    ++DOMWINDOW == 16 (0x520e5d0) [pid = 29700] [serial = 16] [outer =
0x46c4500]
*=> JavaScript strict warning:
chrome://global/content/bindings/tree.xml, line 50: reference to
undefined property this.treeBoxObject.columns
    [29700] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal)
failed: file
/REF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp,
line 8674
    [29700] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal)
failed: file
/REF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp,
line 8674


I tracked down the message is printed from
mozilla/toolkit/content/widgets/tree.xml

http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/content/widgets/tree.xml#56

(Note there is a slight change of line number due to the region that is
not included in the final chrome://global/content/bindings/tree.xml )

To track down what is going on, I modified the code in tree.xml as
follows to see if |this.treeBoxObject.columns| is really "undefined".
Also, I threw in a check for known "undefined property" to see
if my checking works or not.

    <implementation implements="nsIDOMXULTreeElement,
nsIDOMXULMultiSelectControlElement">

      <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->

      <property name="columns"
                onget="
                       if (typeof (this.treeBoxObject.columns) === 'undefined')
dump('\n dump UNDEFINED\n');
                       if (typeof (this.treeBoxObject.xxyyz) === 'undefined') dump('\n
dump xxyyz UNDEFINED\n');
                       if (typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('\n
dump XxYyz UNDEFINED \n');
                       dump('\n this.treeBoxObject.columns = ' +
JSON.stringify(this.treeBoxObject.columns) + '; \n' );
                       return this.treeBoxObject.columns;
                      "/>

      <property name="view"


Note that property, |xxyyz| and |XxYyZ|, are not defined and used for
|treeBoxObject| anywhere in the code.  So it should cause the |if(typeof
...) dump(...)| to be executed. They ARE executed as shown in the
quoted log later in this post. (Note, |'undefined'| must be used, and
|undefined| without quotes does not work. I find many conflciting
description about this in the web.)

( BTW, if I try to print ALL of |this.treeBoxObject|, then eventually
I get cyclic data structure error from |JSON.stringify()|. It can not
handle the conversion of circulara data.  Printing only
|this.treeBoxObject.columns| property avoid the printing of circular
data structure issue during the run of |make mozmill| test suite of
C-C TB. )

Now, with the modification, I would want to
get the print out from
> if (typeof (this.treeBoxObject.columns) === 'undefined') dump('\n dump
UNDEFINED\n');
when JS engine prints out "reference to undefined propety
this.treeBoxObject.columns". But the output from |dump(...) | did not
get printed out!

Below is the excerpt of the log from where the original undefined
warning was regularly observed during the run of |make mozmill| test
suite of full debug version C-C TB.

I still see the "undefined property", and please note the dump for the
undefined |xxyyz| and |XxYyz|, but notice the missing dump for
|this.treeBoxObject.columns| itself.

Something is indeed strange.

==== Excerpt of the Log

++DOMWINDOW == 14 (0x3d2e780) [pid = 10451] [serial = 14] [outer = (nil)]
++DOMWINDOW == 15 (0x47c34b0) [pid = 10451] [serial = 15] [outer =
0x3d1ab80]
++DOMWINDOW == 16 (0x4833b50) [pid = 10451] [serial = 16] [outer =
0x3ce9310]
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];
JavaScript strict warning: chrome://global/content/bindings/tree.xml,
line 50: reference to undefined property this.treeBoxObject.columns
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];

 dump xxyyz UNDEFINED
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];

 dump XxYyz UNDEFINED
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];

 this.treeBoxObject.columns =
{"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];

 dump xxyyz UNDEFINED
 treeBoxObject.QI= [object BoxObject @ 0x2ba7bb0 (native @ 0x4405418)];

 dump XxYyz UNDEFINED

---

Before I submit a bug for JS engine,
I am soliciting comment from javascript developers.

Any ideas about the strange behavior?

I looked at the code to check for undefined property in

http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jsobj.cpp#4465

There seem to be many exceptions, and I wonder if particular execution
of full debug version of TB is hitting a situation which needs to be
handled as an exceptional case or something.

TIA
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

Jason Orendorff-2
On 4/23/14, 9:42 AM, ISHIKAWA, Chiaki wrote:
> It seems that JS Engine's idea of the situation of printing of
> "reference to undefined property" is not quite what typical methods
> which many documents suggest can catch the "undefined-ness".
>
> if (typeof( var.property ) === 'undefined') ...

You can just write `if (var.property === undefined)`.

Or better: `if (!("property" in var))`.

> I am posting this to solicit feedback from JS Engine developers.
> (If there is a better forum to solicit good feedback on this issue,
> please let me know. TIA)

You're in the right place.



> To track down what is going on, I modified the code in tree.xml as
> follows to see if |this.treeBoxObject.columns| is really "undefined".
> Also, I threw in a check for known "undefined property" to see
> if my checking works or not.
>
>     <implementation implements="nsIDOMXULTreeElement,
> nsIDOMXULMultiSelectControlElement">
>
>       <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->
>
>       <property name="columns"
> onget="
>       if (typeof (this.treeBoxObject.columns) === 'undefined')
> dump('\n dump UNDEFINED\n');
>       if (typeof (this.treeBoxObject.xxyyz) === 'undefined') dump('\n
> dump xxyyz UNDEFINED\n');
>       if (typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('\n
> dump XxYyz UNDEFINED \n');
>       dump('\n this.treeBoxObject.columns = ' +
> JSON.stringify(this.treeBoxObject.columns) + '; \n' );
>       return this.treeBoxObject.columns;
>      "/>
>
>       <property name="view"
>
>
> Note that property, |xxyyz| and |XxYyZ|, are not defined and used for
> |treeBoxObject| anywhere in the code.  So it should cause the |if(typeof
> ...) dump(...)| to be executed. They ARE executed as shown in the
> quoted log later in this post. (Note, |'undefined'| must be used, and
> |undefined| without quotes does not work. I find many conflciting
> description about this in the web.)

The result of the `typeof` operator is always a string.

Here is the standard that specifies it:
http://www.ecma-international.org/ecma-262/5.1/#sec-11.4.3

The special value undefined and the string "undefined" are two different
values in JavaScript, just as the Math object is very different from the
string "Math".


> ( BTW, if I try to print ALL of |this.treeBoxObject|, then eventually
> I get cyclic data structure error from |JSON.stringify()|. It can not
> handle the conversion of circulara data.

That's right. JSON does not support cyclic data, and the standard
requires JSON.stringify to throw an exception if given cyclic data. To
work around this, you can use the uneval() builtin-function instead of
JSON:  dump(uneval(this.treeBoxObject)).


>   Printing only
> |this.treeBoxObject.columns| property avoid the printing of circular
> data structure issue during the run of |make mozmill| test suite of
> C-C TB. )
>
> Now, with the modification, I would want to
> get the print out from
>> if (typeof (this.treeBoxObject.columns) === 'undefined') dump('\n dump
> UNDEFINED\n');
> when JS engine prints out "reference to undefined propety
> this.treeBoxObject.columns". But the output from |dump(...) | did not
> get printed out!

Hmm. It's strange that the warning appears before the dump() output in
the log.

> Before I submit a bug for JS engine,
> I am soliciting comment from javascript developers.
>
> Any ideas about the strange behavior?

Nothing so far. One thing you could try, to get more information, is:

    for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
      dump('searching for columns: ' +
           uneval(Object.getOwnPropertyDescriptor(o, 'columns')));
    }

If you do file a bug, be sure to include full steps to reproduce,
including the hg repo and revision number, and the configure and build
command lines you're using. Thanks!

-j
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
In reply to this post by ISHIKAWA,chiaki
Sorry for top-posting:

Here is the latest update.

I modified the source and obtained interesting output.

My version of the source.

Please note that comm-central has a copy of M-C under ./mozilla
subdirectory and I am patching the tree.xml there to produce the detailed log.

Versions of C-C
/extra/ishikawa/download/TB-3HG/NEW-COMMSRC
hg identify
ad5566786bff+ declare-folderNameField.patch/qtip/tip

(M-C portion under C-C : stored in ./mozilla subdirectory)
/extra/ishikawa/download/TB-3HG/NEW-COMMSRC/mozilla
hg identify
d6b6517e7865+ improve-NSRESULT_FOR_ERRNO-macro.patch/qtip/tip

I have applied local patches, but I believe they don't change the JS
Engine behavior at all. Most of them are about stricter checking of
low-level I/O system calls and some known issues of .js files in
mailer.

I inserted the dump statement as suggested, and also
inserted the new type of checking if(!('propertyname' in var)) to see
it works OK. (It does).

The changed code.

    <implementation implements="nsIDOMXULTreeElement,
nsIDOMXULMultiSelectControlElement">

      <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->

      <property name="columns"
                onget="
       if( typeof (this.treeBoxObject.columns) === 'undefined')
dump('columns UNDEFINED\n');
       if( ! ('columns' in this.treeBoxObject )) dump('columns UNDEFINED 2\n');

       if( typeof (this.treeBoxObject.xxyyz) === 'undefined') dump('xxyyz
UNDEFINED\n');
       if( ! ('xxyyz' in this.treeBoxObject )) dump('xxyyz UNDEFINED 2\n');

       if( typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('XxYyz
UNDEFINED\n');
       if( ! ('XxYyz' in this.treeBoxObject)) dump('XxYyz UNDEFINED 2\n');

       dump('\n');
       for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
        dump('searching for columns using uneval: ' +
        uneval(Object.getOwnPropertyDescriptor(o, 'columns')) +'\n');
        }
       dump('\n this.treeBoxObject.columns using JSON= ' +
                       JSON.stringify(this.treeBoxObject.columns) + ';
                       \n' );
       return this.treeBoxObject.columns;"/>

      <property name="view"
                onget="dump('\n this.treeBoxObject= ' + JSON.stringify(this.treeBoxObject)
+ '; \n' );
                       return this.treeBoxObject.view;"
                onset="return this.treeBoxObject.view = val;"/>

        [...]

      <property name="treeBoxObject"
                onget=" dump('\n treeBoxObject.QueryInterface()= ' +
this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject) + ';
\n'); return
this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject);"
                readonly="true"/>
# contentView is obsolete (see bug 202391)

        [...]


The changed output.

The output from dump() may be buffered and came AFTER the initial and only
"JavaScript strict warning: chrome://global/content/bindings/tree.xml, line
50: reference to undefined property this.treeBoxObject.columns" message in
the log.

The buffering may explain that the output from the dump statements come
after the
strict warning "undefined property" (actually interspersed in this example).

OTOH, I am now not entirely sure of where the strict warning comes from.
Exactly which line? Maybe
>        if( typeof (this.treeBoxObject.columns) === 'undefined') dump('columns UNDEFINED\n');


Strange thing is that JSON.stringify() seems to generate meaningful
string even when this strict "undefined property" warning is printed
by JS Engine and the explicit tests don't seem to work.

I am beginning to understand that whenever |thisBoxObject| is accessed,
somehow the code below is executed and that is why we see the dump of
treeBoxObject.QueryInterface() each time such reference is made. How
interesting!

>      <property name="treeBoxObject"
>                onget=" dump('\n treeBoxObject.QueryInterface()= ' + this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject) + '; \n'); return this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject);"
>                readonly="true"/>

I am a newbie to javascript and unfamiliar with the innards of XBL,
etc. I just happened to know some C, C++ and want to make TB a better
mail client. That is all. Debugging JS Engine is not what I
expected to do in order to fix some TB issues :-)

So there are two such outputs of treeBoxObject.QueryInterface
for explicit checking for the "undefined"-ness of this.treeBoxObject.columns.
Two such outputs for checking non-existing "xxyyz" property".
Two such outputs for checking non-existing "XxYyz" property".
One such output for the reference to treeBoxObject inside the for-loop.
The other one for the JSON.stringify().
Finally the other such output for the reference in the return statement.

Considering the delayed output (due to buffering of dump() possibly),
the counts of expected such output lines do match the output in the
log. So it seems that we can co-relate the dump() statements to that of
real output lines.

However, again please note that NO dump() OUTPUT from

>      if( typeof (this.treeBoxObject.columns) === 'undefined') dump('columns UNDEFINED\n');
>      if( ! ('columns' in this.treeBoxObject )) dump('columns UNDEFINED 2\n');

is seen at all! So the explicit check code above does not think 'columns'
property is undefined even though the JS Engine printed strict warning .
>JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns

Since the particular strict warning is printed only once during the
execution of the program initially (and no warning is generated thereafter).
Is there an uninitialized variable issue or something???

=== Log

The excerpt from the log (where the undefined property strict warning is
usually printed during the startup).


++DOCSHELL 0xb980178 == 11 [pid = 26628] [id = 11]
++DOMWINDOW == 14 (0xb9807f8) [pid = 26628] [serial = 14] [outer = (nil)]
++DOMWINDOW == 15 (0xb9b1d98) [pid = 26628] [serial = 15] [outer = 0xb971db8]
++DOMWINDOW == 16 (0xc1efa30) [pid = 26628] [serial = 16] [outer = 0xb963ea8]

 [CI comment. the following two treeBoxObject.QueryInterface are mingled with
  Javascript strict warning: line, but it is likely to be the result of
  buffered output and non-buffered output (stdout vs stderr?). These
  two outputs of QueryInterface are from the explicit if() check for
  undefinedness of |columns| property.]

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
JavaScript strict warning: chrome://global/content/bindings/tree.xml, line
50: reference to undefined property this.treeBoxObject.columns

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
xxyyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
xxyyz UNDEFINED 2

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
XxYyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
XxYyz UNDEFINED 2


 [CI comment: next QueryInterface dump is the reference to treeBoxObject in
  for-loop initialization part.]
 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
searching for columns using uneval: ({configurable:true, enumerable:true,
get:function columns() {
    [native code]
}, set:(void 0)})
searching for columns using uneval: (void 0)
searching for columns using uneval: (void 0)

 [CI comment: the following QueryInterface dump is from the reference to
  treeBoxObject in the reference in JSON.stringify().]
 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

 this.treeBoxObject.columns using JSON=
{"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};

 [CI comment: following QueryInterface() output is to the reference of
  treeBoxObject in the return statement.]

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

  [CI comment:The following two outputs of QueryInterface are from the
  explicit if() check for undefinedness of |columns| property.]

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
xxyyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
xxyyz UNDEFINED 2

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
XxYyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
XxYyz UNDEFINED 2


 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];
searching for columns using uneval: ({configurable:true, enumerable:true,
get:function columns() {
    [native code]
}, set:(void 0)})
searching for columns using uneval: (void 0)
searching for columns using uneval: (void 0)

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

 this.treeBoxObject.columns using JSON=
{"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
0xbeb447c)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
xxyyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
xxyyz UNDEFINED 2

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
XxYyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
XxYyz UNDEFINED 2


 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
searching for columns using uneval: ({configurable:true, enumerable:true,
get:function columns() {
    [native code]
}, set:(void 0)})
searching for columns using uneval: (void 0)
searching for columns using uneval: (void 0)

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 this.treeBoxObject.columns using JSON= {"0":{},"folderNameCol":{}};

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
xxyyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
xxyyz UNDEFINED 2

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
XxYyz UNDEFINED

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
XxYyz UNDEFINED 2


 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
searching for columns using uneval: ({configurable:true, enumerable:true,
get:function columns() {
    [native code]
}, set:(void 0)})
searching for columns using uneval: (void 0)
searching for columns using uneval: (void 0)

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];

 this.treeBoxObject.columns using JSON= {"0":{},"folderNameCol":{}};

 treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
0xbe1fca4)];
GetProp:98: nbytes=64
[26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
file
/new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
line 8687
[26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
file
/new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
line 8687
[26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
file
/new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
line 8687
[26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
file
/new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
line 8687
[26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
file
/new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
line 8687

=== End Log

If you can find anything strange about the output using uneval(), please let
me know.

Unless there is a glaring and easy to spot mistake on my end,
I think  I do need to file a bug for JS Engine's handling of an undefined
property (once for the first time it is accessed? during a program run.)

Exact step is to
 - create a full debug version of TB.
 - Run the |make mozmill| test
   after moving to MOZ_OBJ/mozilla

 - Specifically, to cut down execution time, run make mozmill only for
   a single target
   cd MOZ_OBJ
   make  SOLO_TEST=search-window/test-search-window.js mozmill-one

  - Observe this strange "undefined property message".

Any comment and observation about the strange behavior is appreciated.

There are other undefined property message lines in |make mozmill|.
They may be also false positives.

TIA

On (2014年04月24日 06:57), Jason Orendorff wrote:

> On 4/23/14, 9:42 AM, ISHIKAWA, Chiaki wrote:
>> It seems that JS Engine's idea of the situation of printing of
>> "reference to undefined property" is not quite what typical methods
>> which many documents suggest can catch the "undefined-ness".
>>
>> if (typeof( var.property ) === 'undefined') ...
>
> You can just write `if (var.property === undefined)`.
>
> Or better: `if (!("property" in var))`.
>
>> I am posting this to solicit feedback from JS Engine developers.
>> (If there is a better forum to solicit good feedback on this issue,
>> please let me know. TIA)
>
> You're in the right place.
>
>
>
>> To track down what is going on, I modified the code in tree.xml as
>> follows to see if |this.treeBoxObject.columns| is really "undefined".
>> Also, I threw in a check for known "undefined property" to see
>> if my checking works or not.
>>
>>     <implementation implements="nsIDOMXULTreeElement,
>> nsIDOMXULMultiSelectControlElement">
>>
>>       <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->
>>
>>       <property name="columns"
>> onget="
>>       if (typeof (this.treeBoxObject.columns) === 'undefined')
>> dump('\n dump UNDEFINED\n');
>>       if (typeof (this.treeBoxObject.xxyyz) === 'undefined') dump('\n
>> dump xxyyz UNDEFINED\n');
>>       if (typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('\n
>> dump XxYyz UNDEFINED \n');
>>       dump('\n this.treeBoxObject.columns = ' +
>> JSON.stringify(this.treeBoxObject.columns) + '; \n' );
>>       return this.treeBoxObject.columns;
>>      "/>
>>
>>       <property name="view"
>>
>>
>> Note that property, |xxyyz| and |XxYyZ|, are not defined and used for
>> |treeBoxObject| anywhere in the code.  So it should cause the |if(typeof
>> ...) dump(...)| to be executed. They ARE executed as shown in the
>> quoted log later in this post. (Note, |'undefined'| must be used, and
>> |undefined| without quotes does not work. I find many conflciting
>> description about this in the web.)
>
> The result of the `typeof` operator is always a string.
>
> Here is the standard that specifies it:
> http://www.ecma-international.org/ecma-262/5.1/#sec-11.4.3
>
> The special value undefined and the string "undefined" are two different
> values in JavaScript, just as the Math object is very different from the
> string "Math".
>
>
>> ( BTW, if I try to print ALL of |this.treeBoxObject|, then eventually
>> I get cyclic data structure error from |JSON.stringify()|. It can not
>> handle the conversion of circulara data.
>
> That's right. JSON does not support cyclic data, and the standard
> requires JSON.stringify to throw an exception if given cyclic data. To
> work around this, you can use the uneval() builtin-function instead of
> JSON:  dump(uneval(this.treeBoxObject)).
>
>
>>   Printing only
>> |this.treeBoxObject.columns| property avoid the printing of circular
>> data structure issue during the run of |make mozmill| test suite of
>> C-C TB. )
>>
>> Now, with the modification, I would want to
>> get the print out from
>>> if (typeof (this.treeBoxObject.columns) === 'undefined') dump('\n dump
>> UNDEFINED\n');
>> when JS engine prints out "reference to undefined propety
>> this.treeBoxObject.columns". But the output from |dump(...) | did not
>> get printed out!
>
> Hmm. It's strange that the warning appears before the dump() output in
> the log.
>
>> Before I submit a bug for JS engine,
>> I am soliciting comment from javascript developers.
>>
>> Any ideas about the strange behavior?
>
> Nothing so far. One thing you could try, to get more information, is:
>
>     for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
>       dump('searching for columns: ' +
>            uneval(Object.getOwnPropertyDescriptor(o, 'columns')));
>     }
>
> If you do file a bug, be sure to include full steps to reproduce,
> including the hg repo and revision number, and the configure and build
> command lines you're using. Thanks!
>
> -j
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
I ran the test mentioned under valgrind/memcheck.

Nothing stood out, so uninitialized variable may not be the cause.
(Still the chance of initialized to "improper value" exists.)

Why did JS Engine print "undefined property" when
check with |'columns' in this.treeBoxObject| figures columns thinks it
is defined, and

for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
        dump('searching for columns using uneval: ' +
  uneval(Object.getOwnPropertyDescriptor(o, 'columns')) +'\n');

printed out the following !?

---
searching for columns using uneval: ({configurable:true, enumerable:true,
get:function columns() {
     [native code]
}, set:(void 0)})
searching for columns using uneval: (void 0)
searching for columns using uneval: (void 0)
---


And the warning is printed only on the first access, it seems.
(Subsequent similar values did not trigger the warning. Oh wait, is it
possible JS Engine sets some flag not to print the warning for the same
code or property many times?).

TIA

(2014/04/24 15:08), ishikawa wrote:

> Sorry for top-posting:
>
> Here is the latest update.
>
> I modified the source and obtained interesting output.
>
> My version of the source.
>
> Please note that comm-central has a copy of M-C under ./mozilla
> subdirectory and I am patching the tree.xml there to produce the detailed log.
>
> Versions of C-C
> /extra/ishikawa/download/TB-3HG/NEW-COMMSRC
> hg identify
> ad5566786bff+ declare-folderNameField.patch/qtip/tip
>
> (M-C portion under C-C : stored in ./mozilla subdirectory)
> /extra/ishikawa/download/TB-3HG/NEW-COMMSRC/mozilla
> hg identify
> d6b6517e7865+ improve-NSRESULT_FOR_ERRNO-macro.patch/qtip/tip
>
> I have applied local patches, but I believe they don't change the JS
> Engine behavior at all. Most of them are about stricter checking of
> low-level I/O system calls and some known issues of .js files in
> mailer.
>
> I inserted the dump statement as suggested, and also
> inserted the new type of checking if(!('propertyname' in var)) to see
> it works OK. (It does).
>
> The changed code.
>
>      <implementation implements="nsIDOMXULTreeElement,
> nsIDOMXULMultiSelectControlElement">
>
>        <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->
>
>        <property name="columns"
> onget="
>         if( typeof (this.treeBoxObject.columns) === 'undefined')
> dump('columns UNDEFINED\n');
>         if( ! ('columns' in this.treeBoxObject )) dump('columns UNDEFINED 2\n');
>
>         if( typeof (this.treeBoxObject.xxyyz) === 'undefined') dump('xxyyz
> UNDEFINED\n');
>         if( ! ('xxyyz' in this.treeBoxObject )) dump('xxyyz UNDEFINED 2\n');
>
>         if( typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('XxYyz
> UNDEFINED\n');
>         if( ! ('XxYyz' in this.treeBoxObject)) dump('XxYyz UNDEFINED 2\n');
>
>         dump('\n');
>         for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
> dump('searching for columns using uneval: ' +
> uneval(Object.getOwnPropertyDescriptor(o, 'columns')) +'\n');
> }
>         dump('\n this.treeBoxObject.columns using JSON= ' +
>       JSON.stringify(this.treeBoxObject.columns) + ';
>       \n' );
>         return this.treeBoxObject.columns;"/>
>
>        <property name="view"
> onget="dump('\n this.treeBoxObject= ' + JSON.stringify(this.treeBoxObject)
> + '; \n' );
>       return this.treeBoxObject.view;"
> onset="return this.treeBoxObject.view = val;"/>
>
> [...]
>
>        <property name="treeBoxObject"
>                  onget=" dump('\n treeBoxObject.QueryInterface()= ' +
> this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject) + ';
> \n'); return
> this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject);"
>                  readonly="true"/>
> # contentView is obsolete (see bug 202391)
>
>          [...]
>
>
> The changed output.
>
> The output from dump() may be buffered and came AFTER the initial and only
> "JavaScript strict warning: chrome://global/content/bindings/tree.xml, line
> 50: reference to undefined property this.treeBoxObject.columns" message in
> the log.
>
> The buffering may explain that the output from the dump statements come
> after the
> strict warning "undefined property" (actually interspersed in this example).
>
> OTOH, I am now not entirely sure of where the strict warning comes from.
> Exactly which line? Maybe
>>         if( typeof (this.treeBoxObject.columns) === 'undefined') dump('columns UNDEFINED\n');
>
>
> Strange thing is that JSON.stringify() seems to generate meaningful
> string even when this strict "undefined property" warning is printed
> by JS Engine and the explicit tests don't seem to work.
>
> I am beginning to understand that whenever |thisBoxObject| is accessed,
> somehow the code below is executed and that is why we see the dump of
> treeBoxObject.QueryInterface() each time such reference is made. How
> interesting!
>
>>       <property name="treeBoxObject"
>>                 onget=" dump('\n treeBoxObject.QueryInterface()= ' + this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject) + '; \n'); return this.boxObject.QueryInterface(Components.interfaces.nsITreeBoxObject);"
>>                 readonly="true"/>
>
> I am a newbie to javascript and unfamiliar with the innards of XBL,
> etc. I just happened to know some C, C++ and want to make TB a better
> mail client. That is all. Debugging JS Engine is not what I
> expected to do in order to fix some TB issues :-)
>
> So there are two such outputs of treeBoxObject.QueryInterface
> for explicit checking for the "undefined"-ness of this.treeBoxObject.columns.
> Two such outputs for checking non-existing "xxyyz" property".
> Two such outputs for checking non-existing "XxYyz" property".
> One such output for the reference to treeBoxObject inside the for-loop.
> The other one for the JSON.stringify().
> Finally the other such output for the reference in the return statement.
>
> Considering the delayed output (due to buffering of dump() possibly),
> the counts of expected such output lines do match the output in the
> log. So it seems that we can co-relate the dump() statements to that of
> real output lines.
>
> However, again please note that NO dump() OUTPUT from
>
>>       if( typeof (this.treeBoxObject.columns) === 'undefined') dump('columns UNDEFINED\n');
>>       if( ! ('columns' in this.treeBoxObject )) dump('columns UNDEFINED 2\n');
>
> is seen at all! So the explicit check code above does not think 'columns'
> property is undefined even though the JS Engine printed strict warning .
>> JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns
>
> Since the particular strict warning is printed only once during the
> execution of the program initially (and no warning is generated thereafter).
> Is there an uninitialized variable issue or something???
>
> === Log
>
> The excerpt from the log (where the undefined property strict warning is
> usually printed during the startup).
>
>
> ++DOCSHELL 0xb980178 == 11 [pid = 26628] [id = 11]
> ++DOMWINDOW == 14 (0xb9807f8) [pid = 26628] [serial = 14] [outer = (nil)]
> ++DOMWINDOW == 15 (0xb9b1d98) [pid = 26628] [serial = 15] [outer = 0xb971db8]
> ++DOMWINDOW == 16 (0xc1efa30) [pid = 26628] [serial = 16] [outer = 0xb963ea8]
>
>   [CI comment. the following two treeBoxObject.QueryInterface are mingled with
>    Javascript strict warning: line, but it is likely to be the result of
>    buffered output and non-buffered output (stdout vs stderr?). These
>    two outputs of QueryInterface are from the explicit if() check for
>    undefinedness of |columns| property.]
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> JavaScript strict warning: chrome://global/content/bindings/tree.xml, line
> 50: reference to undefined property this.treeBoxObject.columns
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> xxyyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> xxyyz UNDEFINED 2
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> XxYyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> XxYyz UNDEFINED 2
>
>
>   [CI comment: next QueryInterface dump is the reference to treeBoxObject in
>    for-loop initialization part.]
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> searching for columns using uneval: ({configurable:true, enumerable:true,
> get:function columns() {
>      [native code]
> }, set:(void 0)})
> searching for columns using uneval: (void 0)
> searching for columns using uneval: (void 0)
>
>   [CI comment: the following QueryInterface dump is from the reference to
>    treeBoxObject in the reference in JSON.stringify().]
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>   this.treeBoxObject.columns using JSON=
> {"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};
>
>   [CI comment: following QueryInterface() output is to the reference of
>    treeBoxObject in the return statement.]
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>    [CI comment:The following two outputs of QueryInterface are from the
>    explicit if() check for undefinedness of |columns| property.]
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> xxyyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> xxyyz UNDEFINED 2
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> XxYyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> XxYyz UNDEFINED 2
>
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
> searching for columns using uneval: ({configurable:true, enumerable:true,
> get:function columns() {
>      [native code]
> }, set:(void 0)})
> searching for columns using uneval: (void 0)
> searching for columns using uneval: (void 0)
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>   this.treeBoxObject.columns using JSON=
> {"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xab79b98 (native @
> 0xbeb447c)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> xxyyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> xxyyz UNDEFINED 2
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> XxYyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> XxYyz UNDEFINED 2
>
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> searching for columns using uneval: ({configurable:true, enumerable:true,
> get:function columns() {
>      [native code]
> }, set:(void 0)})
> searching for columns using uneval: (void 0)
> searching for columns using uneval: (void 0)
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   this.treeBoxObject.columns using JSON= {"0":{},"folderNameCol":{}};
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> xxyyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> xxyyz UNDEFINED 2
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> XxYyz UNDEFINED
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> XxYyz UNDEFINED 2
>
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> searching for columns using uneval: ({configurable:true, enumerable:true,
> get:function columns() {
>      [native code]
> }, set:(void 0)})
> searching for columns using uneval: (void 0)
> searching for columns using uneval: (void 0)
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
>
>   this.treeBoxObject.columns using JSON= {"0":{},"folderNameCol":{}};
>
>   treeBoxObject.QueryInterface()= [object BoxObject @ 0xc26f368 (native @
> 0xbe1fca4)];
> GetProp:98: nbytes=64
> [26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
> file
> /new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
> line 8687
> [26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
> file
> /new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
> line 8687
> [26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
> file
> /new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
> line 8687
> [26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
> file
> /new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
> line 8687
> [26628] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed:
> file
> /new-hd1/extra/ishikawa/TB-3HG/NEW-COMMSRC/mozilla/docshell/base/nsDocShell.cpp,
> line 8687
>
> === End Log
>
> If you can find anything strange about the output using uneval(), please let
> me know.
>
> Unless there is a glaring and easy to spot mistake on my end,
> I think  I do need to file a bug for JS Engine's handling of an undefined
> property (once for the first time it is accessed? during a program run.)
>
> Exact step is to
>   - create a full debug version of TB.
>   - Run the |make mozmill| test
>     after moving to MOZ_OBJ/mozilla
>
>   - Specifically, to cut down execution time, run make mozmill only for
>     a single target
>     cd MOZ_OBJ
>     make  SOLO_TEST=search-window/test-search-window.js mozmill-one
>
>    - Observe this strange "undefined property message".
>
> Any comment and observation about the strange behavior is appreciated.
>
> There are other undefined property message lines in |make mozmill|.
> They may be also false positives.
>
> TIA
>
> On (2014年04月24日 06:57), Jason Orendorff wrote:
>> On 4/23/14, 9:42 AM, ISHIKAWA, Chiaki wrote:
>>> It seems that JS Engine's idea of the situation of printing of
>>> "reference to undefined property" is not quite what typical methods
>>> which many documents suggest can catch the "undefined-ness".
>>>
>>> if (typeof( var.property ) === 'undefined') ...
>>
>> You can just write `if (var.property === undefined)`.
>>
>> Or better: `if (!("property" in var))`.
>>
>>> I am posting this to solicit feedback from JS Engine developers.
>>> (If there is a better forum to solicit good feedback on this issue,
>>> please let me know. TIA)
>>
>> You're in the right place.
>>
>>
>>
>>> To track down what is going on, I modified the code in tree.xml as
>>> follows to see if |this.treeBoxObject.columns| is really "undefined".
>>> Also, I threw in a check for known "undefined property" to see
>>> if my checking works or not.
>>>
>>>      <implementation implements="nsIDOMXULTreeElement,
>>> nsIDOMXULMultiSelectControlElement">
>>>
>>>        <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->
>>>
>>>        <property name="columns"
>>> onget="
>>>       if (typeof (this.treeBoxObject.columns) === 'undefined')
>>> dump('\n dump UNDEFINED\n');
>>>       if (typeof (this.treeBoxObject.xxyyz) === 'undefined') dump('\n
>>> dump xxyyz UNDEFINED\n');
>>>       if (typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('\n
>>> dump XxYyz UNDEFINED \n');
>>>       dump('\n this.treeBoxObject.columns = ' +
>>> JSON.stringify(this.treeBoxObject.columns) + '; \n' );
>>>       return this.treeBoxObject.columns;
>>>      "/>
>>>
>>>        <property name="view"
>>>
>>>
>>> Note that property, |xxyyz| and |XxYyZ|, are not defined and used for
>>> |treeBoxObject| anywhere in the code.  So it should cause the |if(typeof
>>> ...) dump(...)| to be executed. They ARE executed as shown in the
>>> quoted log later in this post. (Note, |'undefined'| must be used, and
>>> |undefined| without quotes does not work. I find many conflciting
>>> description about this in the web.)
>>
>> The result of the `typeof` operator is always a string.
>>
>> Here is the standard that specifies it:
>> http://www.ecma-international.org/ecma-262/5.1/#sec-11.4.3
>>
>> The special value undefined and the string "undefined" are two different
>> values in JavaScript, just as the Math object is very different from the
>> string "Math".
>>
>>
>>> ( BTW, if I try to print ALL of |this.treeBoxObject|, then eventually
>>> I get cyclic data structure error from |JSON.stringify()|. It can not
>>> handle the conversion of circulara data.
>>
>> That's right. JSON does not support cyclic data, and the standard
>> requires JSON.stringify to throw an exception if given cyclic data. To
>> work around this, you can use the uneval() builtin-function instead of
>> JSON:  dump(uneval(this.treeBoxObject)).
>>
>>
>>>    Printing only
>>> |this.treeBoxObject.columns| property avoid the printing of circular
>>> data structure issue during the run of |make mozmill| test suite of
>>> C-C TB. )
>>>
>>> Now, with the modification, I would want to
>>> get the print out from
>>>> if (typeof (this.treeBoxObject.columns) === 'undefined') dump('\n dump
>>> UNDEFINED\n');
>>> when JS engine prints out "reference to undefined propety
>>> this.treeBoxObject.columns". But the output from |dump(...) | did not
>>> get printed out!
>>
>> Hmm. It's strange that the warning appears before the dump() output in
>> the log.
>>
>>> Before I submit a bug for JS engine,
>>> I am soliciting comment from javascript developers.
>>>
>>> Any ideas about the strange behavior?
>>
>> Nothing so far. One thing you could try, to get more information, is:
>>
>>      for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
>>        dump('searching for columns: ' +
>>             uneval(Object.getOwnPropertyDescriptor(o, 'columns')));
>>      }
>>
>> If you do file a bug, be sure to include full steps to reproduce,
>> including the hg repo and revision number, and the configure and build
>> command lines you're using. Thanks!
>>
>> -j
>>
> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
>



_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

Andrew Sutherland-5
I think you may just be a victim of the FakeTreeBoxObject:
http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2539

It does not define columns.

Andrew
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
(2014/04/26 2:40), Andrew Sutherland wrote:
> I think you may just be a victim of the FakeTreeBoxObject:
> http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2539
>
> It does not define columns.
>
> Andrew

Thank you for the hint.

But, then how did JS Engine print |.columns| when I asked for
the value using JSON.stringify() or for that matter
why v('.columns' in ...) test suceeded.

That was indeed perplexing.

Anyway, do you suggest that if I add
get columns() { return this.domNode.boxObject.columns }

maybe the issue will disappear?

Strange thing is:
there is already function get:function columns in the uneval() dump
as the first element of this.treeBoxObject.

 > searching for columns using uneval: ({configurable:true, enumerable:true,
 > get:function columns() {
 >      [native code]
 > }, set:(void 0)})

But maybe as is suggested, it is in the superclass?
(JSON.stringify() traversed the call hierarchy to access the |.columns|
in the superclass??? Beats me.)

> Why did JS Engine print "undefined property" when
> check with |'columns' in this.treeBoxObject| figures columns thinks it
> is defined, and
>
> for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
> dump('searching for columns using uneval: ' +
>   uneval(Object.getOwnPropertyDescriptor(o, 'columns')) +'\n');
>
> printed out the following !?
>
> ---
> searching for columns using uneval: ({configurable:true, enumerable:true,
> get:function columns() {
>      [native code]
> }, set:(void 0)})
> searching for columns using uneval: (void 0)
> searching for columns using uneval: (void 0)

if |prpertyname in variable| is not in line with the idea of strict
checking of property being defined in JS Engine, I think we are in trouble.

But thank you for the hint.




_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

Andrew Sutherland-5
On 04/26/2014 08:57 PM, ISHIKAWA,chiaki wrote:

> (2014/04/26 2:40), Andrew Sutherland wrote:
>> I think you may just be a victim of the FakeTreeBoxObject:
>> http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2539 
>>
>>
>> It does not define columns.
>>
>> Andrew
>
> Thank you for the hint.
>
> But, then how did JS Engine print |.columns| when I asked for
> the value using JSON.stringify() or for that matter
> why v('.columns' in ...) test suceeded.

The FakeTreeBoxObject is only used in Thunderbird 3-pane tabs that are
in the background, not the foreground.  I am assuming that you are
running your tests against a real tree box object, but the warnings
about undefined access are coming from a FakeTreeBoxObject.  You may
explicitly have addressed this in your messages (which I would like to
apologize for not reading in their entirety as I'm doing a drive-by and
the check is pretty easy) and indeed I do think that if you do this:

> Anyway, do you suggest that if I add
> get columns() { return this.domNode.boxObject.columns }
>
> maybe the issue will disappear?

then yes, I think the problem will disappear.  Or at least it's worth a try!

If that doesn't fix the problem, my guess is wrong and you can ignore me :)

Andrew
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
(2014/04/27 10:40), Andrew Sutherland wrote:

> On 04/26/2014 08:57 PM, ISHIKAWA,chiaki wrote:
>> (2014/04/26 2:40), Andrew Sutherland wrote:
>>> I think you may just be a victim of the FakeTreeBoxObject:
>>> http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2539
>>>
>>>
>>> It does not define columns.
>>>
>>> Andrew
>>
>> Thank you for the hint.
>>
>> But, then how did JS Engine print |.columns| when I asked for
>> the value using JSON.stringify() or for that matter
>> why v('.columns' in ...) test suceeded.
>
> The FakeTreeBoxObject is only used in Thunderbird 3-pane tabs that are
> in the background, not the foreground.  I am assuming that you are
> running your tests against a real tree box object, but the warnings
> about undefined access are coming from a FakeTreeBoxObject.

Actually, I am just running |make mozmill| test suite and find
suspicious error/warning messages and try to eliminate them from the
tests. So I suspect that it is probably the case, but
given that the message is printed during startup, I am not entirely sure
myself what is going on...

> You may
> explicitly have addressed this in your messages (which I would like to
> apologize for not reading in their entirety as I'm doing a drive-by and
> the check is pretty easy) and indeed I do think that if you do this:
>
>> Anyway, do you suggest that if I add
>> get columns() { return this.domNode.boxObject.columns }
>>
>> maybe the issue will disappear?
>
> then yes, I think the problem will disappear.  Or at least it's worth a
> try!
>
> If that doesn't fix the problem, my guess is wrong and you can ignore me :)
>
> Andrew

Andrew,
I inserted |  get columns() { return this.domNode.boxObject.columns},|
as in

   ...
   get nextSibling() {return this.domNode.boxObject.nextSibling},
   get previousSibling() {return this.domNode.boxObject.previousSibling},
* get columns() { return this.domNode.boxObject.columns},

But no change, I still see the following warning ...

--- excerpt from the log---
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];
JavaScript strict warning: chrome://global/content/bindings/tree.xml,
line 50: reference to undefined property this.treeBoxObject.columns
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];

  dump xxyyz UNDEFINED
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];

  dump xxyyz UNDEFINED 2
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];

  dump XxYyz UNDEFINED
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];
searching for columns: ({configurable:true, enumerable:true,
get:function columns() {
     [native code]
}, set:(void 0)})
  treeBoxObject.QI= [object BoxObject @ 0x1b33b10 (native @ 0x3390358)];
--- end excerpt

Getting curious, I searched for the use of |columns| more,
and found that it is referred in
a call to FTBO_stubOutAttributes(),

http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2672
--- call site---
FTBO_stubOutAttributes(FakeTreeBoxObject.prototype, [
   "columns", <=== ****
   "focused",
   "treeBody",
   "rowHeight",
   "rowWidth",
   "horizontalPosition",
   "selectionRegion",
   ]);
-----

The string "stubOut" in the function name looked suspicious.

When I look at the definition of FTBO_stubOutAttributes()
and a comment preceding it (quoted below)
http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2634

I think if TB is calling the above function I inserted (or for that
matter simply accessing |.columns| property against this
fakeTreeBoxObject, I would have obtained a loud warning if I am not
mistaken. (OR, maybe the following function is buggy and masks
the unwanted access?)

/*
  * Provide attribute and function implementations that complain very
loudly if
  *  they are used.  Now, XPConnect will return an error to callers if
we don't
  *  implement part of the interface signature, but this is unlikely to
provide
  *  the visibility we desire.  In fact, since it is a simple nsresult
error,
  *  it may make things completely crazy.  So this way we can yell via dump,
  *  throw an exception, etc.
  */
function FTBO_stubOutAttributes(aObj, aAttribNames) {
   for (let [, attrName] in Iterator(aAttribNames)) {
     let myAttrName = attrName;
     aObj.__defineGetter__(attrName,
       function() {
        let msg = "Read access to stubbed attribute " + myAttrName;
        dump(msg + "\n");
        debugger;
        throw new Error(msg);
       });
     aObj.__defineSetter__(attrName,
       function() {
        let msg = "Write access to stubbed attribute " + myAttrName;
        dump(msg + "\n");
        debugger;
        throw new Error(msg);
       });
   }
}


I am really stumped.

JS Engine has a function which seems to check for "undefined property".

http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jsobj.cpp#4394
template <AllowGC allowGC>
4393 static MOZ_ALWAYS_INLINE bool
4394 GetPropertyHelperInline (...
  [omission]


In it, there are series of exceptions that follows these lines:

4420         /*
4421          * Give a strict warning if foo.bar is evaluated by a
script for an
4422          * object foo with no property named 'bar'.
4423          */
4424         if (vp.isUndefined()) {
4425             jsbytecode *pc = nullptr;
4426             RootedScript script(cx, cx->currentScript(&pc));
4427             if (!pc)
4428                 return true;
4429             JSOp op = (JSOp) *pc;

I excerpt the comment from each if check.

         /* Don't warn if extra warnings not enabled or for random
getprop operations. */
         /* Don't warn repeatedly for the same script. */
         /*
          * Don't warn in self-hosted code (where the further presence of
          * JS::ContextOptions::werror() would result in impossible-to-avoid
          * errors to entirely-innocent client code).
          */
        /* We may just be checking if that object has an iterator. */
        /* Do not warn about tests like (obj[prop] == undefined). */

I am wondering if the particular value of
uneval(Object.getOwnPropertyDescriptor(o, 'columns') printed as follows

({configurable:true, enumerable:true, get:function columns() {
     [native code]
}, set:(void 0)})

needs to be special cased and not recognized as "undefined" in that
function?

To me, seeing |get:function columns() { [native code }| suggests that
|columns| is, certainly, defined as a get (get attr) function (or
whatever is called in JavaScript).

But I am no JS expert. Comment from someone familiar with
|GetPropertyHelperInline| function will shed light on the issue.

Yes, it may be a surprise to find a missing exception handling, but
maybe not many people run full debug version of FF or TB, and for that
matter, many do NOT check the log file of test from what I understand.
So such false positive strict warning may not have been noticed before (?).
BTW, People may be shocked to learn large number of these spurious
error/warning messages that are printed during the run of full debug
build of FF such as |mach mochitests-plain|. The sheer number of these
suspicious warnings/error message makes one wonder what are the expected
ignorable output lines, and what are troublesome output lines that need
serious investigation :-(.

TIA




_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
In reply to this post by Andrew Sutherland-5
Filed a bugzilla:

Bug 1003240 - JS Engine reports FALSE-POSITIVE(?) "strict warning" for
"undefined property" under a certain condition.

https://bugzilla.mozilla.org/show_bug.cgi?id=1003240


_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

Steve Fink-4
In reply to this post by Andrew Sutherland-5
On 04/27/2014 07:51 AM, ISHIKAWA,chiaki wrote:

> Yes, it may be a surprise to find a missing exception handling, but
> maybe not many people run full debug version of FF or TB, and for that
> matter, many do NOT check the log file of test from what I understand.
> So such false positive strict warning may not have been noticed before (?).
> BTW, People may be shocked to learn large number of these spurious
> error/warning messages that are printed during the run of full debug
> build of FF such as |mach mochitests-plain|. The sheer number of these
> suspicious warnings/error message makes one wonder what are the expected
> ignorable output lines, and what are troublesome output lines that need
> serious investigation :-(.

Yes, this is a problem common with much of our code. Apparently people
are reluctant to remove warnings because when something goes wrong,
they're helpful in figuring out what happened. They're sort of like
trace statements.

I'm guessing that in practice, lots of people dump warnings into the
code, then only pay attention to their own and ignore everyone else's.
As you say, this ends up masking the real warnings. It's a nightmarish
situation, highly unfriendly to people getting involved with the code.
Anything you can do to cut out the noise is great (in my opinion).

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

Terrence Cole-3
On 04/30/2014 10:11 AM, Steve Fink wrote:

> On 04/27/2014 07:51 AM, ISHIKAWA,chiaki wrote:
>> Yes, it may be a surprise to find a missing exception handling, but
>> maybe not many people run full debug version of FF or TB, and for that
>> matter, many do NOT check the log file of test from what I understand.
>> So such false positive strict warning may not have been noticed before (?).
>> BTW, People may be shocked to learn large number of these spurious
>> error/warning messages that are printed during the run of full debug
>> build of FF such as |mach mochitests-plain|. The sheer number of these
>> suspicious warnings/error message makes one wonder what are the expected
>> ignorable output lines, and what are troublesome output lines that need
>> serious investigation :-(.
>
> Yes, this is a problem common with much of our code. Apparently people
> are reluctant to remove warnings because when something goes wrong,
> they're helpful in figuring out what happened. They're sort of like
> trace statements.
>
> I'm guessing that in practice, lots of people dump warnings into the
> code, then only pay attention to their own and ignore everyone else's.
> As you say, this ends up masking the real warnings. It's a nightmarish
> situation, highly unfriendly to people getting involved with the code.
> Anything you can do to cut out the noise is great (in my opinion).

Bill added an environment variable at some point to totally squelch
spurious output. I remember there was mail on dev-platform. Does anyone
remember what the variable was called?

> _______________________________________________
> dev-tech-js-engine mailing list
> [hidden email]
> https://lists.mozilla.org/listinfo/dev-tech-js-engine
>
_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

Josh Matthews-4
In reply to this post by Steve Fink-4
On 04/30/2014 01:59 PM, Terrence Cole wrote:
> Bill added an environment variable at some point to totally squelch
> spurious output. I remember there was mail on dev-platform. Does anyone
> remember what the variable was called?

MOZ_QUIET

_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
In reply to this post by ISHIKAWA,chiaki
(2014/04/30 11:39), ISHIKAWA, Chiaki wrote:
> Filed a bugzilla:
>
> Bug 1003240 - JS Engine reports FALSE-POSITIVE(?) "strict warning" for
> "undefined property" under a certain condition.
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=1003240
>
>
OK, from the comment 5 of this bugzilla entry.
I am quoting it here.

--- begin quote
Folks, there are some strange issues.

I refreshed my source (c-c) yesterday. Suddenly, with all the local
patches, the strict warning with this latest C-C (with its
accompanying M-C portion) disappeared.

The bogus warning in question:
JavaScript strict warning: chrome://global/content/bindings/tree.xml,
line 50: reference to undefined property this.treeBoxObject.columns

Previously, even with all my local patches, the bogus strict warning
appeared.
(The latest previous source was
/REF-COMM-CENTRAL/comm-central/mozilla
hg identify
1abce683acef+ qtip/tip/tree-fix-936007.xml
[and I checked the C-C with M-C portion somewhat earlier in April, too.]

However, if I remove the local patches, the strict warning comes back
again with the latest C-C tree.

So I checked the local patches very carefully to see which patch hides
this bogus strict warning.

Now I have learned that running
|if (typeof(this.treeBoxObject.columns) === 'undefined'|
once seems to somehow hides this "strict warning"
while |if ( !('columns' in this.treeBoxObject) )| check alone does not.

Very strange, isn't it?

Details: how to reproduce what I observed.

I broke down  the local patch to tree.xml debugg into two parts.
One has only |if ( !('columns' in this.treeBoxObject) )| check in it.
The other has additional |(this.treeBoxObject.columns) === 'undefined'|
check following the "in" test.

[1-a] With the patch that has only
|if ( !('columns' in this.treeBoxObject) )| check,
the hg queue looks like this.
(under ./mozilla directory of local C-C tree.)

17 A tree-fix-936007.xml: Bug 936007
18 U tree-fix-936007-add-2.patch: accessing  if (typeof
(this.treeBoxObject.c...
19 U not-applicable-webrtc-change.patch: trying to modify the code so
that it...

[1-b] *The patch diff portion *
diff --git a/toolkit/content/widgets/tree.xml
b/toolkit/content/widgets/tree.xml
--- a/toolkit/content/widgets/tree.xml
+++ b/toolkit/content/widgets/tree.xml
@@ -48,18 +48,27 @@
        </xul:hbox>
      </content>

      <implementation implements="nsIDOMXULTreeElement,
nsIDOMXULMultiSelectControlElement">

        <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->

        <property name="columns"
- onget="return this.treeBoxObject.columns;"/>
+ onget="if ( !('columns' in this.treeBoxObject) ) dump('\ndump
.columns UNDEFINED 2\n');
+       for (let o = this.treeBoxObject; o; o =
Object.getPrototypeOf(o)) {
+   dump('searching for columns: ' +
uneval(Object.getOwnPropertyDescriptor(o, 'columns')) + '\n');
+       }
+ dump('\n this.treeBoxObject.columns = ' +
JSON.stringify(this.treeBoxObject.columns) + '; \n' );
+       dump('\nthis.treeBoxObject instanceof
Components.interfaces.nsITreeBoxObject=' +
+ (this.treeBoxObject instanceof
+ Components.interfaces.nsITreeBoxObject) + '\n');

+ return this.treeBoxObject.columns;
+      "/>
        <property name="view"
                 onget="return this.treeBoxObject.view;"
                 onset="return this.treeBoxObject.view = val;"/>

        <property name="body"
                 onget="return this.treeBoxObject.treeBody;"/>

        <property name="editable"

[1-c] Excerpt from the log.

Note that we have "JavaScript strict warning:
chrome://global/content/bindings/tree.xml, line 50: reference to
undefined property this.treeBoxObject.columns" in the excerpt below.

Also, note that
|if ( !('columns' in this.treeBoxObject) )| does not trigger, and
does not print the dump "dump .columns UNDEFINED 2" message.
So actually the |.columns| does seem to be defined (and yes, uneval()
and JSON.stringify() print
some seemingly sane value.)

++DOMWINDOW == 14 (0x4007850) [pid = 5882] [serial = 14] [outer = (nil)]
++DOMWINDOW == 15 (0x49e91e0) [pid = 5882] [serial = 15] [outer = 0x3ff39f0]
++DOMWINDOW == 16 (0x4a512a0) [pid = 5882] [serial = 16] [outer = 0x3fdfcb0]
searching for columns: ({configurable:true, enumerable:true,
get:function columns() {
     [native code]
}, set:(void 0)})
searching for columns: (void 0)
searching for columns: (void 0)
JavaScript strict warning: chrome://global/content/bindings/tree.xml,
line 50: reference to undefined property this.treeBoxObject.columns

  this.treeBoxObject.columns =
{"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};

this.treeBoxObject instanceof Components.interfaces.nsITreeBoxObject=true
          ...
==============

Now, let us apply the additional patch to check for the
"undefined"-ness using
|if (typeof (this.treeBoxObject.columns) ==='undefined')|
as well, and see what happens.

[2-a] hg queue status.
       ...
17 A tree-fix-936007.xml: Bug 936007
18 A tree-fix-936007-add-2.patch: accessing  if (typeof
(this.treeBoxObject.columns) === 'undefined') dump('...
19 U not-applicable-webrtc-change.patch: trying to modify the code so
that it would compile without webrtc

[2-b] diff portion for the additional patch.

Note the addition of
|if (typeof (this.treeBoxObject.columns) === 'undefined') |
check AFTER
|if ( !('columns' in this.treeBoxObject) ) | check.

There are a few cosmetic changes, too.

diff --git a/toolkit/content/widgets/tree.xml
b/toolkit/content/widgets/tree.xml
--- a/toolkit/content/widgets/tree.xml
+++ b/toolkit/content/widgets/tree.xml
@@ -49,24 +49,26 @@
      </content>

      <implementation implements="nsIDOMXULTreeElement,
nsIDOMXULMultiSelectControlElement">

        <!-- ///////////////// nsIDOMXULTreeElement ///////////////// -->

        <property name="columns"
                onget="if ( !('columns' in this.treeBoxObject) ) dump('\ndump .columns
UNDEFINED 2\n');
+       if ( typeof(this.treeBoxObject.columns) === 'undefined')
dump('\n dump UNDEFINED\n');
                       for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) {
                           dump('searching for columns: ' +
uneval(Object.getOwnPropertyDescriptor(o, 'columns')) + '\n');
                       }
- dump('\n this.treeBoxObject.columns = ' +
JSON.stringify(this.treeBoxObject.columns) + '; \n' );
+ dump('\n this.treeBoxObject.columns = ' +
+    JSON.stringify(this.treeBoxObject.columns) + '; \n' );
                       dump('\nthis.treeBoxObject instanceof
Components.interfaces.nsITreeBoxObject=' +
- (this.treeBoxObject instanceof
- Components.interfaces.nsITreeBoxObject) + '\n');
-
+    (this.treeBoxObject instanceof
+    Components.interfaces.nsITreeBoxObject) +
+    '\n');
                        return this.treeBoxObject.columns;
                      "/>
        <property name="view"
                 onget="return this.treeBoxObject.view;"
                 onset="return this.treeBoxObject.view = val;"/>

        <property name="body"
                 onget="return this.treeBoxObject.treeBody;"/>

[2-c] Excerpt from log.

Note that there is no longer strict warning for |.columns|.

++DOMWINDOW == 14 (0x2aca270) [pid = 13513] [serial = 14] [outer = (nil)]
++DOMWINDOW == 15 (0x34ac1e0) [pid = 13513] [serial = 15] [outer =
0x2ab6400]
++DOMWINDOW == 16 (0x35141f0) [pid = 13513] [serial = 16] [outer =
0x2aa2860]
searching for columns: ({configurable:true, enumerable:true,
get:function columns() {
     [native code]
}, set:(void 0)})
searching for columns: (void 0)
searching for columns: (void 0)

  this.treeBoxObject.columns =
{"0":{},"1":{},"2":{},"3":{},"4":{},"5":{},"6":{},"7":{},"8":{},"9":{},"10":{},"11":{},"12":{},"13":{},"14":{},"15":{},"16":{},"17":{},"18":{},"threadCol":{},"flaggedCol":{},"attachmentCol":{},"subjectCol":{},"unreadButtonColHeader":{},"senderCol":{},"recipientCol":{},"junkStatusCol":{},"receivedCol":{},"dateCol":{},"statusCol":{},"sizeCol":{},"tagsCol":{},"accountCol":{},"priorityCol":{},"unreadCol":{},"totalCol":{},"locationCol":{},"idCol":{}};

this.treeBoxObject instanceof Components.interfaces.nsITreeBoxObject=true

WHY does the different behavior occur when I inserted
|if (typeof(this.treeBoxObject.columns) === 'undefined')| check.

I think practical reason that we don't see the strict warning anymore
after |if (typeof(...) === 'undefined'| check is the latest
modifications in following function.
(In April, there were a flurry of patches and some touched this function
and elsewhere.)

http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jsobj.cpp#4394

template <AllowGC allowGC>
4393 static MOZ_ALWAYS_INLINE bool
4394 GetPropertyHelperInline (...
  [omission]

This function has the following code:
4459 /* Do not warn about tests like (obj[prop] == undefined). */
4460 pc += js_CodeSpec[op].length;
4461 if (Detecting(cx, script, pc))
4462     return true;

Maybe the above code checks for the typeof() === 'undefined' ???
I don't particular see special code for "in" check, though.)

I have no idea what was the exact cause, but I have a suspiction the check
did not trigger before.

But given the heuristics used inside

      GetPropertyHelperInline (...

to check whether to raise "undefined" flag seems to depend on the
sequence of bytecode (?) of JS code, a slight change in the produced
code sequence, etc. may suddenly enable/disable such checks and thus
the slightly different behavior may result.

I am wondering if my above speculation is correct since the changes to
the file
in April time frame from "hg log" has some changes that are concerned with
this function.
Most notable are the changes from

Bug 547140 - Remove JSRESOLVE_ASSIGNING and resolve flags
https://bugzilla.mozilla.org/show_bug.cgi?id=547140

Anyway, my original question/bug still remains.

Why does JS Engine thinks |this.treeBoxObject.columns| is undefined
while |'columns' in this.treeBoxObject| check, and
|typeof(this.treeBoxObject) === 'undefined'| check all suggest that it
*IS* defined, and JSON.stringify() prints some reasonable value even.

Maybe the following particular value/form, printed by uneval() at the
time the BOGUS strict warning is printed, is unfriendly to the JS
Engine's idea of defined property check?  I wonder what "[native
code]" means.

({configurable:true, enumerable:true, get:function columns() {
     [native code]
}, set:(void 0)})

TIA

PS: I think we only see the bogus strict warning once [when we see
them] for the invocation of TB because of the following lines: If we
warn once, we won't the second time.

4443 /* Don't warn repeatedly for the same script. */
4444 if (!script || script->warnedAboutUndefinedProp())
4445     return true;
--- end quote



_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine
Reply | Threaded
Open this post in threaded view
|

Re: Possibly buggy detection of "undefined property" by JS Engine?

ISHIKAWA,chiaki
In reply to this post by ISHIKAWA,chiaki
(2014/05/08 3:21), ISHIKAWA,chiaki wrote:

> (2014/04/30 11:39), ISHIKAWA, Chiaki wrote:
>> Filed a bugzilla:
>>
>> Bug 1003240 - JS Engine reports FALSE-POSITIVE(?) "strict warning" for
>> "undefined property" under a certain condition.
>>
>> https://bugzilla.mozilla.org/show_bug.cgi?id=1003240
>>
>>
> OK, from the comment 5 of this bugzilla entry.
> I am quoting it here.

I posted another comment about a week ago.
comment 6 of this bugzilla entry.

I wonder if someone can see the possible bug here.
(I wish I know the internal details of JS engine so that I can
figure out the root cause. But I am a mere user of TB and am
not even familiar with "shape" mentioned in the code, etc.)

---
Leaving the strange behavior in comment 5 behind,
I noticed  the following:

http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jit/BaselineIC.cpp#3317

3313 // Look up a property's shape on an object, being careful never to
do any effectful
3314 // operations.  This procedure not yielding a shape should not be
taken as a lack of
3315 // existence of the property on the object.
3316 static bool
3317 EffectlesslyLookupProperty(JSContext *cx, HandleObject obj,
HandlePropertyName name,
3318                            MutableHandleObject holder,
MutableHandleShape shape,
3319                            bool *checkDOMProxy=nullptr,
3320                            DOMProxyShadowsResult
*shadowsResult=nullptr,
3321                            bool *domProxyHasGeneration=nullptr)

Please note the comment that says, "This procedure not yielding a
shape should not be taken as a lack of existence of the property on
the object.".

OTOH,
http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jsobj.cpp#4394


4392 template <AllowGC allowGC>
4393 static MOZ_ALWAYS_INLINE bool
4394 GetPropertyHelperInline(JSContext *cx,
4395                         typename MaybeRooted<JSObject*,
allowGC>::HandleType obj,
4396                         typename MaybeRooted<JSObject*,
allowGC>::HandleType receiver,
4397                         typename MaybeRooted<jsid,
allowGC>::HandleType id,
4398                         typename MaybeRooted<Value,
allowGC>::MutableHandleType vp)
4399 {
4400     /* This call site is hot -- use the always-inlined variant of
LookupNativeProperty(). */
4401     typename MaybeRooted<JSObject*, allowGC>::RootType obj2(cx);
4402     typename MaybeRooted<Shape*, allowGC>::RootType shape(cx);
4403     if (!LookupPropertyInline<allowGC>(cx, obj, id, &obj2, &shape))
4404         return false;
4405
4406     if (!shape) {
4407         if (!allowGC)
4408             return false;
4409

Note the big |if (!shape)| block.
When shape is null after a return of LookupPropertyInline,
several |if| checks are performed whether the
situation is really an undefined property or not.
There are multiple of situations when the property is deemed NOT
undefined.

I am beginning to suspect that there is a missing special |if| that
saves the day for the particular value of |columns|
as noted in the original post:

--- quote

I excerpt the comment from each if check.

        /* Don't warn if extra warnings not enabled or for random getprop
operations. */

        /* Don't warn repeatedly for the same script. */

        /*
         * Don't warn in self-hosted code (where the further presence of
         * JS::ContextOptions::werror() would result in impossible-to-avoid
         * errors to entirely-innocent client code).
         */

        /* We may just be checking if that object has an iterator. */

        /* Do not warn about tests like (obj[prop] == undefined). */

I am wondering if the particular value of
uneval(Object.getOwnPropertyDescriptor(o, 'columns') printed as follows

({configurable:true, enumerable:true, get:function columns() {
     [native code]
}, set:(void 0)})

needs to be special cased and not recognized as "undefined" in that
function?

--- end quote

Maybe, checking if the object has a (native-code?) getter function that
returns
the required property name [in this case, |columns|]
in its getOwnPropertyDescriptor(),  and if so, the
property should be deemed defined and handled thusly?

What do people in the know think?

TIA



_______________________________________________
dev-tech-js-engine mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-js-engine