webpack/uglify-es depend on new JS frontend bindings

[classilla]

After much analysis I have concluded the issues with current webpack (and uglify, when used to build the front end package) are due to the new front-end bindings introduced in Firefox 51. The symptom is usually this is undefined in the JavaScript console, see

angular/angular-cli#9340 (comment)

Currently affected test sites:
https://www.citibank.com (at account login)
https://cloud.google.com/blog (doesn't work hardly at all)

However, it's probable this affects lots of sites and is likely to affect others as they upgrade their front end blobs.

The new front end bindings are https://bugzilla.mozilla.org/show_bug.cgi?id=1263355

This is a huge undertaking. It isn't clear which part fixes the problem.

[classilla]

Best guess for the actual error is MSG_DEF(JSMSG_UNEXPECTED_TYPE, 2, JSEXN_TYPEERR, "{0} is {1}"). This appears in a number of places, but the most likely ones are:

builtin/Object.cpp:        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_UNEXPECTED_TYPE,
builtin/ReflectParse.cpp:            ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
builtin/ReflectParse.cpp:                ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
builtin/ReflectParse.cpp:            ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE, JSDVG_SEARCH_STACK,
builtin/SymbolObject.cpp:        ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE, JSDVG_SEARCH_STACK,
jscntxt.cpp:                                          JSMSG_UNEXPECTED_TYPE, bytes.get(),
jscntxt.cpp:                                          JSMSG_UNEXPECTED_TYPE, bytes.get(),
jsobj.cpp:        JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_UNEXPECTED_TYPE,

[classilla]

It's coming from js::ReportIsNullOrUndefined in jscntxt.cpp.

[classilla]

I think the change needs to occur in BytecodeEmitter::emitNameOp because that can emit a JSOP_UNDEFINED for this.

[classilla]

One other possibility is BytecodeEmitter::emitThisLiteral but this bug did a lot of changes to emitNameOp's successor emitGetNameAtLocation, so I think that's more likely where the money is. We just need to figure out how we can get the scope there and we could make an analogous change to emit JSOP_GIMPLICITTHIS if global.

[classilla]

That didn't seem to help. For the record, we did this, which is probably on second thought a no-op:

diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitt
er.cpp
index df0d8a7b0..5d68b8aa9 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2586,20 +2586,23 @@ BytecodeEmitter::emitNameOp(ParseNode* pn, bool callCont
ext)
         } else {
             if (!emitAtomOp(pn, op))
                 return false;
         }
     }
 
     /* Need to provide |this| value for call */
     if (callContext) {
-        if (op == JSOP_GETNAME || op == JSOP_GETGNAME) {
+        if (op == JSOP_GETNAME) {
             JSOp thisOp = needsImplicitThis() ? JSOP_IMPLICITTHIS : JSOP_GIMPLI
CITTHIS;
             if (!emitAtomOp(pn, thisOp))
                 return false;
+        } else if (op == JSOP_GETGNAME) {
+            if (!emitAtomOp(pn, JSOP_GIMPLICITTHIS))
+                return false;
         } else {
             if (!emit1(JSOP_UNDEFINED))
                 return false;
         }
     }
 
     return true;
 }

[classilla]

Maybe also BytecodeEmitter::emitCallOrNew ?

[classilla]

Not fixed by this either:

diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitt
er.cpp
index df0d8a7b0..abadb9ef4 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2586,17 +2586,17 @@ BytecodeEmitter::emitNameOp(ParseNode* pn, bool callCont
ext)
         } else {
             if (!emitAtomOp(pn, op))
                 return false;
         }
     }
 
     /* Need to provide |this| value for call */
     if (callContext) {
-        if (op == JSOP_GETNAME || op == JSOP_GETGNAME) {
+        if (1) { // op == JSOP_GETNAME || op == JSOP_GETGNAME) {
             JSOp thisOp = needsImplicitThis() ? JSOP_IMPLICITTHIS : JSOP_GIMPLI
CITTHIS;
             if (!emitAtomOp(pn, thisOp))
                 return false;
         } else {
             if (!emit1(JSOP_UNDEFINED))
                 return false;
         }
     }

[classilla]

Nor this:

diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitt
er.cpp
index df0d8a7b0..43a39796b 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2586,24 +2586,27 @@ BytecodeEmitter::emitNameOp(ParseNode* pn, bool callCont
ext)
         } else {
             if (!emitAtomOp(pn, op))
                 return false;
         }
     }
 
     /* Need to provide |this| value for call */
     if (callContext) {
+#if(0)
         if (op == JSOP_GETNAME || op == JSOP_GETGNAME) {
             JSOp thisOp = needsImplicitThis() ? JSOP_IMPLICITTHIS : JSOP_GIMPLI
CITTHIS;
             if (!emitAtomOp(pn, thisOp))
                 return false;
         } else {
             if (!emit1(JSOP_UNDEFINED))
                 return false;
         }
+#endif
+        if(!emitAtomOp(pn, JSOP_IMPLICITTHIS)) return false;
     }
 
     return true;
 }
 
 bool
 BytecodeEmitter::emitPropLHS(ParseNode* pn)
 {

[rardin]

Issue 533 no longer seems to impact use of the Citi site, evidently due to changes on the Citi side. I was able to login to my account, pay my bill and print a PDF of the transaction with no problem this morning.