Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9974992
or point your web browser to
http://mondrian/9974992
to review the following code:
Change 9974992 by stevebl...@steveblock-gears1 on 2009/02/03 11:09:02 *pending*
Temporarily disables testRemoveCrossThread() for Opera Mobile.
This test makes assumptions about the message queues in Gears which are
not true for Opera Mobile.
Deepak, it looks like the reason that this test fails on Opera Mobile
is that it makes assumptions about the way
messages are delivered between threads in Gears, which are not true for
Opera Mobile. Take a look at the patch and
let me know if that sounds like a reasonable diagnosis. If so, we'll
disable the test for now and revisit the issue later.
Note that to make this change, I've modified the build info string for
Gears for Opera from 'npapi' to 'opera'.
The use of 'npapi' was for leagcy reasons and I think this makes sense
in general. Let me know if that's OK with you.
R=andreip
[email protected],[email protected],[email protected]
DELTA=20 (20 added, 0 deleted, 0 changed)
OCL=9974992
Affected files ...
...
//depot/googleclient/gears/opensource/gears/test/testcases/database_tests.js#8
edit
... //depot/googleclient/gears/opensource/gears/test/tester/assert.js#13 edit
20 delta lines: 20 added, 0 deleted, 0 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 9974992 by stevebl...@steveblock-gears1 on 2009/02/03 11:09:02 *pending*
Temporarily disables testRemoveCrossThread() for Opera Mobile.
This test makes assumptions about the message queues in Gears which are
not true for Opera Mobile.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/test/testcases/database_tests.js#8
edit
... //depot/googleclient/gears/opensource/gears/test/tester/assert.js#13 edit
====
//depot/googleclient/gears/opensource/gears/test/testcases/database_tests.js#8
-
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/test/testcases/database_tests.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/database_tests.js
2009-02-03 11:09:50.000000000 +0000
+++ googleclient/gears/opensource/gears/test/testcases/database_tests.js
2009-02-03 11:06:53.000000000 +0000
@@ -605,6 +605,25 @@
}
function testRemoveCrossThread() {
+ if (isWince && isOpera) {
+ // This test makes two assumptions about the queues used to deliver
messages
+ // between threads within Gears.
+ // - The message service and workerpool module use the same message queue
+ // - This queue is FIFO
+ //
+ // When the worker calls db.remove(), GearsDatabase::Remove() sends a
+ // 'database deleted' thread message to update database objects in other
+ // threads. The worker then sends a workerpool message to the parent
thread.
+ // By the above assumptions, the 'database deleted' message is received and
+ // processed before the workerpool message, thus ensuring that the database
+ // object is aware of the deletion and returns the expected error.
+ //
+ // These assumptions are not true for Opera Mobile.
+ //
+ // TODO(steveblock): Revisit this issue and resolve properly. See bug XXXX.
+ return;
+ }
+
var db1 = google.gears.factory.create('beta.database');
db1.open('testremoveddb');
==== //depot/googleclient/gears/opensource/gears/test/tester/assert.js#13 -
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/test/tester/assert.js ====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/tester/assert.js 2009-02-03
11:09:50.000000000 +0000
+++ googleclient/gears/opensource/gears/test/tester/assert.js 2009-02-03
10:58:33.000000000 +0000
@@ -50,6 +50,7 @@
var isFirefox = google.gears.factory.getBuildInfo().indexOf(';firefox') > -1;
var isSafari = google.gears.factory.getBuildInfo().indexOf(';safari') > -1;
var isNPAPI = google.gears.factory.getBuildInfo().indexOf(';npapi') > -1;
+var isOpera = google.gears.factory.getBuildInfo().indexOf(';opera') > -1;
/**
* Whether the installed Gears extension has the test scriptable object.
