Closed Bug 874069 Opened 6 years ago Closed Last year

1500+1st B2G can't use geolocation


(Core :: DOM: Geolocation, defect)

Not set





(Reporter: dougt, Unassigned, Mentored)


(Whiteboard: [lang=c++])


(3 files)

Because of the limit in geo:

It looks like we will not be able to have the 1501st application use geolocation.
josh, maybe we should make this list handling smarter?
For WatchPosition that would make a lot of sense. I doubt that the pending callbacks list needs anything better.
The other way we could sidestep the issue is by making each ContentParent hold on to a unique Geolocation object instead of having one global one for all IPC listeners.
good first bug?
Whiteboard: [mentor=jdm][lang=c++]
I want to work on this, please suggest where can I start?
In places in ContentParent where we get a global geolocation object (such as, we instead want to try storing a member variable in the constructor and using do_CreateInstance instead.
If i am getting this right?

I should store member variable in ContentParent.h

  nsCOMPtr<nsIDOMGeoGeolocation> mGeo;  //private

Then in ContentParent.cpp (under constructor)

  mGeo = do_CreateInstance(";1");

Then instead of 

 nsCOMPtr<nsIDOMGeoGeolocation> geo = do_GetService(";1");

just use mGeo pointer everywhere instead of creating as above in file ContentParent.cpp
Exactly right.
I applied these changes.
But there is compilation error-
 "d:\mozilla-source\mozilla-central\obj-i686-pc-mingw32\dist\include\nsIDOMGeoGeolocation.h(21) : fatal error C1083: Cannot open include file: 'DictionaryHelpers.h': No such file or directory"
You can get around that by simply forward-declaring nsIDOMGeolocation in the header file (ie. "class nsIDOMGeolocation;").
To be specific, you can forward-declare the class instead of including the header file.
Attachment #754499 - Flags: review?(josh)
Assignee: nobody → manu_talkin
Comment on attachment 754499 [details] [diff] [review]
Changed ContentParent class to use its own gelocation object instead of global geolocation object used through out application

Review of attachment 754499 [details] [diff] [review]:

Great! There are a couple changes to make, and then you should be sure to follow, so that your change has a commit message like "Bug 874069 - Use one geolocation instance object per ContentParent. r=jdm"

::: dom/ipc/ContentParent.cpp
@@ +1051,5 @@
>      // No more than one of !!aApp, aIsForBrowser, and aIsForPreallocated should
>      // be true.
>      MOZ_ASSERT(!!aApp + aIsForBrowser + aIsForPreallocated <= 1);
> +	// Create Geolocation object for this ContentParent

This comment doesn't really add much.

@@ +1052,5 @@
>      // be true.
>      MOZ_ASSERT(!!aApp + aIsForBrowser + aIsForPreallocated <= 1);
> +	// Create Geolocation object for this ContentParent
> +	mGeo = do_CreateInstance(";1");

Nit: please use spaces instead of tabs.

@@ +2371,5 @@
>      return true;
>  }
> +int32_t
> +ContentParent::AddGeolocationListener(nsIDOMGeoPositionCallback* watcher, bool highAccuracy)

Now that this is a member function, get rid of the watcher parameter and just use |this| instead.

@@ +2376,2 @@
>  {
> +  

Nit: delete this blank line.
Attachment #754499 - Flags: review?(josh) → feedback+
Attachment #755809 - Flags: review?(josh)
Comment on attachment 755809 [details] [diff] [review]
Use one geolocation instance object per ContentParent.

Review of attachment 755809 [details] [diff] [review]:

Great! I apologize for taking so long on this.
Attachment #755809 - Flags: review?(josh) → review+
Keywords: checkin-needed
Manu, can you look into the test failures that occurred?
Flags: needinfo?(manu_talkin)
 I ran ./mach xpcshell-test and found out that test passes for test_contentPrefs_parentipc.js
but there are other failures.
Any suggestions?
Flags: needinfo?(manu_talkin)
Oh, one thing that could make a difference is if you're not using a debug build. explains that you need a .mozconfig file in the mozilla-central root that contains "ac_add_options --enable-debug". You'll need to do a full rebuild, unfortunately.
Assignee: manu_talkin → nobody
Mentor: josh
Whiteboard: [mentor=jdm][lang=c++] → [lang=c++]
Mass closing as we are no longer working on b2g/firefox os.
Closed: Last year
Resolution: --- → WONTFIX
Mass closing as we are no longer working on b2g/firefox os.
You need to log in before you can comment on or make changes to this bug.