Opened 11 years ago

Closed 11 years ago

#7127 closed defect (fixed)

Upgrade custom Rhino to 1_7R1

Reported by: Chris Mitchell Owned by: James Burke
Priority: high Milestone: 1.3
Component: ShrinkSafe Version: 1.1.1
Keywords: Cc: alex
Blocked By: Blocking:

Description

Dojo 1.1.1 currently uses Rhino 1_6R7. The latest version is 1_7R1. 1_7R1 fixes a licensing issue where some Sun code was pulled into the Rhino binary. It makes it easier for companies who want to redistribute shrinksafe if Dojo 1.2 builds were based on Rhino 1_7R1. Alternatively, we could modify custom_rhino.jar to remove the Sun files from the jar, since they are only used by the Rhino debugger...but this is not the preferred route.

Change History (8)

comment:1 Changed 11 years ago by Chris Mitchell

The details of the issue with Rhino including Sun files are described in the following Rhino bug:

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

The source files of concern are:

AbstractCellEditor?.java JTreeTable.java TreeTableModel?.java TreeTableModelAdapter?.java

which build the following class files in the Rhino binary distribution:

2705 Defl:N 1136 58% 10-19-07 04:24 3d649329 org/mozilla/javascript/tools/debugger/downloaded/AbstractCellEditor.class 1239 Defl:N 497 60% 10-19-07 04:24 9cca73c2 org/mozilla/javascript/tools/debugger/downloaded/JTreeTable$ListToTreeSelectionModelWrapper?$ListSelectionHandler?.class 2628 Defl:N 1097 58% 10-19-07 04:24 efb49637 org/mozilla/javascript/tools/debugger/downloaded/JTreeTable$ListToTreeSelectionModelWrapper?.class 2391 Defl:N 1124 53% 10-19-07 04:24 f0cf7fb8 org/mozilla/javascript/tools/debugger/downloaded/JTreeTable$TreeTableCellEditor?.class 2618 Defl:N 1232 53% 10-19-07 04:24 5390ca31 org/mozilla/javascript/tools/debugger/downloaded/JTreeTable$TreeTableCellRenderer?.class 3734 Defl:N 1521 59% 10-19-07 04:24 b327dd04 org/mozilla/javascript/tools/debugger/downloaded/JTreeTable.class

496 Defl:N 286 42% 10-19-07 04:24 ccb2f410 org/mozilla/javascript/tools/debugger/downloaded/TreeTableModel.class

1063 Defl:N 472 56% 10-19-07 04:24 75b17837 org/mozilla/javascript/tools/debugger/downloaded/TreeTableModelAdapter$1.class 1260 Defl:N 510 60% 10-19-07 04:24 a3d4bbd8 org/mozilla/javascript/tools/debugger/downloaded/TreeTableModelAdapter$2.class

836 Defl:N 399 52% 10-19-07 04:24 746cf2e4 org/mozilla/javascript/tools/debugger/downloaded/TreeTableModelAdapter$3.class

2890 Defl:N 1133 61% 10-19-07 04:24 cff6f90c org/mozilla/javascript/tools/debugger/downloaded/TreeTableModelAdapter.class

These files have all been removed from the Rhino 1_7R1 release.

Also, as described in the bug above, Sun has released these files under a new license agreement that should be more acceptable to companies redistributing Rhino. So, these files are currently scheduled to be put back into Rhino in the 1_7R2 release, which is currently under development.

comment:2 Changed 11 years ago by Chris Mitchell

Actually, they are not "scheduled" to be put back into 1_7R2. There is just a comment that they could potentially be reintroduced into Rhino under the new terms going forward.

comment:3 Changed 11 years ago by Adam Peller

Cc: alex added

James has the code ported over to 1.7R1, but it fails in cases where functions are nested several layers deep. I've isolated it to the following test case:

var foo=function(){
        function bar(){ // bar should be compressed since it's not global.  It is not compressed unless you either change this definition to var bar=function(...) or eliminate the line below...
                (function(){}); // without a function definition inside bar, the symbol 'bar' compresses properly.  Any sort of function definition will do here.
        }

        bar();
}

foo();

I think the problem may be in TokenMapper.collectFuncNodes(). If I run the example below, it does a tree walk, and as it unwinds, the third-level nesting causes the function "bar" to go into the "3" bucket in functionVarMappings; without it, "bar" ends up in the "2" bucket and the code compresses properly.

It makes no sense to my why this would have regressed in Rhino 1.7, but I don't fully understand the tree walk code in collectFuncNodes, which I believe is our code. Why does it put the var names in the last bucket when it unwinds (functionVarMappings.size()-1) This value seems to depend on the depth of the function nesting rather than the relative position of the scope chains.

                        bindingNames = (HashMap) functionVarMappings
                                        .get(functionVarMappings.size() - 1);
                        bindingNames.put(new Integer(level), parseTree
                                        .getParamAndVarNames());

when isInScopeChain goes to look up 'bar' in functionVarMappings(functionNum) where functionNum is 2 my example, the value is not found unless I remove the third-level nested function.

comment:4 Changed 11 years ago by Adam Peller

I've removed the sun files from custom_rhino.jar (see #7503) in the interim, so if we have to we can defer this ticket to 1.3.

comment:5 Changed 11 years ago by James Burke

Milestone: 1.2future

I have not been able to look at this more since peller isolated the test case. Going to defer for now, but still very interested to get this to work at some point.

comment:6 Changed 11 years ago by Adam Peller

Component: BuildToolsShrinkSafe
Owner: changed from James Burke to alex

comment:7 Changed 11 years ago by Adam Peller

Owner: changed from alex to James Burke

comment:8 Changed 11 years ago by James Burke

Milestone: future1.3
Resolution: fixed
Status: newclosed

SVN/Trac integration broken? Fixed in [15529], by the patch in ticket #7913.

Note: See TracTickets for help on using tickets.