989692 - Add a setting to force more debugging information to console

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Doug Turner (:dougt)]

Attachment: Add geolocation.debugging.enabled setting

We need to extract more debug information of what is going in a geolocation request in a FFOS build.  Having a setting to force output to console would be very helpful.

jdm, i am very sad that the settings api needs a window.

Comment 1

[Doug Turner (:dougt)]

Attachment: geolocation.debugging.enabled setting in gaia

Comment 2

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 8399021 [details] [diff] [review]
Add geolocation.debugging.enabled setting

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

Your could use nsISettingsService via the contract-id @mozilla.org/settingsService;1

Comment 3

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Kan-Ru Chen [:kanru] from comment #2)
> Comment on attachment 8399021 [details] [diff] [review]
> Add geolocation.debugging.enabled setting
> 
> Review of attachment 8399021 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Your could use nsISettingsService via the contract-id
> @mozilla.org/settingsService;1

The other parts of the patch looks good.

Comment 4

[Doug Turner (:dougt)]

Comment on attachment 8399021 [details] [diff] [review]
Add geolocation.debugging.enabled setting

thanks kanru!

Comment 5

[Doug Turner (:dougt)]

Attachment: Add geolocation.debugging.enabled setting

Comment 6

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 8400115 [details] [diff] [review]
Add geolocation.debugging.enabled setting

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

You're almost there!

