Summary: | [WK2] Fix assertion after r132386. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Byungwoo Lee <bw80.lee> | ||||
Component: | WebKit EFL | Assignee: | Byungwoo Lee <bw80.lee> | ||||
Status: | NEW --- | ||||||
Severity: | Normal | CC: | andersca, gyuyoung.kim, kangil.han, kenneth, kling, laszlo.gombos, lucas.de.marchi, ostap73 | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 100334, 100342, 102200, 102651 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Byungwoo Lee
2012-10-24 17:09:04 PDT
Created attachment 170529 [details]
Patch
Comment on attachment 170529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170529&action=review > Source/WebKit2/Platform/CoreIPC/MessageReceiverMap.cpp:81 > + // ASSERT(!decoder.destinationID()); Can you do additional condition to assert for the messages that should have destinationID? If the assertion is incorrect, it should be removed, not commented out. I'm not sure if I understand how you determined that it was incorrect though. Thanks for your review, Viatcheslav and Alexey. In the r132386, new addMessageReceiver() function (that has destinationID as it's parameter) is added to set messageReceiver to m_messageReceivers. (not m_globalMessageReceivers) I think the purpose of this patch is that, if messageReceiver need destinationID, not to use the messageReceiver globally. So, If there can be two destinationID for DownloadProxy::DidStart, messageReceiver of DownloadProxy should be set for each destinationID. I thought that this patch is on progress, because some related source doesn't handled. (e.g. messageReceiver for DownloadProxy is set to global message receiver in the WebContext, but DownloadProxy ipc messages uses downloadID for the destinationID. Is it better to fix each issues on setting messageReceivers of DownloadProxy and VibrationProvider instead of patching temporarily? Andersca told to check this issue. (in irc channel) Do we need work-around patch to keep EFL bot green for API test at a moment? (In reply to comment #6) > Do we need work-around patch to keep EFL bot green for API test at a moment? Now, I'm not sure that work-around patch is the right way, because this patch is already work-around :) I made a patch to fix the assertions from the messageReceiver of WebVibrationProxy : Bug 100334 (In reply to comment #7) > (In reply to comment #6) > > Do we need work-around patch to keep EFL bot green for API test at a moment? > > Now, I'm not sure that work-around patch is the right way, because this patch is already work-around :) > > I made a patch to fix the assertions from the messageReceiver of WebVibrationProxy : Bug 100334 I made a patch about the assertion from the message receiver of DownloadProxy : Bug 100342 (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > Do we need work-around patch to keep EFL bot green for API test at a moment? > > > > Now, I'm not sure that work-around patch is the right way, because this patch is already work-around :) > > > > I made a patch to fix the assertions from the messageReceiver of WebVibrationProxy : Bug 100334 > > I made a patch about the assertion from the message receiver of DownloadProxy : Bug 100342 So, as I understand, this bug will be fixed by pending commit in bug 100342 and can be closed? (In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > (In reply to comment #6) > > > > Do we need work-around patch to keep EFL bot green for API test at a moment? > > > > > > Now, I'm not sure that work-around patch is the right way, because this patch is already work-around :) > > > > > > I made a patch to fix the assertions from the messageReceiver of WebVibrationProxy : Bug 100334 > > > > I made a patch about the assertion from the message receiver of DownloadProxy : Bug 100342 > > So, as I understand, this bug will be fixed by pending commit in bug 100342 and can be closed? Yes, but Bug 100334(or Bug 100473) also need to be fixed to close this issue. |