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

[jo...@gmail.com]

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"];