::: dom/system/NetworkGeolocationProvider.js
@@ +88,5 @@
>      this.started = true;
>  
> +    try {
> +      let settings = Cc["@mozilla.org/settingsService;1"].getService(Ci.nsISettingsService);
> +      settings.addObserver(SETTING_DEBUG_ENABLED, function(setting) {

nsISettingsService does not have a addObserver interface. You need to use the observer service (Services.obs) and listen to the topic "mozsettings-changed".

Comment 7

[Doug Turner (:dougt)]

> You're almost there!

how about I test something before sending it your way  :(

Comment 8

[Doug Turner (:dougt)]

Attachment: Add geolocation.debugging.enabled setting

Comment 9

[Doug Turner (:dougt)]

Comment on attachment 8400115 [details] [diff] [review]
Add geolocation.debugging.enabled setting

>From e6113813eb7d4d1a9d8bc80fa8a641b9ac6f2e97 Mon Sep 17 00:00:00 2001
>From: Doug Turner <[e-mail]>
>Date: Mon, 24 Mar 2014 21:58:18 -0700
>Subject: [PATCH] Add GPS Debugging Logging
>
>---
> dom/system/NetworkGeolocationProvider.js       | 18 +++++++++++
> dom/system/gonk/GonkGPSGeolocationProvider.cpp | 44 +++++++++++++++-----------
> 4 files changed, 69 insertions(+), 18 deletions(-)
>
>diff --git a/dom/system/NetworkGeolocationProvider.js b/dom/system/NetworkGeolocationProvider.js
>index 114176f..68b23a1 100755
>--- a/dom/system/NetworkGeolocationProvider.js
>+++ b/dom/system/NetworkGeolocationProvider.js
>@@ -9,6 +9,7 @@ const Ci = Components.interfaces;
> const Cc = Components.classes;
> 
> const POSITION_UNAVAILABLE = Ci.nsIDOMGeoPositionError.POSITION_UNAVAILABLE;
>+const SETTING_DEBUG_ENABLED = "geolocation.debugging.enabled";
> 
> let gLoggingEnabled = false;
> 
>@@ -83,8 +84,25 @@ WifiGeoPositionProvider.prototype = {
>   startup:  function() {
>     if (this.started)
>       return;
>+
>     this.started = true;
> 
>+    try {
>+      let settings = Cc["@mozilla.org/settingsService;1"].getService(Ci.nsISettingsService);
>+      settings.addObserver(SETTING_DEBUG_ENABLED, function(setting) {
>+        if (setting.settingName == SETTING_DEBUG_ENABLED) {
>+          gLoggingEnabled = setting.settingValue;
>+        }
>+      });
>+      let lock = settings.mozSettings.createLock();
>+      let req = lock.get(SETTING_DEBUG_ENABLED);
>+      req.onsuccess = function (event) {
>+        gLoggingEnabled = event.target.result[SETTING_DEBUG_ENABLED];
>+      }
>+    } catch(ex) {
>+      // This platform doesn't have the settings interface, and that is just peachy
>+    }
>+
>     if (gWifiScanningEnabled) {
>       this.wifiService = Cc["@mozilla.org/wifi/monitor;1"].getService(Components.interfaces.nsIWifiMonitor);
>       this.wifiService.startWatching(this);
>diff --git a/dom/system/gonk/GonkGPSGeolocationProvider.cpp b/dom/system/gonk/GonkGPSGeolocationProvider.cpp
>index 7d67220..9c51c5e 100644
>--- a/dom/system/gonk/GonkGPSGeolocationProvider.cpp
>+++ b/dom/system/gonk/GonkGPSGeolocationProvider.cpp
>@@ -36,15 +36,18 @@
> #include "nsIRadioInterfaceLayer.h"
> #endif
> 
>+#define SETTING_DEBUG_ENABLED "geolocation.debugging.enabled"
>+
> #ifdef AGPS_TYPE_INVALID
> #define AGPS_HAVE_DUAL_APN
> #endif
> 
>-#define DEBUG_GPS 0
>+#define FLUSH_AIDE_DATA 0
> 
> using namespace mozilla;
> 
> static const int kDefaultPeriod = 1000; // ms
>+static int gGPSDebugging = false;
> 
> static const char* kNetworkConnStateChangedTopic = "network-connection-state-changed";
> 
>@@ -141,12 +144,11 @@ GonkGPSGeolocationProvider::SvStatusCallback(GpsSvStatus* sv_info)
> void
> GonkGPSGeolocationProvider::NmeaCallback(GpsUtcTime timestamp, const char* nmea, int length)
> {
>-#if DEBUG_GPS
>-  printf_stderr("*** nmea info\n");
>-  printf_stderr("timestamp:\t%lld\n", timestamp);
>-  printf_stderr("nmea:     \t%s\n", nmea);
>-  printf_stderr("length:   \t%d\n", length);
>-#endif
>+  if (gGPSDebugging) {
>+    nsContentUtils::LogMessageToConsole("NMEA: timestamp:\t%lld", timestamp);
>+    nsContentUtils::LogMessageToConsole("NMEA: nmea:     \t%s", nmea);
>+    nsContentUtils::LogMessageToConsole("NMEA  length:   \%d", length);
>+  }
> }
> 
> void
>@@ -538,13 +540,13 @@ GonkGPSGeolocationProvider::InjectLocation(double latitude,
>                                            double longitude,
>                                            float accuracy)
> {
>-#ifdef DEBUG_GPS
>-  printf_stderr("*** injecting location\n");
>-  printf_stderr("*** lat: %f\n", latitude);
>-  printf_stderr("*** lon: %f\n", longitude);
>-  printf_stderr("*** accuracy: %f\n", accuracy);
>-#endif
>-
>+  if (gGPSDebugging) {
>+    nsContentUtils::LogMessageToConsole("*** injecting location");
>+    nsContentUtils::LogMessageToConsole("*** lat: %f", latitude);
>+    nsContentUtils::LogMessageToConsole("*** lon: %f", longitude);
>+    nsContentUtils::LogMessageToConsole("*** accuracy: %f", accuracy);
>+  }
>+  
>   MOZ_ASSERT(NS_IsMainThread());
>   if (!mGpsInterface) {
>     return;
>@@ -617,7 +619,7 @@ GonkGPSGeolocationProvider::StartGPS()
>   mGpsInterface->set_position_mode(positionMode,
>                                    GPS_POSITION_RECURRENCE_PERIODIC,
>                                    update, 0, 0);
>-#if DEBUG_GPS
>+#if FLUSH_AIDE_DATA
>   // Delete cached data
>   mGpsInterface->delete_aiding_data(GPS_DELETE_ALL);
> #endif
>@@ -703,6 +705,7 @@ GonkGPSGeolocationProvider::Startup()
> {
>   MOZ_ASSERT(NS_IsMainThread());
> 
>+  RequestSettingValue(SETTING_DEBUG_ENABLED);
>   if (mStarted) {
>     return NS_OK;
>   }
>@@ -808,10 +811,10 @@ NS_IMETHODIMP
> GonkGPSGeolocationProvider::Handle(const nsAString& aName,
>                                    JS::Handle<JS::Value> aResult)
> {
>-  if (aName.EqualsLiteral("ril.supl.apn")) {
>-    JSContext *cx = nsContentUtils::GetCurrentJSContext();
>-    NS_ENSURE_TRUE(cx, NS_OK);
>+  JSContext *cx = nsContentUtils::GetCurrentJSContext();
>+  NS_ENSURE_TRUE(cx, NS_OK);
> 
>+  if (aName.EqualsLiteral("ril.supl.apn")) {
>     // When we get the APN, we attempt to call data_call_open of AGPS.
>     if (aResult.isString()) {
>       // NB: No need to enter a compartment to read the contents of a string.
>@@ -821,6 +824,11 @@ GonkGPSGeolocationProvider::Handle(const nsAString& aName,
>         SetAGpsDataConn(apn);
>       }
>     }
>+  } else if (aName.EqualsLiteral(SETTING_DEBUG_ENABLED)) {
>+    if (!aResult.isBoolean()) {
>+      return NS_ERROR_FAILURE;
>+    }
>+    gGPSDebugging = aResult.toBoolean();
>   }
>   return NS_OK;
> }
>-- 
>1.8.5.3
>

Comment 10

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 8401050 [details] [diff] [review]
Add geolocation.debugging.enabled setting

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

Nice!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f70d7183aaef

Please submit a pull request if the Gaia patch needs landing as well.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f70d7183aaef

Comment 13

[Doug Turner (:dougt)]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/3b6bce46fcef