Make rpc.js window.frames code more robust (issue166046)

[johnfargo]

Reviewers: shindig.remailer_gmail.com,

Description:
Turns out IE8 treats this as an Array access rather than Object/Map:
window.frames["1234"]
This caused gadgets.rpc to break when the frame name consists only of
isDigit characters.

I propose this patch as a way of centralizing the targetWindow retrieval
mechanism for rpc. It also tries falling back to
document.getElementById(...) if a window.frames find doesn't work.

Comments welcome.

Please review this at http://codereview.appspot.com/166046

Affected files:
  features/src/main/javascript/features/rpc/rmr.transport.js
  features/src/main/javascript/features/rpc/rpc.js
  features/src/main/javascript/features/rpc/wpm.transport.js


Index: features/src/main/javascript/features/rpc/rpc.js
===================================================================
--- features/src/main/javascript/features/rpc/rpc.js    (revision 886897)
+++ features/src/main/javascript/features/rpc/rpc.js    (working copy)
@@ -295,6 +295,30 @@
     return protocol + "://" + host + portStr;
   }

+  function getTargetWindow(id) {
+    if (typeof id === "undefined" ||
+        id === "..") {
+      return window.parent;
+    }
+
+    // Cast to a String to avoid an index lookup.
+    id = String(id);
+
+    // Try window.frames first
+    var target = window.frames[id];
+    if (target) {
+      return target;
+    }
+
+    // Fall back to getElementById()
+    target = document.getElementById(id);
+    if (target && target.contentWindow) {
+      return target.contentWindow;
+    }
+
+    return null;
+  }
+
   // Pick the most efficient RPC relay mechanism.
   var transport = getTransport();

@@ -369,12 +393,7 @@
         return false;
       }

-      var targetEl = null;
-      if (target === '..') {
-        targetEl = window.parent;
-      } else {
-        targetEl = window.frames[target];
-      }
+      var targetEl = getTargetWin(target);
       try {
         // If this succeeds, then same-domain policy applied
         sameDomain[target] = targetEl.gadgets.rpc.receiveSameDomain;
@@ -800,6 +819,9 @@
       }
     },

+    /** Returns the window keyed by the ID. null/".." for parent, else  
child */
+    getTargetWindow: getTargetWindow,
+
     /** Exported constant, for use by transports only. */
     ACK: ACK,

Index: features/src/main/javascript/features/rpc/rmr.transport.js
===================================================================
--- features/src/main/javascript/features/rpc/rmr.transport.js	(revision  
886897)
+++ features/src/main/javascript/features/rpc/rmr.transport.js	(working  
copy)
@@ -195,12 +195,13 @@
     rmr_channels[frameId].searchCounter++;

     try {
+      var targetWin = gadgets.rpc.getTargetWin(frameId);
       if (frameId === '..') {
         // We are a gadget.
-        channelWindow = window.parent.frames['rmrtransport-' +  
gadgets.rpc.RPC_ID];
+        channelWindow = targetWin.frames['rmrtransport-' +  
gadgets.rpc.RPC_ID];
       } else {
         // We are a container.
-        channelWindow = window.frames[frameId].frames['rmrtransport-..'];
+        channelWindow = targetWin.frames['rmrtransport-..'];
       }
     } catch (e) {
       // Just in case; may happen when relay is set to about:blank or  
unset.
Index: features/src/main/javascript/features/rpc/wpm.transport.js
===================================================================
--- features/src/main/javascript/features/rpc/wpm.transport.js	(revision  
886897)
+++ features/src/main/javascript/features/rpc/wpm.transport.js	(working  
copy)
@@ -81,7 +81,7 @@
     },

     call: function(targetId, from, rpc) {
-      var targetWin = targetId === '..' ? window.parent :  
window.frames[targetId];
+      var targetWin = gadgets.rpc.getTargetWin(targetId);
       // targetOrigin = canonicalized relay URL
       var origRelay = gadgets.rpc.getRelayUrl(targetId) ||
                       gadgets.util.getUrlParameters()["parent"];