1046026 - [B2G][SMS] Re-Write Storage Full Error Handling in MobileMessageDB by adopting "QuotaExceededError" from IndexedDB.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

While developing Bug 974820, 
I was not aware of the availability of |QuotaExceededError| from |event.target.error.name| of |IDBRequest| for identifying error of accessing DB when storage is low, so
DiskSpaceWatcher.isDiskFull is adopted instead to prevent further modification of MobileMessageDB when device storage is low.

This might cause some inconsistency behavior to other indexedDB instances.
For example, when storage is low, it is allowed to delete records from indexedDB.
However, in current implementation of MobileMessageDB, deletion is forbidden.

File this bug to re-implement it with |QuotaExceededError|.

Comment 1

[at-kitamura]

[Blocking Requested - why for this release]:
Nominating this for 2.0, as partners need this on a product.

Comment 2

[Hsin-Yi Tsai (she/her)[:hsinyi]]

(In reply to at-kitamura from comment #1)
> [Blocking Requested - why for this release]:
> Nominating this for 2.0, as partners need this on a product.

May I understand what exactly the issue you are encountering on your product? Can I say that what you want is "allow to delete records from indexedDB" when the storage is full? Is my understanding correct? Thank you.

Comment 3

[Makoto Kato [:m_kato]]

set partner flag requested by partner.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Atsushi,

As you mentioned offlined, for indexedDB, |add| and |modify| is restricted but |delete| is allowed when storage is low.
However, due to the thread-based messaging design, there is still some limitation you need to know for deleting messages from MobileMessageDB even if the |QuotaExceededError| is adopted by MobileMessageDB 
The reasons are that:
1. Each thread has the some fields such as |lastMessageSubject|, |body| to summarize the information of the last message in that thread.
2. So, when deleting the last message of the thread, the thread record will be updated according the content of next message of this thread. In this case, the |modify| operation will be aborted due to low storage.

Hence, deleting the last message of a thread will be failed due to the limitation mentioned above.

Comment 5

[bhavana bajaj [:bajaj]]

I am reaching out to wayne to help reachout offline as we are past the time to do any RIL related changes and I think this has existed in our past releases so unclear why would stop 2.0 for this. But given this has been reported by a partner, we can look into fixing this in 2.1.

Comment 6

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #4)
> Hence, deleting the last message of a thread will be failed due to the
> limitation mentioned above.
More specifically speaking,
1. This will only fail when deleting the latest message of a thread with other messages available.
2. When deleting thread, the 1st save message will be deleted firstly.
   Hence, the deletion of thread is working even when the device storage is low.

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #6)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #4)
> > Hence, deleting the last message of a thread will be failed due to the
> > limitation mentioned above.
> More specifically speaking,
> 1. This will only fail when deleting the latest message of a thread with
> other messages available.
> 2. When deleting thread, the 1st save message will be deleted firstly.
>    Hence, the deletion of thread is working even when the device storage is
> low.
One more restriction for deletion is that if there is any unread messages to be deleted and the thread won't be empty after deleting, then it will be failed to update the unreadCount of the thread.

Comment 8

[Preeti Raghunath(:Preeti)]

Wayne

Too late for RIL changes. Please help get partner's OK on not doing RIL change at this time.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 1 v1: Deprecate the Optional Property |isDiskFull| in MobileMessageDB.

Part1: Deprecate the Optional Property |isDiskFull| in MobileMessageDB.

Hi, Vicamo,

May I have your review for this change?

Thanks!

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 2 v1: Check Failure Cause according to |event.target.error.name|.

Part 2: Check Failure Cause according to |event.target.error.name|. r=vyang

Hi, Vicamo,

May I have your review for this change?

Thanks!

Comment 11

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 3 v1: Remove the edundant function name from the implementation of EventHandler.

Part 3: Remove the edundant function name from the implementation of EventHandler.

Hi, Vicamo,

May I have your review for this change?

Thanks!

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 4 v1: Update threads at once after all messages are deleted.

Part 4: Update threads at once after all messages are deleted.

Hi, Vicamo,

May I have your review for this change?

Thanks!

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 5 v1: Re-write test cases with 'disk-space-watcher' notification.

Part 5 v1: Re-write test cases with 'disk-space-watcher' notification.

Hi, Vicamo,

May I have your review for this change?

Thanks!

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Summarize the impact after applying this change:
It's allowed to delete messages when storage is full with the following restrictions:
If the deletion won't remove all the messages in a thead and
1. the last message of the thread or
2. an unread message
are to be deleted.

Due to thread-based design in MobileMessageDB and transaction-based design
in indexedDB, this will trigger an update of the thread's |subject|, |message body|,
|unread count|, but the update operation is restricted by indexedDB when storage is full.

To allow user to clear an entire thread even an unread message thread, then
a follow-up in gaia is needed to pass the ids of the messages to be 
deleted at once instead of one by one to prevent any unnecessary update of
the thread's unread count.

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #14)
> To allow user to clear an entire thread even an unread message thread, then
> a follow-up in gaia is needed to pass the ids of the messages to be 
> deleted at once instead of one by one to prevent any unnecessary update of
> the thread's unread count.

NI Julien to see the possibility of having message ids at once when deleting
threads in the thread-list UI?

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8468296 [details] [diff] [review]
Patch Part 2 v1: Check Failure Cause according to |event.target.error.name|.

Review of attachment 8468296 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/mobilemessage/src/gonk/MobileMessageDB.jsm
@@ +3306,5 @@
>     *        id, and message timestamp.
>     */
>    transact: function(mmdb, txn, error, filter, reverse, collect) {
>      if (error) {
> +      //TODO look at event.target.error.name, pick appropriate error constant.

nit: // TODO

Comment 17

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8468299 [details] [diff] [review]
Patch Part 4 v1: Update threads at once after all messages are deleted.

Review of attachment 8468299 [details] [diff] [review]:
-----------------------------------------------------------------

Nice job!

::: dom/mobilemessage/src/gonk/MobileMessageDB.jsm
@@ +3050,5 @@
> +                                           threadInfo.removedMsgIds,
> +                                           threadInfo.ignoredUnreadCount,
> +                                           deletedInfo);
> +        }
> +      };

