Handle UNSOLICITED_SIM_SMS_STORAGE_FULL in ril_worker.js.

I think we'll need a API for fetching device storage availability.

Revise

Comment on attachment 609276 [details] [diff] [review] Part 1: Handle SIM storage full : V2 Review of attachment 609276 [details] [diff] [review]: ----------------------------------------------------------------- I don't really understand where you're going with this patch. Can you please elaborate on how you see this tie in with the rest of the code? We don't actually store SMS on the SIM card right now, do we? So why do we need to worry about these events right now? I don't want to add dead code unnecessarily. ::: dom/system/gonk/ril_worker.js @@ +631,5 @@ > + */ > + _simStorageAvailable: true, > + get simStorageAvailable() { > + return this._simStorageAvailable; > + }, Why so complicated? Just make it a regular property! @@ +647,5 @@ > + if (available != this._deviceStorageAvailable) { > + this._deviceStorageAvailable = available; > + this.reportSMSMemoryStatus(available); > + } > + }, I don't understand the need for this getter/setter. You don't use it at all! @@ +2290,5 @@ > //TODO > + > + if (this._reportSMSMemoryStatusPending) { > + this.reportSMSMemoryStatus(this._deviceStorageAvailable); > + } It seems like this should be hung off of the ICC availability, not the radio state. (Please rebase on top of bug 728886).

(In reply to Philipp von Weitershausen [:philikon] from comment #3) > Comment on attachment 609276 [details] [diff] [review] > Part 1: Handle SIM storage full : V2 > > Review of attachment 609276 [details] [diff] [review]: > ----------------------------------------------------------------- > > I don't really understand where you're going with this patch. Can you please > elaborate on how you see this tie in with the rest of the code? > > We don't actually store SMS on the SIM card right now, do we? So why do we > need to worry about these events right now? I don't want to add dead code > unnecessarily. Yes, we don't. Shall I also pending USIM download in bug 736706? There're still many missing parts to complete it, i.e. raw PDU access.

(In reply to Vicamo Yang [:vicamo] from comment #4) > Yes, we don't. Shall I also pending USIM download in bug 736706? There're > still many missing parts to complete it, i.e. raw PDU access. What user-facing features does USIM enable? I think for now we should focus on expanding our features: voicemail, credit, and other notifications from the carrier, MMS, etc.

Message Waiting Indication(MWI), bug 736710, a feature that notifies "You've got mail".

try to meet review comment #3

Comment on attachment 610458 [details] [diff] [review] Handle SIM storage full : V3 Review of attachment 610458 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +649,5 @@ > + > + /** > + * Device storage space availability. > + * > + * TODO: need device storage API for storage space availability. Please file a bug for this and reference it in this comment. @@ +656,5 @@ > + > + /** > + * Waiting for a successful REQUEST_REPORT_SMS_MEMORY_STATUS reply from RIL. > + */ > + _reportSMSMemoryStatusPending: true, Why does this default to `true`? I guess my real question is: why is this needed? Are you introducing this because you don't want multiple REQUEST_REPORT_SMS_MEMORY_STATUS requests to occur concurrently? Or because we only need to send it off once? The naming and comments of this flag seems to indicate the former, but your usage of it makes me think the latter. @@ +661,5 @@ > + > + /** > + * Report SMS storage status to RIL. > + */ > + _reportSMSMemoryStatus: function _reportSMSMemoryStatus(available) { Why the leading underscore?

Hi Philipp, This bug is intended for bug 736706 USIM download, which is pending for several missed functions. The effect of SMS_MEMORY_STATUS request is still left unclear for me. I think I'd better study more and make sure I know the whole picture of USIM download before continuing working on this issue.

Unblock bug 709564 temporarily for milestone 3.

rebase & remove device storage available flag. TODO: devicestorage stat().

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #11) > Created attachment 662128 [details] [diff] [review] > Handle SIM storage full : WIP v4 > > rebase & remove device storage available flag. TODO: devicestorage stat(). devicestorage doesn't have an interface to monitor /data. Besides, the correct way should be listening to QuotaExceededError event of SmsDatabaseService.

I'm going to remove this as a blocker for the now-closed bug 831079 because it's causing a weird dependency graph elsewhere.

Put this bug into backlog.

Firefox OS is not being worked on