1132774 - [B2G][SMS] Enable DEBUG Flag in SmsService if the default value of "ril.debugging.enabled" is true.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

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

Similar to MmsService, we should enable the DEBUG flag in SmsService.js if the default value of "ril.debugging.enabled" is set to true.

After running the script of enable_debug_flags.sh in 
https://github.com/bevis-tseng/Debug_Tools
we shall be able to see the debug message in SmsService.js when sending/receiving SMS.

Comment 1

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

Hi Samael,

Would you like to take this bug to warm up? :)

Comment 2

[Samael Wang [:freesamael] (away for now)]

Attachment: bug-1132774-fix.patch

Comment 3

[Samael Wang [:freesamael] (away for now)]

try server result
https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab6f30acf6e

Comment 4

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

Comment on attachment 8569026 [details] [diff] [review]
bug-1132774-fix.patch

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

Please see my suggestion inline.

Thanks!

::: dom/mobilemessage/gonk/SmsService.js
@@ +100,5 @@
> +try {
> +  let debugPref = Services.prefs.getBoolPref(kPrefRilDebuggingEnabled);
> +  DEBUG = DEBUG || debugPref;
> +} catch (e) {}
> +

Similar logic is available in SmsService._updateDebugFlag().
I suggest to reuse this in the constructor of SmsService instead.

BTW, please revise the following line in _updateDebugFlag()
      DEBUG = RIL.DEBUG_RIL ||
              Services.prefs.getBoolPref(kPrefRilDebuggingEnabled);
to 
      DEBUG = DEBUG ||
              Services.prefs.getBoolPref(kPrefRilDebuggingEnabled);
because DEBUG has already been initialized with RIL.DEBUG_RIL at declaration.

@@ +105,1 @@
>  function SmsService() {

this._updateDebugFlag();

Comment 5

[Samael Wang [:freesamael] (away for now)]

Attachment: [B2G][SMS] Enable DEBUG Flag in SmsService if the default value of "ril.debugging.enabled" is true. r=btseng

Comment 6

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

Comment on attachment 8571170 [details] [diff] [review]
[B2G][SMS] Enable DEBUG Flag in SmsService if the default value of "ril.debugging.enabled" is true. r=btseng

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

Looks good to me. 

Please set r=me in the patch description and 
add "checkin-needed" in the field of |keywords| after you got positive result from try server.

Thanks!

Comment 7

[Samael Wang [:freesamael] (away for now)]

try server result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4276892baad2

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/95443c5c35fd

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/95443c5c35fd