Open
Bug 1003240
Opened 10 years ago
Updated 2 years ago
JS Engine reports FALSE-POSITIVE(?) "strict warning" for "undefined property" under a certain condition.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
NEW
People
(Reporter: ishikawa, Unassigned)
Details
Attachments
(1 file)
|
3.71 KB,
text/plain
|
Details |
Host: linux (both under 32-bit and 64-bit) (Have not tested window version.) This bug entry originated from the question posed in: Bug 936007 - JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns Short Story: With debug build of TB (C-C), JS Engine reports FALSE-POSITIVE(?) "strict warning" for "undefined property" under a certain condition. Problem is this warning seems to be a false positive. The common idiom of checking the undefined property, |propertyname|, ! ( 'propertyname' in variable ) does not evaluate to true. And for that matter, JSON stringify() of |variable.property| prints out a seemingly valid value. So the JS Engine's "strict warning" seems false positive. Details: During the run of |make mozmill| test suite using debug build of TB, I see following "strict warning" many times: JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns Version of source: C-C /REF-COMM-CENTRAL/comm-central hg identify 656bef26207d+ message-from-compact.patch/qtip/tip (Version of M-C repository inside ./mozilla) /REF-COMM-CENTRAL/comm-central/mozilla hg identify 1abce683acef+ qtip/tip/tree-fix-936007.xml I get the strict warning for each startup of TB and more. (In total, I get the warning 55 times or 56 times. 55 JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns Some tests timeout on local PC, and so the number of tests executed during |make mozmill| remain around 1000 and go up about 10 depending on the success of these timed-out tests.) The location where the warning is produced has been traced to the following location: mozilla/toolkit/content/widgets/tree.xml On MXR it is the folloging place. http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/content/widgets/tree.xml#55 55 <property name="columns" 56 onget="return this.treeBoxObject.columns;"/> It is where |.columns| property is returned for this.treeBoxObject. Line number in mxr is 55 and off several lines since the file is pre-processed and an earlier '#ifdef XP_MACOSX' block is eliminated. False Positive?: I inserted the following tests where the 'undefined property' was reported to see if common idiom of checking 'undefined property' returns true for this particular property |columns|. if (typeof (this.treeBoxObject.columns) === 'undefined') dump('\n dump UNDEFINED\n'); if (! ('columns' in this.treeBoxObject) ) dump('\n dump UNDEFINED 2\n'); But both if-statements did not execute the statement when its conditional expression is true. Note JS Engine "propelry" detected the undefined property when I tested for NON-EXISTING |XyXyz| property. if (typeof (this.treeBoxObject.XxYyz) === 'undefined') dump('\n dump XxYyz UNDEFINED \n'); if ( ! ('XxYyz' in this.treeBoxObject) ) dump('\n dump XxYyz UNDEFINED 2\n'); When I inserted the above immediately following the place where the strict warning was issued, I got the output from the dump statements. So as far as the detection of NON-EXISTING property goes, it *IS* working. So if |.columns| property is truely undefined, JS Engine ought to have picked it up with the common idiom of checking. It did not. So I suspect that the property is defined. Value Printed: After asking in dev-tech-js-engine mailing list (mozilla.dev.tech.js-engine newsgroup) I was asked to dump the value of thisBoxObject using uneval() when the above code was executed by using the following code. for (let o = this.treeBoxObject; o; o = Object.getPrototypeOf(o)) { dump('searching for columns: ' + uneval(Object.getOwnPropertyDescriptor(o, 'columns')) + '\n'); } The following is the output: the first loop seems to print a valid value (I am no JS expert, and so I need experts' opinion). 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) Value for this.treeBoxObject.columns as seen by JSON.stringify(). After the loop above, I printed the value of this.treeBoxObject.columns using JSON.stringify(), and JS-Engine does not complain any more and prints out the value shown below. dump('\n this.treeBoxObject.columns = ' + JSON.stringify(this.treeBoxObject.columns) + '; \n' ); 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":{}}; The value above seems to cotain the information for a window of the 3-window display of TB (folder pane, header pane, and message text pane). Details 2: In the newsgroup/mailing discussion, it was pointed out that there is a possibility that I am seeing an artifact caused by the the FakeTreeBoxObject: http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2539 which does NOT not define columns. I quote excerpts from the few exchanges: > 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! Yes, I thought it was worth a try. However, I tried to define "get columns()" function there, but the strict warning still appears, and when I read the folderDisplay.js a little more in detail, I think the functions there would complain very loud if |columns| is requested to FakeTreeBoxObject. > 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?) http://dxr.mozilla.org/comm-central/source/mail/base/content/folderDisplay.js#2634 >/* > * 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) But still this may have some relevance, and so I am pointing it out. (It is possible that exception, dump() etc. may be hidden beneath a rug by XPConnect magic or something???) Current Status of Investigation: I am stumped. I am not a JS expert. I am just a user of TB who would like it to be more robust. I looked for the function where the property's definedness is checked. 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). Obviously JS Engine seems to use that function to obtain a value for |.columns| to print JSON.stingify() value from it. The above function calls another function called LookupPropertyInline() defined in http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jsobj.cpp#4025 I see a comment in the function as follows. 4031 /* NB: The logic of this procedure is implicitly reflected in IonBuilder.cpp's 4032 * |CanEffectlesslyCallLookupGenericOnObject| logic. 4033 * If this changes, please remember to update the logic there as well. 4034 */ 4035 Maybe we have a outdated logical mismatch somewhere in JS Engine code base? Note: At least there is NO symbol or string, |CanEffeectlesslyCallLookupGenericOnObject| in comm-central (or for that matter in M-C) today. (From googling, I think this existed in 2013.) By searching for "lookupGeneric" in comm-central, I could find the following code http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jit/IonBuilder.cpp#6069 6069 static bool 6070 ClassHasEffectlessLookup(const Class *clasp, PropertyName *name) 6071 { 6072 return clasp->isNative() && !clasp->ops.lookupGeneric; 6073 } Maybe this function corresponds to now missing |CanEffeectlesslyCallLookupGenericOnObject|. (If so, we should at least correct the outdated misspelllings.) Actually, there is not much logic in it. It has only a single line. So I checked the usage of this function |ClassHasEffectlessLookup|.) Then I found an interesting function in which |ClassHasEffectLessLookup| is used.: http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jit/IonBuilder.cpp#6127 6126 JSObject * 6127 IonBuilder::testSingletonProperty(JSObject *obj, PropertyName *name) 6128 and the caller of the function above: http://mxr.mozilla.org/comm-central/source/mozilla/js/src/jit/IonBuilder.cpp#6172 6172 bool 6173 IonBuilder::testSingletonPropertyTypes(MDefinition *obj, JSObject *singleton, PropertyName *name, 6174 Maybe the logic in these functions does not handle the particular value of this.treeBoxObject.columns as having a valid value for |.columns| property? Just a possibility. Anyway, I am looking for experts' opinion on the real cause of this strange false-positive(?) strict warning. It is disturbing to see "undefined property" warning from a mail client's log. If such warnings remain in test suite log, I don't think the program ought to be shipped to wider community until such warnings are eradicated. (And that is why I am working on to eliminate such warning one by one.) If JS Engine on which TB itself depends has a problem, it is more disturbing. TIA PS: When I tried to submit this bug a few similar entries showed up. Some are already resolved. But they seem to point to some deep issues in JS Engine. I am not sure which are related to this bug report. Bug 383524 - Undefined property warning is incorrect when reached from the JS API Bug 383330 - Incorrect "reference to undefined property" strict warning when accessing property of an imported object
|
||
Comment 1•10 years ago
|
||
Please provide concise steps to reproduce.
|
Reporter | |
Comment 2•10 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #1) > Please provide concise steps to reproduce. Well the shortest path would be: 1. Create full debug version of TB (from C-C) 2. cd MOZ_OBJ then run 3. make SOLO_TEST=search-window/test-search-window.js mozmill-one You will see a false-positive(?) strict warning: --- quote log ++DOCSHELL 0x3942820 == 11 [pid = 26033] [id = 11] ++DOMWINDOW == 14 (0x3943330) [pid = 26033] [serial = 14] [outer = (nil)] ++DOMWINDOW == 15 (0x43b5be0) [pid = 26033] [serial = 15] [outer = 0x392d540] ++DOMWINDOW == 16 (0x44200a0) [pid = 26033] [serial = 16] [outer = 0x38f9ad0] JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 50: reference to undefined property this.treeBoxObject.columns [26033] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file /REF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp, line 8687 [26033] WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv) && subjPrincipal) failed: file /REF-COMM-CENTRAL/comm-central/mozilla/docshell/base/nsDocShell.cpp, line 8687 --- end log I am trying to see if there is a shorter method to reproduce this, but since I noticed the strange warning during |make mozmill| test suite run, I can't think of a simple alternative. Or if you build full debug version and then run full mozmill test on TB TryServer, I believe you will see the same warning. TIA
|
|
Reporter | |
Comment 3•10 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #1) > Please provide concise steps to reproduce. I just realized that - just starting the debug build of TB in a TTY console prints out the warning into the invoking terminal ! This is the simplest method! You may need to have a few messages in your Inbox (I have not tested the initial empty state.) TIA
|
|
Reporter | |
Comment 4•10 years ago
|
||
Just FYI: Here is a copy of remote JS console (remote JS debugging) [ https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging ] I connected to a TB (C-C debug build) running under linux from FF under Windows, and copy the console. TB was started and fetched a mail when the remote debugger connection was made. TB was in 3-window display mode, showing folder window, header window, and message body window. This is the cleanest log text I could obtain so far to demonstrate the issue (False Positive?) undefined property. --- begin quote 1399061765343 addons.manager DEBUG Loaded provider scope for resource://gre/modules/addons/XPIProvider.jsm: ["XPIProvider"] 1399061765345 addons.manager DEBUG Loaded provider scope for resource://gre/modules/LightweightThemeManager.jsm: ["LightweightThemeManager"] 1399061765348 addons.xpi DEBUG startup 1399061765351 addons.xpi DEBUG checkForChanges 1399061765390 addons.xpi DEBUG No changes found mutating the [[Prototype]] of an object will cause your code to run very slowly; instead create the object with the correct initial [[Prototype]] value using Object.create steelApplication.js:783 ReferenceError: reference to undefined property this.treeBoxObject.columns tree.xml:50 <==*!*!*!*! ReferenceError: reference to undefined property aTabType.panelId tabmail.xml:352 TypeError: anonymous function does not always return a value osfile_shared_front.jsm:551 ReferenceError: reference to undefined property message.level Log.jsm:543 1399061767516 addons.xpi-utils DEBUG Starting async load of XPI database /home/ishikawa/.thunderbird/u1ku1i3y.default/extensions.json 1399061767634 addons.xpi-utils DEBUG Async JSON file read took 0 MS 1399061767634 addons.xpi-utils DEBUG Finished async read of XPI database, parsing... 1399061767636 addons.xpi-utils DEBUG Successfully read XPI database Unknown property '-moz-border-radius'. Declaration dropped. start:135 Unknown property '-moz-border-radius'. Declaration dropped. start:168 ReferenceError: reference to undefined property event.target._folder messenger.xul:1 Blocklist::notify: Requesting https://addons.mozilla.org/blocklist/3/%7B3550f703-e582-4d05-9a08-453d09bdfdc6%7D/31.0a1/Thunderbird/20140502015416/Linux_x86_64-gcc3/en-US/default/Linux%203.12-1-amd64%20(GTK%202.24.23)/default/default/1/52/1/ Blocklist state for {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d} changed from 0 to 0 Blocklist state for jid0-edalmuivkozlouyij0lpdx548bc@jetpack changed from 0 to 0 Blocklist state for {972ce4c6-7e08-4474-a285-3208198ce6fd} changed from 0 to 0 Blocklist state for Gnome Shell Integration changed from 0 to 0 Blocklist state for Shockwave Flash changed from 0 to 0 Blocklist state for iTunes Application Detector changed from 0 to 0 ReferenceError: reference to undefined property b.view tree.xml:1027 ReferenceError: reference to undefined property this.treeBoxObject.view tree.xml:63 ReferenceError: reference to undefined property b.view tree.xml:1027 ReferenceError: reference to undefined property this.treeBoxObject.view tree.xml:63 --- end quote
|
|
Reporter | |
Comment 5•10 years ago
|
||
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;|
|
Reporter | |
Comment 6•10 years ago
|
||
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
|
||
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•