960961 - B2G RIL: fix and suppress some unnecessary ril logs

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Jessica Jong [:jessica] (inactive)]

Add back function names in |RIL[FOO] = function FOO() {...}|, or the following log will not show properly: 'Handling parcel as FOO'.

Comment 1

[Jessica Jong [:jessica] (inactive)]

I will also remove some logs and shouldn't appear in logcat.

Comment 2

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: add back function names in ril_worker.js

Comment 3

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: removes logs that shouldn't appear in logcat.

Removing lastKnown(Home)Network log as they contain sensitive information and MobileConnection log as it appears no matter ril debugging flag is enabled or not.

Comment 4

[Jessica Jong [:jessica] (inactive)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=26e3d9a73f39

Comment 5

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: removes logs that shouldn't appear in logcat, v2.

Modify dom/mobileconnection/src/MobileConnection.cpp instead of dom/network/src/MobileConnection.cpp.

Comment 6

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

Sorry for jumping in.

Could we also take the chance to add some more ril logs as https://bug960537.bugzilla.mozilla.org/attachment.cgi?id=8362404? I feel like these two benefit to debugging data connection setup. :)

Comment 7

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

Comment on attachment 8361597 [details] [diff] [review]
Part 1: add back function names in ril_worker.js

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

Thank you :)

Comment 8

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

Comment on attachment 8362408 [details] [diff] [review]
Part 2: removes logs that shouldn't appear in logcat, v2.

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

I think there are still a few unwrapped ones in ril_worker.  Could you also help wrapping them with DEBUG?  Thank you :)

Comment 9

[Jessica Jong [:jessica] (inactive)]

(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #6)
> Sorry for jumping in.
> 
> Could we also take the chance to add some more ril logs as
> https://bug960537.bugzilla.mozilla.org/attachment.cgi?id=8362404? I feel
> like these two benefit to debugging data connection setup. :)

Sure. I will upload another patch for this.

Comment 10

[Jessica Jong [:jessica] (inactive)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #8)
> Comment on attachment 8362408 [details] [diff] [review]
> Part 2: removes logs that shouldn't appear in logcat, v2.
> 
> Review of attachment 8362408 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think there are still a few unwrapped ones in ril_worker.  Could you also
> help wrapping them with DEBUG?  Thank you :)

Sure. I will check RadioInterfaceLayer.js and RILContentHelper.js as well.

Comment 11

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: add back function names in ril_worker.js (final)

Add r=vicamo.

Comment 12

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: remove ril logs or wrap them with DEBUG flag, v3.

- From comment 8, check also ril_worker.js, RadioInterfaceLayer.js and RILContentHelper.js.
- Add r=vicamo.

Comment 13

[Jessica Jong [:jessica] (inactive)]

(In reply to Jessica Jong [:jjong] [:jessica] from comment #12)
> Created attachment 8362823 [details] [diff] [review]
> Part 2: remove ril logs or wrap them with DEBUG flag, v3.
> 
> - From comment 8, check also ril_worker.js, RadioInterfaceLayer.js and
> RILContentHelper.js.
> - Add r=vicamo.

I am not sure if the debug message in onerror() in ril_worker.js is left without the DEBUG on purpose. Please let me know if it is the case.

Comment 14

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 3: add logs for setup/deactivate data call (final)

- Add logs as requested in comment 6.
- Also add the same logs in 'deactivateDataCallByType()'.
- Add r=vicamo.

Comment 15

[Jessica Jong [:jessica] (inactive)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=bde07f7f6b5c
Note: the failure in B2G ICS Emulator Debug (5) is being tracked in Bug 961107.

Comment 16

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

Comment on attachment 8362823 [details] [diff] [review]
Part 2: remove ril logs or wrap them with DEBUG flag, v3.

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

::: dom/system/gonk/ril_worker.js
@@ +6238,5 @@
>    // Always print the NITZ info so we can collection what different providers
>    // send down the pipe (see bug XXX).
>    // TODO once data is collected, add in |if (DEBUG)|
>  
> +  if (DEBUG) debug("DateTimeZone string " + dateString);

nit: please also remove "// Always ..." and add "// See also bug 714352 - Listen for NITZ updates from rild."

@@ +11481,5 @@
>        if (options.sw1 && options.sw2) {
>          errorMsg += "(" + options.sw1.toString(16) +
>                      "/" + options.sw2.toString(16) + ")";
>        }
> +      if (DEBUG) debug(errorMsg);

nit: this has been wrapped by DEBUG at line 11475.

Comment 17

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: remove ril logs or wrap them with DEBUG flag (final)

Address review comment 16.

Comment 18

[Jessica Jong [:jessica] (inactive)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=8aaf1a17af0b

Comment 19

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

   https://hg.mozilla.org/integration/b2g-inbound/rev/98f0077e1820
   https://hg.mozilla.org/integration/b2g-inbound/rev/356ce1bba953
   https://hg.mozilla.org/integration/b2g-inbound/rev/d32b34edd752

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/98f0077e1820
https://hg.mozilla.org/mozilla-central/rev/356ce1bba953
https://hg.mozilla.org/mozilla-central/rev/d32b34edd752