Hello steveblock, nicolasroard,
I'd like you to do a code review. Please execute
g4 diff -c 9170361
or point your web browser to
http://mondrian/9170361
to review the following code:
Change 9170361 by [EMAIL PROTECTED] on 2008/11/26 15:45:47 *pending*
Fix the fix in 7909770: make the AndroidMessageQueue unit test
behave correctly when run in a Gears worker.
There were two problems:
1. The test thread was always sending messages back to the
Webkit thread, so when the unit test was run in a worker, the
handler for the messages sent by the test thread was never
invoked, causing a deadlock.
2. The MainMessageHandler was a global. When run in the WebKit
thread, the test set the handler's done flag to true. When
subsequently running the same test in the worker, the flag was
true to begin with!
PRESUBMIT=passed
R=steveblock,nicolasroard
[EMAIL PROTECTED]
DELTA=31 (10 added, 7 deleted, 14 changed)
OCL=9170361
Affected files ...
...
//depot/googleclient/gears/opensource/gears/base/common/message_queue_android.cc#7
edit
31 delta lines: 10 added, 7 deleted, 14 changed
Also consider running:
g4 lint -c 9170361
which verifies that the changelist doesn't introduce new style violations.
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 9170361 by [EMAIL PROTECTED] on 2008/11/26 15:45:47 *pending*
Fix the fix in 7909770: make the AndroidMessageQueue unit test behave
correctly when run in a Gears worker.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/base/common/message_queue_android.cc#7
edit
====
//depot/googleclient/gears/opensource/gears/base/common/message_queue_android.cc#7
-
/home/andreip/Gears/googleclient/gears/opensource/gears/base/common/message_queue_android.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/message_queue_android.cc
2008-11-26 15:48:41.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/message_queue_android.cc
2008-11-26 15:47:28.000000000 +0000
@@ -410,7 +410,7 @@
LOG(("Got data %d", count_));
if (count_ == 600) {
// Tell the main thread we got all data.
- LOG(("Sending to main thread"));
+ LOG(("Sending to parent thread"));
message = new TestMessage(
ThreadMessageQueue::GetInstance()->GetCurrentThreadId(), count_);
ThreadMessageQueue::GetInstance()->Send(sender_,
@@ -446,12 +446,10 @@
class TestThread : public Thread {
public:
+ explicit TestThread(ThreadId parent_id) : parent_id_(parent_id) {}
virtual void Run() {
- AndroidThreadMessageQueue* queue = static_cast<AndroidThreadMessageQueue*>(
- ThreadMessageQueue::GetInstance());
-
- BackgroundMessageHandler handler1(queue->MainThreadId());
- BackgroundMessageHandler handler2(queue->MainThreadId());
+ BackgroundMessageHandler handler1(parent_id_);
+ BackgroundMessageHandler handler2(parent_id_);
ThreadMessageQueue::GetInstance()->RegisterHandler(
kMessageType1, &handler1);
ThreadMessageQueue::GetInstance()->RegisterHandler(
@@ -461,40 +459,45 @@
AndroidMessageLoop::Start();
LOG(("Test thread shutting down."));
}
+
+ private:
+ ThreadId parent_id_;
};
-
-// Global message handler
-MainMessageHandler global_handler;
bool TestThreadMessageQueue(std::string16* error) {
AndroidThreadMessageQueue* queue = static_cast<AndroidThreadMessageQueue*>(
ThreadMessageQueue::GetInstance());
- // Init the message queue for the main thread
- queue->InitThreadMessageQueue();
- queue->RegisterHandler(kMessageType3, &global_handler);
+ // The message queue for this thread is already initialized:
+ // - if this is the main browser thread, the queue was initialized
+ // in NP_Initialized.
+ // - if this is a worker, the queue was initialized in
+ // Thread::ThreadMain().
+ // Our message handler
+ MainMessageHandler local_handler;
+ queue->RegisterHandler(kMessageType3, &local_handler);
// Start the worker.
- scoped_ptr<TestThread> thread(new TestThread);
+ ThreadId self_id = queue->GetCurrentThreadId();
+ scoped_ptr<TestThread> thread(new TestThread(self_id));
ThreadId worker_thread_id = thread->Start();
// Send three messages, sleep, send another three.
- ThreadId main_thread_id = queue->MainThreadId();
- TestMessage* message = new TestMessage(main_thread_id, 1);
+ TestMessage* message = new TestMessage(self_id, 1);
queue->Send(worker_thread_id, kMessageType1, message);
- message = new TestMessage(main_thread_id, 2);
+ message = new TestMessage(self_id, 2);
queue->Send(worker_thread_id, kMessageType1, message);
- message = new TestMessage(main_thread_id, 3);
+ message = new TestMessage(self_id, 3);
queue->Send(worker_thread_id, kMessageType1, message);
sleep(1);
- message = new TestMessage(main_thread_id, 100);
+ message = new TestMessage(self_id, 100);
queue->Send(worker_thread_id, kMessageType2, message);
- message = new TestMessage(main_thread_id, 200);
+ message = new TestMessage(self_id, 200);
queue->Send(worker_thread_id, kMessageType2, message);
- message = new TestMessage(main_thread_id, 300);
+ message = new TestMessage(self_id, 300);
queue->Send(worker_thread_id, kMessageType2, message);
- while (!global_handler.IsDone()) {
+ while (!local_handler.IsDone()) {
AndroidMessageLoop::RunOnce();
}
LOG(("Message from worker received."));
