Hello andreip, baran,
I'd like you to do a code review. Please execute
g4 diff -c 8787075
or point your web browser to
http://mondrian/8787075
to review the following code:
Change 8787075 by [EMAIL PROTECTED] on 2008/10/29 22:50:28 *pending*
Fixes test for creating worker from URL that redirects cross-origin.
Also updates workerpool documentation to clarify behaviour when
creating workers from cross-origin URLs.
R=andreip,baran
[EMAIL PROTECTED]
DELTA=39 (23 added, 0 deleted, 16 changed)
OCL=8787075
Affected files ...
... //depot/googleclient/gears/opensource/gears/sdk/api_workerpool.html#7 edit
... //depot/googleclient/gears/opensource/gears/test/runner/bootstrap.py#11 edit
... //depot/googleclient/gears/opensource/gears/test/runner/testrunner.py#5 edit
... //depot/googleclient/gears/opensource/gears/test/runner/testwebserver.py#8
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/workerpool_createworkerfromurl_tests.js#4
edit
39 delta lines: 23 added, 0 deleted, 16 changed
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 8787075 by [EMAIL PROTECTED] on 2008/10/29 22:50:28 *pending*
Fixes test for creating worker from URL that redirects cross-origin.
Also updates workerpool documentation to clarify behaviour when
creating workers from cross-origin URLs.
Affected files ...
... //depot/googleclient/gears/opensource/gears/sdk/api_workerpool.html#7 edit
... //depot/googleclient/gears/opensource/gears/test/runner/bootstrap.py#11 edit
... //depot/googleclient/gears/opensource/gears/test/runner/testrunner.py#5 edit
... //depot/googleclient/gears/opensource/gears/test/runner/testwebserver.py#8
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/workerpool_createworkerfromurl_tests.js#4
edit
==== //depot/googleclient/gears/opensource/gears/sdk/api_workerpool.html#7 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/sdk/api_workerpool.html
====
# action=edit type=text
--- googleclient/gears/opensource/gears/sdk/api_workerpool.html 2008-10-30
21:32:19.000000000 +0000
+++ googleclient/gears/opensource/gears/sdk/api_workerpool.html 2008-10-30
21:28:33.000000000 +0000
@@ -348,6 +348,14 @@
• <code>google.gears.workerPool</code> - Gives access to the
WorkerPool that created the worker.
<br><br>
Worker IDs are guaranteed to be unique values that are never reused
within the same WorkerPool.
+ <br><br>
+ If the origin of the worker URL is different from that of the calling
+ script, the worker will inherit inherit permissions from the calling
+ script. Permissions are not inherited, however, if the worker URL
involves
+ a cross-origin redirect. Despite inheriting permissions in this way, all
+ methods on <code>google.gears.factory</code> will fail in the worker
until
+ it calls <code>allowCrossOrigin()</code>.
+ <br><br>
</td>
</tr>
</table>
==== //depot/googleclient/gears/opensource/gears/test/runner/bootstrap.py#11 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/runner/bootstrap.py
====
# action=edit type=xtext
--- googleclient/gears/opensource/gears/test/runner/bootstrap.py
2008-10-30 18:32:36.000000000 +0000
+++ googleclient/gears/opensource/gears/test/runner/bootstrap.py
2008-10-30 18:24:12.000000000 +0000
@@ -176,9 +176,10 @@
installers.append(installer.Firefox2LinuxInstaller('gears-ff2'))
installers.append(installer.Firefox3LinuxInstaller('gears-ff3'))
- # Adding second webserver for cross domain tests.
+ # Adding second and third webservers for cross domain tests.
test_servers.append(TestWebserver(serverRootDir(), port=8001))
test_servers.append(TestWebserver(serverRootDir(), port=8002))
+ test_servers.append(TestWebserver(serverRootDir(), port=8003))
gears_binaries = sys.argv[1]
testrunner = TestRunner(launchers, test_servers, test_url)
==== //depot/googleclient/gears/opensource/gears/test/runner/testrunner.py#5 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/runner/testrunner.py
====
# action=edit type=xtext
--- googleclient/gears/opensource/gears/test/runner/testrunner.py
2008-10-30 18:32:37.000000000 +0000
+++ googleclient/gears/opensource/gears/test/runner/testrunner.py
2008-10-30 18:18:27.000000000 +0000
@@ -111,6 +111,7 @@
web_servers = []
web_servers.append(TestWebserver(server_root_dir(), port=8001))
web_servers.append(TestWebserver(server_root_dir(), port=8002))
+ web_servers.append(TestWebserver(server_root_dir(), port=8003))
installers = []
==== //depot/googleclient/gears/opensource/gears/test/runner/testwebserver.py#8
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/runner/testwebserver.py
====
# action=edit type=xtext
--- googleclient/gears/opensource/gears/test/runner/testwebserver.py
2008-10-30 18:32:37.000000000 +0000
+++ googleclient/gears/opensource/gears/test/runner/testwebserver.py
2008-10-30 21:29:21.000000000 +0000
@@ -530,6 +530,7 @@
else:
port_number = 8001
- # Instantiating second server for cross domain tests.
+ # Instantiating second and third servers for cross domain tests.
TestWebserver(server_root_dir(), port=port_number)
TestWebserver(server_root_dir(), port=(port_number + 1)).startServing()
+ TestWebserver(server_root_dir(), port=(port_number + 2)).startServing()
\ No newline at end of file
====
//depot/googleclient/gears/opensource/gears/test/testcases/workerpool_createworkerfromurl_tests.js#4
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/testcases/workerpool_createworkerfromurl_tests.js
====
# action=edit type=text
---
googleclient/gears/opensource/gears/test/testcases/workerpool_createworkerfromurl_tests.js
2008-10-30 21:32:19.000000000 +0000
+++
googleclient/gears/opensource/gears/test/testcases/workerpool_createworkerfromurl_tests.js
2008-10-30 21:30:21.000000000 +0000
@@ -34,7 +34,7 @@
var currentPort = currentUrl.substring(currentUrl.lastIndexOf(':') + 1,
currentUrl.length);
currentPort = currentPort.substring(0, currentPort.indexOf('/'));
-crossOriginPort = parseInt(currentPort) + 1;
+var crossOriginPort = parseInt(currentPort) + 1;
var crossOriginPath =
currentUrl.substring(0, 1 + currentUrl.lastIndexOf(':')) +
@@ -160,20 +160,32 @@
function testRedirectToCrossOrigin() {
// Tests loading a worker where the URL results in a cross-origin redirect.
- if (isSafari) {
- // TODO(steveblock): Work out why this fails on Safari.
- } else {
- var wp = google.gears.factory.create('beta.workerpool');
- wp.onmessage = function(text, sender, m) {
- completeAsync();
- };
- var workerUrl = crossOriginPath + crossOriginWorkerFile;
- var redirectUrl = localPath + redirect;
- var childId = wp.createWorkerFromUrl(redirectUrl + workerUrl);
-
- startAsync();
- wp.sendMessage('cross origin ping', childId);
+ // Gears will not grant permission to the redirected origin, so the worker
+ // will throw an exception when it tries to create a new Gears object. This
+ // test relies on the redirected origin not already having local storage
+ // permissions.
+ var wp = google.gears.factory.create('beta.workerpool');
+ // We must set an onmessage handler.
+ //wp.onmessage = function(text, sender, m) {}
+ window.onerror = function(message) {
+ assertEqual(
+ 'Error in worker 1 at line 21. Page does not have permission to use ' +
+ 'Google Gears.',
+ message,
+ 'Worker from cross-origin redirect should not inherit permissions.');
+ completeAsync();
}
+ var crossOriginPort2 = parseInt(currentPort) + 2;
+ var crossOriginPath2 =
+ currentUrl.substring(0, 1 + currentUrl.lastIndexOf(':')) +
+ crossOriginPort2 +
+ '/testcases/';
+ var workerUrl = crossOriginPath2 + crossOriginWorkerFile;
+ var redirectUrl = localPath + redirect;
+ var childId = wp.createWorkerFromUrl(redirectUrl + workerUrl);
+
+ startAsync();
+ wp.sendMessage('cross origin ping', childId);
}
function testCrossOriginToRedirect() {
