Closed Bug 989692 Opened 11 years ago Closed 11 years ago

Add a setting to force more debugging information to console

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: dougt, Assigned: dougt)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
Attachment #8399021 - Flags: review?(josh)
Assignee: nobody → dougt
Attachment #8399022 - Flags: review?(fabrice)
Depends on: 989691
Attachment #8399021 - Attachment is patch: true
Attachment #8399021 - Attachment mime type: text/x-patch → text/plain
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
Attachment #8399021 - Flags: review?(josh) → review-
(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 on attachment 8399021 [details] [diff] [review] Add geolocation.debugging.enabled setting thanks kanru!
Attachment #8399022 - Flags: review?(fabrice) → review?(kchen)
Attachment #8399021 - Attachment is obsolete: true
Attachment #8400115 - Flags: review?(kchen)
Attachment #8399022 - Flags: review?(kchen) → review+
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".
Attachment #8400115 - Flags: review?(kchen) → review-
> You're almost there! how about I test something before sending it your way :(
Attachment #8401050 - Flags: review?(kchen)
Comment on attachment 8400115 [details] [diff] [review] Add geolocation.debugging.enabled setting >From e6113813eb7d4d1a9d8bc80fa8a641b9ac6f2e97 Mon Sep 17 00:00:00 2001 >From: Doug Turner <doug.turner@gmail.com> >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 >
Attachment #8400115 - Attachment is obsolete: true
Comment on attachment 8401050 [details] [diff] [review] Add geolocation.debugging.enabled setting Review of attachment 8401050 [details] [diff] [review]: ----------------------------------------------------------------- Nice!
Attachment #8401050 - Flags: review?(kchen) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/f70d7183aaef Please submit a pull request if the Gaia patch needs landing as well.
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Depends on: 999572
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: