In RILContentHelper, we use only one |_window| to cache the requester's window in sendMMI() [1]. But actually there is a potential issue here, if there are two requests come at the same time, the |_window| will be override by latter one. We should use |_windowsMap| to cache window, just like what we did in CardLock related function [2]. [1] http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RILContentHelper.js#857 [2] Please see bug 873380.

Quick patch and I am trying to add a test case for the request error.

Add marionette test for error case.

Comment on attachment 816942 [details] [diff] [review] Fix the potential issue in MMI request handler, v3 Review of attachment 816942 [details] [diff] [review]: ----------------------------------------------------------------- This patch looks good to me. But I'd like to add another test case for calling two sendMMI in a row to address what you have fixed in this patch. ::: dom/network/tests/marionette/test_mobile_mmi.js @@ +76,4 @@ > tasks.next(); > } > request.onerror = function onerror() { > ok(false, "request success"); The message here is incorrect

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4) > Comment on attachment 816942 [details] [diff] [review] > Fix the potential issue in MMI request handler, v3 > > Review of attachment 816942 [details] [diff] [review]: > ----------------------------------------------------------------- > > This patch looks good to me. > But I'd like to add another test case for calling two sendMMI in a row to > address what you have fixed in this patch. Sure, I will add another test case in next version, thank you. > > ::: dom/network/tests/marionette/test_mobile_mmi.js > @@ +76,4 @@ > > tasks.next(); > > } > > request.onerror = function onerror() { > > ok(false, "request success"); > > The message here is incorrect

(In reply to Edgar Chen [:edgar][:echen] from comment #5) > (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4) > > Comment on attachment 816942 [details] [diff] [review] > > Fix the potential issue in MMI request handler, v3 > > > > Review of attachment 816942 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > This patch looks good to me. > > But I'd like to add another test case for calling two sendMMI in a row to > > address what you have fixed in this patch. > Sure, I will add another test case in next version, thank you. Hmm, after did some tests, even we calling two sendMMI in a row in marionette, the request still can be processed correctly. Because these two requests are belonging to the same window. I will file another bug for figuring out how to add a test case for this fix. Let's land this fix first.

Comment on attachment 816942 [details] [diff] [review] Fix the potential issue in MMI request handler, v3 Review of attachment 816942 [details] [diff] [review]: ----------------------------------------------------------------- Add r=me

(In reply to Edgar Chen [:edgar][:echen] from comment #6) > (In reply to Edgar Chen [:edgar][:echen] from comment #5) > > (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4) > > > Comment on attachment 816942 [details] [diff] [review] > > > Fix the potential issue in MMI request handler, v3 > > > > > > Review of attachment 816942 [details] [diff] [review]: > > > ----------------------------------------------------------------- > > > > > > This patch looks good to me. > > > But I'd like to add another test case for calling two sendMMI in a row to > > > address what you have fixed in this patch. > > Sure, I will add another test case in next version, thank you. > > Hmm, after did some tests, even we calling two sendMMI in a row in > marionette, the request still can be processed correctly. Because these two > requests are belonging to the same window. I will file another bug for > figuring out how to add a test case for this fix. Let's land this fix first. Not very sure, but seems [1] might help?! [1] http://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/test_outgoing_emergency_in_airplane_mode.js#14

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #8) > (In reply to Edgar Chen [:edgar][:echen] from comment #6) > > (In reply to Edgar Chen [:edgar][:echen] from comment #5) > > > (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4) > > > > Comment on attachment 816942 [details] [diff] [review] > > > > Fix the potential issue in MMI request handler, v3 > > > > > > > > Review of attachment 816942 [details] [diff] [review]: > > > > ----------------------------------------------------------------- > > > > > > > > This patch looks good to me. > > > > But I'd like to add another test case for calling two sendMMI in a row to > > > > address what you have fixed in this patch. > > > Sure, I will add another test case in next version, thank you. > > > > Hmm, after did some tests, even we calling two sendMMI in a row in > > marionette, the request still can be processed correctly. Because these two > > requests are belonging to the same window. I will file another bug for > > figuring out how to add a test case for this fix. Let's land this fix first. > > Not very sure, but seems [1] might help?! > > [1] > http://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/ > test_outgoing_emergency_in_airplane_mode.js#14 I have tried this way, but still doesn't work. :( Thanks anyway.

(In reply to Edgar Chen [:edgar][:echen] from comment #9) > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #8) > > (In reply to Edgar Chen [:edgar][:echen] from comment #6) > > > (In reply to Edgar Chen [:edgar][:echen] from comment #5) > > > > (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4) > > > > > Comment on attachment 816942 [details] [diff] [review] > > > > > Fix the potential issue in MMI request handler, v3 > > > > > > > > > > Review of attachment 816942 [details] [diff] [review]: > > > > > ----------------------------------------------------------------- > > > > > > > > > > This patch looks good to me. > > > > > But I'd like to add another test case for calling two sendMMI in a row to > > > > > address what you have fixed in this patch. > > > > Sure, I will add another test case in next version, thank you. > > > > > > Hmm, after did some tests, even we calling two sendMMI in a row in > > > marionette, the request still can be processed correctly. Because these two > > > requests are belonging to the same window. I will file another bug for > > > figuring out how to add a test case for this fix. Let's land this fix first. > > > > Not very sure, but seems [1] might help?! > > > > [1] > > http://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/ > > test_outgoing_emergency_in_airplane_mode.js#14 > > I have tried this way, but still doesn't work. :( > Thanks anyway. So sad, QQ

1). Correct the message mention in comment #4. 2). Add r=allstars.chh after r+.

try server: https://tbpl.mozilla.org/?tree=Try&rev=3825a7376a7c