We need to rename every single line for the SMS database codes like: s/Sms/MobileMessage and reallocate all the related files into a more generic folder from: dom/sms/interfaces/nsISmsDatabaseService.idl dom/sms/src/ril/SmsDatabaseService.js to: dom/mobilemessage/interfaces/nsIMobileMessageDatabaseService.idl dom/mobilemessage/src/ril/MobileMessageDatabaseService.js This is a big work and needs to be merged fast after reviewed, since lots of engineers are working on the SMS database things and it's easy to produce conflicts. After this is done, at bug 811252 we need to further redesign the database APIs for MMS-specific functions (also keep it backward-compatible to SMS at the same time). Cc'ing some guys used to be involved in the SMS database design.

Is it really worth the hassle to move/rename all this code?

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #1) > Is it really worth the hassle to move/rename all this code? Honestly, I don't have strong opinion on that. After discussing with Vicamo, we're just hoping to make the naming things and architecture more generic before we really start to include the MMS database APIs. Indeed, It's a bit dangerous to rename/move all the codes since our SMS database APIs have already been there for a long time and used in lots of place. Another happy solution might be providing another |nsIMmsDatabaseService| for MMS database and all its API implementation will be redirected to (or constructed by) the current |nsISmsDatabaseService| APIs. Or even directly use |nsISmsDatabaseService| in MMS if we can tolerate to see the SMS keyword in the MMS codes. Do others have any preferences? I'm still open to doing this or not.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #1) > Is it really worth the hassle to move/rename all this code? For me, yes. If there will be only one mobilemessage API in the end as defined in bug 760065, then mobilemessage/ should be its final home and MobileMessageDatabaseService should be its final name. If your concern is mostly based on naming differences from b2g18 or other stable branches, it will be something we can't really afford all the time. More and more breaking changes are going to be landed in m-c and MobileMessage is no exception.

(In reply to Gene Lian [:gene] from comment #5) > Created attachment 707563 [details] [diff] [review] > Part 2, create dom/mobilemessage to put DB codes Btw, the Bugzilla Diff reviewer cannot parse and show the renaming change very well like: diff --git a/dom/sms/interfaces/nsISmsDatabaseService.idl b/dom/mobilemessage/interfaces/nsIMobileMessageDatabaseService.idl rename from dom/sms/interfaces/nsISmsDatabaseService.idl rename to dom/mobilemessage/interfaces/nsIMobileMessageDatabaseService.idl diff --git a/dom/sms/interfaces/nsIRilSmsDatabaseService.idl b/dom/mobilemessage/interfaces/nsIRilMobileMessageDatabaseService.idl rename from dom/sms/interfaces/nsIRilSmsDatabaseService.idl rename to dom/mobilemessage/interfaces/nsIRilMobileMessageDatabaseService.idl Have to check it in the raw diff file.

Comment on attachment 707563 [details] [diff] [review] Part 2, create dom/mobilemessage to put DB codes Review of attachment 707563 [details] [diff] [review]: ----------------------------------------------------------------- Gene, can we also move/rename SmsDatabaseService.*? ::: dom/dom-config.mk @@ +12,5 @@ > dom/media \ > dom/network/src \ > dom/settings \ > dom/phonenumberutils \ > + dom/mobilemessage/src \ Do we really need it now?

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #7) > Comment on attachment 707563 [details] [diff] [review] > Part 2, create dom/mobilemessage to put DB codes > > Review of attachment 707563 [details] [diff] [review]: > ----------------------------------------------------------------- > > Gene, can we also move/rename SmsDatabaseService.*? I don't quite understand. I've already moved/renamed all the files. Maybe comment #6 makes you missed the changes. ;) > ::: dom/dom-config.mk > @@ +12,5 @@ > > dom/media \ > > dom/network/src \ > > dom/settings \ > > dom/phonenumberutils \ > > + dom/mobilemessage/src \ > > Do we really need it now? Why not? All the SmsDatabaseService.* have been moved to dom/mobilemessage/src (into the sub-catogories android/fallback/ril respectively) and renamed as MobileMessageDatabaseService.*. I can talk to you in person later. ;)

(In reply to Gene Lian [:gene] from comment #8) > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #7) > > Gene, can we also move/rename SmsDatabaseService.*? > > I don't quite understand. I've already moved/renamed all the > files. Maybe comment #6 makes you missed the changes. ;) Ah, it does. :) > > ::: dom/dom-config.mk > > @@ +12,5 @@ > > > dom/media \ > > > dom/network/src \ > > > dom/settings \ > > > dom/phonenumberutils \ > > > + dom/mobilemessage/src \ > > > > Do we really need it now? > > Why not? All the SmsDatabaseService.* have been moved to > dom/mobilemessage/src (into the sub-catogories > android/fallback/ril respectively) and renamed as > MobileMessageDatabaseService.*. I can talk to you in person > later. ;) Just don't pollute dom-config if not necessary. See bug 778093 comment #33.

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #9) > Just don't pollute dom-config if not necessary. See bug 778093 comment #33. Indeed, we don't this. :) Thanks for the feedback. I'll upload formal patches for your reviews after having more tests to make sure I wouldn't mess up the existing SMS functions.

Fixed compiling errors on android/fallback plarforms.

Wrong patch. Sorry for the noise.

See some try results first: https://tbpl.mozilla.org/?tree=Try&rev=ae201e0f511b

Comment on attachment 708509 [details] [diff] [review] Part 2, create dom/mobilemessage to put DB codes, V2.1 Review of attachment 708509 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobilemessage/src/Makefile.in @@ +62,5 @@ > + $(NULL) > + > +CPPSRCS += \ > + MobileMessageDatabaseService.cpp \ > + $(NULL) trailing space ::: dom/sms/src/android/SmsDatabaseService.cpp @@ +2,5 @@ > /* This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this file, > * You can obtain one at http://mozilla.org/MPL/2.0/. */ > > +#include "mozilla/dom/sms/SmsFilter.h" Please add an additional entry in LOCAL_INCLUDES in Makefile instead. @@ +8,3 @@ > #include "AndroidBridge.h" > > +using namespace mozilla::dom::sms; As :mounir suggested, we should have mozilla::dom only, but this can be another follow-up. ::: dom/sms/src/SmsServicesFactory.cpp @@ +7,5 @@ > #include "nsXULAppAPI.h" > #include "SmsService.h" > #include "SmsIPCService.h" > #ifndef MOZ_B2G_RIL > +#include "mozilla/dom/mobilemessage/MobileMessageDatabaseService.h" ditto.

Rebased. r=vicamo

Rebased. r=vicamo

Rebased. r=vicamo

Rebased. r=vicamo Sorry for all the noises...

https://hg.mozilla.org/integration/mozilla-inbound/rev/97a64853f10c https://hg.mozilla.org/integration/mozilla-inbound/rev/1eab3e0d7f53

https://hg.mozilla.org/mozilla-central/rev/97a64853f10c https://hg.mozilla.org/mozilla-central/rev/1eab3e0d7f53

This bug relates to MMS features and needs to be tagged as leo+ so that we can uplift it into the b2g-18 branch.

Leo triage: leo+ for MMS blockers

This is a b2g-18-specific patch for part 1.

This is a b2g-18-specific patch for part 2.

https://hg.mozilla.org/releases/mozilla-b2g18/rev/6d8751e34809 https://hg.mozilla.org/releases/mozilla-b2g18/rev/7e97fff932ee