nit: please place an empty line after this line.

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8468300 [details] [diff] [review]
Patch Part 5 v1: Re-write test cases with 'disk-space-watcher' notification.

Review of attachment 8468300 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/mobilemessage/tests/marionette/test_mmdb_full_storage.js
@@ +18,5 @@
>      iccId: "1029384756"
>    };
>  }
>  
> +function setStorageFull(enabled) {

nit: aFull

@@ +22,5 @@
> +function setStorageFull(enabled) {
> +  SpecialPowers.notifyObserversInParentProcess(null,
> +                                               "disk-space-watcher",
> +                                               enabled ? "full" : "free");
> +

nit: empty line.

@@ -32,5 @@
>  
>  function testSaveSendingMessage(aMmdb) {
>    log("testSaveSendingMessage()");
>  
> -  gIsDiskFull = true;

Hi, I'd still want to preserve this so that we may know it's under disk full status at the instant you read this line. We're flipping disk full status to and from again and again in this script. An explicit indication would be really helpful.

@@ +110,5 @@
> +  let numOfTestMessages = 5;
> +
> +  // Save several unread messages to the same thread then delete them.
> +  for (let i = 0; i < numOfTestMessages; i++) {
> +    promises.push(saveReceivedMessage(aMmdb, newSavableMessage(testAddress))

We need an |setStorageFull(true)| before the for-loop, don't we?

@@ +114,5 @@
> +    promises.push(saveReceivedMessage(aMmdb, newSavableMessage(testAddress))
> +      .then((aValue) => { savedMsgIds.push(aValue[1].id); }));
> +  }
> +
> +  setStorageFull(false);

Not here. At the moment you have:

  promises.push(saveReceivedMessage(aMmdb, newSavableMessage(testAddress))

the |saveReceivedMessage(...)| has been executed and returned a deferred promise.

@@ +120,5 @@
> +    .then(() => setStorageFull(true))
> +    // Failure is expected when deleting the last one.
> +    .then(() => deleteMessage(aMmdb, [savedMsgIds[numOfTestMessages - 1]], 1))
> +    .then(null, (aValue) => aValue)
> +    .then(isCallbackStorageFullError)

Please add empty lines to separate adjacent blocks.

  return Promise.all(promises)

    // Failure is expected when deleting the last one.
    .then(() => setStorageFull(true))
    ...

    // Failure is expected when deleting an unread message.
    ...

    // Delete all messages in the thread.
    ...

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8468300 [details] [diff] [review]
Patch Part 5 v1: Re-write test cases with 'disk-space-watcher' notification.

Review of attachment 8468300 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/mobilemessage/tests/marionette/test_mmdb_full_storage.js
@@ -37,1 @@
>    return saveSendingMessage(aMmdb, newSavableMessage())

Thanks for explaining this.
I'll replace it with |setStorageFull(true)| for better understanding.

@@ +110,5 @@
> +  let numOfTestMessages = 5;
> +
> +  // Save several unread messages to the same thread then delete them.
> +  for (let i = 0; i < numOfTestMessages; i++) {
> +    promises.push(saveReceivedMessage(aMmdb, newSavableMessage(testAddress))

I'll move |setStorageFull(false)| at line#118 before this for-loop.

@@ +114,5 @@
> +    promises.push(saveReceivedMessage(aMmdb, newSavableMessage(testAddress))
> +      .then((aValue) => { savedMsgIds.push(aValue[1].id); }));
> +  }
> +
> +  setStorageFull(false);

Oops! You are right. Sorry for misleading.

Comment 20

[Julien Wajsberg [:julienw]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #15)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #14)
> > To allow user to clear an entire thread even an unread message thread, then
> > a follow-up in gaia is needed to pass the ids of the messages to be 
> > deleted at once instead of one by one to prevent any unnecessary update of
> > the thread's unread count.
> 
> NI Julien to see the possibility of having message ids at once when deleting
> threads in the thread-list UI?

I think Oleg has a patch about this somewhere (I even thought it was landed already ;) ).

Oleg, do you remember where you have this code?

Comment 21

[Jason Smith [:jsmith]]

Backlog - too risky to take at this point of the release, so we're going to take this for a future release. If the partner desires to have this patch on 2.0, then triage would suggest that the patches be added to their custom gaia/gecko build for 2.0.

Comment 22

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #20)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #15)
> > (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #14)
> > > To allow user to clear an entire thread even an unread message thread, then
> > > a follow-up in gaia is needed to pass the ids of the messages to be 
> > > deleted at once instead of one by one to prevent any unnecessary update of
> > > the thread's unread count.
> > 
> > NI Julien to see the possibility of having message ids at once when deleting
> > threads in the thread-list UI?
> 
> I think Oleg has a patch about this somewhere (I even thought it was landed
> already ;) ).
> 
> Oleg, do you remember where you have this code?

Yeah, I've just extracted related code from my too-old-experimental-branch:

https://github.com/azasypkin/gaia/commit/94af0418be892ad393f1bf5eb6bc7b9569a08b37

I'm going to move forward with it once patch for bug 1041124 is landed.

Comment 23

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 5 v2: Re-write test cases with 'disk-space-watcher' notification.

1. Have |setStorageFull(true)| and |setStorageFull(false)| as a closure of each test case.
2. |setStorageFull(false)| before saving test messages.

Hi Vicamo,

May I have your review for this patch again?

Thanks!

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8468982 [details] [diff] [review]
Patch Part 5 v2: Re-write test cases with 'disk-space-watcher' notification.

Review of attachment 8468982 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you!

Comment 25

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 1: Deprecate the Optional Property |isDiskFull| in MobileMessageDB. r=vyang

Update final patch.

Comment 26

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 2: Check Failure Cause according to |event.target.error.name|. r=vyang.

update final patch to address nit in comment 16.

Comment 27

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 3: Remove the edundant function name from the implementation of EventHandler. r=vyang

Rebase.

Comment 28

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 4: Update threads at once after all messages are deleted. r=vyang

update final patch to address nit in comment 17.

Comment 29

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 5: Re-write test cases with 'disk-space-watcher' notification. r=vyang

update final patch.

Comment 30

[Bevis Tseng[:bevis][:btseng](Exited)]

try server result is green:
https://tbpl.mozilla.org/?tree=Try&rev=dbfe141f5a72

Comment 31

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/integration/b2g-inbound/rev/3deaa74ba2ab
https://hg.mozilla.org/integration/b2g-inbound/rev/b21936aad355
https://hg.mozilla.org/integration/b2g-inbound/rev/080e4bbc0f30
https://hg.mozilla.org/integration/b2g-inbound/rev/76595550f3a1
https://hg.mozilla.org/integration/b2g-inbound/rev/55e9f9515a88

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3deaa74ba2ab
https://hg.mozilla.org/mozilla-central/rev/b21936aad355
https://hg.mozilla.org/mozilla-central/rev/080e4bbc0f30
https://hg.mozilla.org/mozilla-central/rev/76595550f3a1
https://hg.mozilla.org/mozilla-central/rev/55e9f9515a88

Comment 33

[Wayne Chang [:wchang]]

[Blocking Requested - why for this release]:
Needed by partner to correct indexedDB behaviour when storage is low. There is no RIL interface change involved nor does this impact any partner RIL implementation.

Adding :m1 to comment if there are any concerns at their end.

Comment 34

[Michael Vines [:m1] [:evilmachines]]

My only comment is that remind that we've stopped importing tip v2.0 gecko/gaia wholesale until the existing gecko/gaia stability issues are resolved, so it would be a couple weeks at best before we pick up this bug in our build (like all other non-CAF v2.0 blockers).

Comment 35

[bhavana bajaj [:bajaj]]

(In reply to Wayne Chang [:wchang] from comment #33)
> [Blocking Requested - why for this release]:
> Needed by partner to correct indexedDB behaviour when storage is low. There
> is no RIL interface change involved nor does this impact any partner RIL
> implementation.
> 
> Adding :m1 to comment if there are any concerns at their end.

wayne,firstly I do not think the risk part was  ever accounted here which is the main reason for push back. I think given where we are in the cycle this is too late for 2.0 if we want to give a stable build to the partner. As :mvines noted, the downstream(OEM/ODM) partner will not get it until too late, leaving this absolutely untested.

Comment 36

[bhavana bajaj [:bajaj]]

CAF reached out to us offline and is ok to cherry-pick this in their current build given the customer requirement. I have also discussed the risk offline with Bevis and he is comfortable with the changes given this is well tested and has automated tests. I am going to block on this, but if we have any fallouts from this change we should back this out immediately.

Bevis, can you please ensure to test this on 2.0 once it lands or reach out to QA for help here?

Comment 37

[Julien Wajsberg [:julienw]]

I think we need a fix in Gaia::SMS to make this work properly, so that we can delete without writing (see comment 22). Without this change, I think the gecko patch does not bring any user-facing improvement.

Bevis, do you agree?

(setting NO_UPLIFT until we have more information about this)

Comment 38

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #37)
> I think we need a fix in Gaia::SMS to make this work properly, so that we
> can delete without writing (see comment 22).

Just for the record, we already have Gaia::SMS patch ready for review in bug 1053952 for this.

Comment 39

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] from comment #37)
> I think we need a fix in Gaia::SMS to make this work properly, so that we
> can delete without writing (see comment 22). Without this change, I think
> the gecko patch does not bring any user-facing improvement.
> 
> Bevis, do you agree?
> 
> (setting NO_UPLIFT until we have more information about this)

NI Wayne to see if gaia part (bug 1053952) is needed or will be done by customer instead.

Comment 40

[Wayne Chang [:wchang]]

Correct, apps are handled by partner on their end on their product.

We can plan to do the same handling for a future release, Julien if you will, please file a bug for that improvement and maybe nominate it for 2.1? Partner contribution for this may come at a later stage if we don't do it by then.

Thanks

Comment 41

[Julien Wajsberg [:julienw]]

(In reply to Wayne Chang [:wchang] from comment #40)
> Correct, apps are handled by partner on their end on their product.
> 
> We can plan to do the same handling for a future release, Julien if you
> will, please file a bug for that improvement and maybe nominate it for 2.1?

Bug is already filed and I won't nominate it. It will be done as time allows, we have enough work "needed" for 2.1 for now.

As I understand it, CAF wants it in 2.0 though. Oleg's patch is ready for a first review so maybe it's doable in one of the next sprints. I just don't want to make it a requirement unless CAF really wants it.

Comment 42

[Michael Vines [:m1] [:evilmachines]]

(In reply to Julien Wajsberg [:julienw] (PTO 08/20 -> 09/15; contact schung instead) from comment #41)
> As I understand it, CAF wants it in 2.0 though. Oleg's patch is ready for a
> first review so maybe it's doable in one of the next sprints. I just don't
> want to make it a requirement unless CAF really wants it.

We don't need that bug to be fixed in v2.0.  The customer managing this for the v2.0 release is fine for us.

Comment 43

[Ryan VanderMeulen (PTO, back 6-April)]

Sounds like the 2.0+ blocking status should be cleared and this should be wontfix for 2.0?

Comment 44

[Wayne Chang [:wchang]]

There were some offline discussion with CAF and it was decided that this bug 1046026 should land on 2.0.

Sorry Ryan, didn't see the [no_uplift] tag here so had overlooked.

Comment 45

[Ryan VanderMeulen (PTO, back 6-April)]

At this point, it'll need b2g32 approval per the policy changed announced a couple days ago.

Comment 46

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8469135 [details] [diff] [review]
Patch Part 1: Deprecate the Optional Property |isDiskFull| in MobileMessageDB. r=vyang

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Customer need the deletion operation can be done when storage full to meet their requirement. This patch provides this possibility for customer to improve SMS experience in their own message app enhancement.
Testing completed: Yes. Test case is included in Patch#5.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:N/A

Comment 47

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8469137 [details] [diff] [review]
Patch Part 2: Check Failure Cause according to |event.target.error.name|. r=vyang.

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Customer need the deletion operation can be done when storage full to meet their requirement. This patch provides this possibility for customer to improve SMS experience in their own message app enhancement.
Testing completed: Yes. Test case is included in Patch#5.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:N/A

Comment 48

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8469138 [details] [diff] [review]
Patch Part 3: Remove the edundant function name from the implementation of EventHandler. r=vyang

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Customer need the deletion operation can be done when storage full to meet their requirement. This patch provides this possibility for customer to improve SMS experience in their own message app enhancement.
Testing completed: Yes. Test case is included in Patch#5.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:N/A

Comment 49

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8469139 [details] [diff] [review]
Patch Part 4: Update threads at once after all messages are deleted. r=vyang

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Customer need the deletion operation can be done when storage full to meet their requirement. This patch provides this possibility for customer to improve SMS experience in their own message app enhancement.
Testing completed: Yes. Test case is included in Patch#5.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:N/A

Comment 50

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8469140 [details] [diff] [review]
Patch Part 5: Re-write test cases with 'disk-space-watcher' notification. r=vyang

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Customer need the deletion operation can be done when storage full to meet their requirement. This patch provides this possibility for customer to improve SMS experience in their own message app enhancement.
Testing completed: Yes. Test case is included in Patch#5.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:N/A

Comment 51

[bhavana bajaj [:bajaj]]

(In reply to at-kitamura from comment #1)
> [Blocking Requested - why for this release]:
> Nominating this for 2.0, as partners need this on a product.

can you also verify by doing some testing here on your side to ensure there are no regressions/fallouts ? Thanks!

Comment 52

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/41cb5db839d1
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/8baa57ac7ba7
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/9ee3868eff09
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/fb535f7cbf59
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/41ca5dcfb079

Comment 53

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/41cb5db839d1
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/8baa57ac7ba7
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/9ee3868eff09
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/fb535f7cbf59
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/41ca5dcfb079

Comment 54

[at-kitamura]

(In reply to bhavana bajaj [:bajaj] from comment #51)
> (In reply to at-kitamura from comment #1)
> > [Blocking Requested - why for this release]:
> > Nominating this for 2.0, as partners need this on a product.
> 
> can you also verify by doing some testing here on your side to ensure there
> are no regressions/fallouts ? Thanks!

We tried to pick these implementation and checked behavior when storage capacity less than 5MB.

 - could delete messages
 - MobileMessageDB.jsm returned error information

These also seem OK for us.

Thanks for your early implementation.

Comment 55

[Julien Wajsberg [:julienw]]

Should we also uplift bug 1053952?

Bevis, can you precise what's missing if we don't uplift the Gaia patch ?

Comment 56

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (PTO 08/19 -> 09/15; contact schung for SMS matters) from comment #55)
> Should we also uplift bug 1053952?
> 
> Bevis, can you precise what's missing if we don't uplift the Gaia patch ?

Sure,
It has addressed in comment 14:
1. Any deleting operation that will trigger an update of thread is forbidden by indexDB when storage is full.
2. In current thread deletion operation, the message ids of the thread to be deleted is provided one by one by gaia which might trigger the update if the last message or an unread message is deleted with other messages available.

But there is no concern from partner side for this in 2.0. :)

Comment 57

[Julien Wajsberg [:julienw]]

Kitamura-san, can you please give your advice on comment 56?