Last Comment Bug 782146 - Missing elements on lebonforfait.fr
: Missing elements on lebonforfait.fr
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 17 Branch
: x86_64 All
: -- normal (vote)
: mozilla17
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
Depends on:
Blocks: 769911
  Show dependency treegraph
 
Reported: 2012-08-12 13:18 PDT by Mathieu Marquer
Modified: 2012-08-15 14:03 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
disable missing prop IC for ListBase proxies (1.12 KB, patch)
2012-08-13 07:31 PDT, Brian Hackett (:bhackett)
luke: review+
Details | Diff | Review

Description Mathieu Marquer 2012-08-12 13:18:01 PDT
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20120812030538

Steps to reproduce:

Go on http://www.lebonforfait.fr/comparateur.html and move the cursors


Actual results:

The "results" area on the right appears empty


Expected results:

A list of results should appear. Not sure when the bug appeared, works OK on the latest stable release
Comment 1 Alice0775 White 2012-08-12 18:25:54 PDT
Regrwssion window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/1bbc0b65dffb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120807030518
Bad:
http://hg.mozilla.org/mozilla-central/rev/2637d896de91
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120807063927
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1bbc0b65dffb&tochange=2637d896de91


Regrwssion window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b4a63a0b90c2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120806153305
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/89ea9764f9e9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120806140630
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b4a63a0b90c2&tochange=89ea9764f9e9



In local build
Last Good: f3bd764deb31
First Bad: 684958bd600b

Triggered by: 684958bd600b	  Brian Hackett — Generate ICs which see through ListBase proxies, bug 769911. r=peterv,dvander
Comment 2 Alice0775 White 2012-08-12 18:28:17 PDT
An error in error console:
Error: TypeError: document.forms.formulaire is undefined
Source file: http://www.lebonforfait.fr/scripts/Forfait.js
Line: 57
Comment 3 Alice0775 White 2012-08-12 18:59:29 PDT
In adittion to comment #1,
bug 769911 was backed out and landing again as follows.

bad
http://hg.mozilla.org/integration/mozilla-inbound/rev/4129a9d0f887
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120806143827
Working again:
http://hg.mozilla.org/integration/mozilla-inbound/rev/1184b4442755
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120806154026
Working again Pushlog: 
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4129a9d0f887&tochange=1184b4442755

Working:
http://hg.mozilla.org/integration/mozilla-inbound/rev/cf99db30a20c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120806165827
Broken again:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0127c9c41bf2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120806175526
Broken again Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=cf99db30a20c&tochange=0127c9c41bf2
Comment 4 Brian Hackett (:bhackett) 2012-08-13 07:31:13 PDT
Created attachment 651378 [details] [diff] [review]
disable missing prop IC for ListBase proxies

When the patch for bug 769911 was written, it depended for correctness on the fact that we didn't generate ICs for missing properties on the prototype chain, as ListBase proxies will search elsewhere if the prototype lookup fails.  Before the bug landed, bug 777630 added a missing prop IC, and I didn't update bug 769911 before landing.  This path disables the missing prop IC for Listbase proxies.
Comment 5 Luke Wagner [:luke] 2012-08-13 09:02:52 PDT
Could you explain a bit more?  Normally, proxies won't get a missing-prop ic b/c ProxyClass.getProperty != JS_PropertyStub; why doesn't the ListBase proxy hit this?  Is this constraint imposed by other IC code or by the ListBase proxy implementation?
Comment 6 Brian Hackett (:bhackett) 2012-08-13 15:16:01 PDT
Actually, Proxy classes have a default getProperty hook, just the class operations are custom (as they are for all non native classes).
Comment 7 Luke Wagner [:luke] 2012-08-13 15:54:22 PDT
Comment on attachment 651378 [details] [diff] [review]
disable missing prop IC for ListBase proxies

Interesting.  This leads to the question "why didn't the aobj->isNative() check above in lookup() catch this?".  The answer is: because, in the special case of ListBase, the isNative() check was performed on ListBase's prototype, not the ListBase.  This suggests using the condition "!obj->isNative()" instead of IsCacheableListBase(obj) which has the benefit that it is obvious why this condition is necessary and more future proof against this type of bug.  r+ if you agree.
Comment 8 Brian Hackett (:bhackett) 2012-08-13 17:52:34 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a80bf450ed28
Comment 9 Ed Morley [:emorley] 2012-08-14 06:00:30 PDT
https://hg.mozilla.org/mozilla-central/rev/a80bf450ed28

Note You need to log in before you can comment on or make changes to this bug.