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)
Core
DOM: Geolocation
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: dougt, Assigned: dougt)
References
Details
Attachments
(2 files, 2 obsolete files)
2.16 KB,
patch
|
kanru
:
review+
|
Details | Diff | Splinter Review |
6.15 KB,
patch
|
kanru
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•11 years ago
|
Attachment #8399021 -
Flags: review?(josh)
Assignee | ||
Comment 1•11 years ago
|
||
Assignee: nobody → dougt
Attachment #8399022 -
Flags: review?(fabrice)
Updated•11 years ago
|
Attachment #8399021 -
Attachment is patch: true
Attachment #8399021 -
Attachment mime type: text/x-patch → text/plain
Comment 2•11 years ago
|
||
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-
Comment 3•11 years ago
|
||
(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.
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 8399021 [details] [diff] [review]
Add geolocation.debugging.enabled setting
thanks kanru!
Assignee | ||
Updated•11 years ago
|
Attachment #8399022 -
Flags: review?(fabrice) → review?(kchen)
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8399021 -
Attachment is obsolete: true
Attachment #8400115 -
Flags: review?(kchen)
Updated•11 years ago
|
Attachment #8399022 -
Flags: review?(kchen) → review+
Comment 6•11 years ago
|
||
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-
Assignee | ||
Comment 7•11 years ago
|
||
> You're almost there!
how about I test something before sending it your way :(
Assignee | ||
Comment 8•11 years ago
|
||
Attachment #8401050 -
Flags: review?(kchen)
Assignee | ||
Comment 9•11 years ago
|
||
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 10•11 years ago
|
||
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+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 11•11 years ago
|
||
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
Comment 12•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Assignee | ||
Comment 13•11 years ago
|
||
status-b2g-v1.4:
--- → fixed
Updated•11 years ago
|
status-b2g-v2.0:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•