Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Device Orientation - Make EnableDeviceMotion finer grain

RESOLVED FIXED in mozilla14

Status

()

Core
Geolocation
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

unspecified
mozilla14
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
EnableDeviceMotion currently enables all sensors:

        EnableSensor(hal::SENSOR_ORIENTATION);                                             EnableSensor(hal::SENSOR_ACCELERATION);                                            EnableSensor(hal::SENSOR_LINEAR_ACCELERATION);                                     EnableSensor(hal::SENSOR_GYROSCOPE); 


We can do better by only requiring SENSOR_ORIENTATION for device orientation and the other 3 for device motion.
(Assignee)

Updated

5 years ago
Assignee: nobody → doug.turner
(Assignee)

Comment 1

5 years ago
Created attachment 608867 [details] [diff] [review]
patch 1/2.  This makes EnableDeviceMotion finer grain.
Attachment #608867 - Flags: review?(mwu)
(Assignee)

Comment 2

5 years ago
Created attachment 608869 [details] [diff] [review]
patch 2/2.  This renames device motion to device sensors

since this class is really does more than device motion, lets rename the fucker.
Attachment #608869 - Flags: review?(mwu)

Updated

5 years ago
Attachment #608869 - Flags: review?(mwu) → review+

Comment 3

5 years ago
Comment on attachment 608867 [details] [diff] [review]
patch 1/2.  This makes EnableDeviceMotion finer grain.

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

Not entirely sure if I'm qualified to review this, but I think I understand what's going on.

::: content/events/src/nsEventListenerManager.cpp
@@ +84,5 @@
>  #include "nsIContentSecurityPolicy.h"
>  #include "nsJSEnvironment.h"
>  #include "xpcpublic.h"
>  #include "sampler.h"
> +#

?

::: dom/base/nsGlobalWindow.cpp
@@ +10060,5 @@
>  
>    if (!suspended) {
> +    nsCOMPtr<nsIDeviceMotion> ac = do_GetService(NS_DEVICE_MOTION_CONTRACTID);
> +    if (ac)
> +      ac->PauseAllSensors();

Shouldn't we disable just the sensors for this window instead all sensors?

::: dom/system/nsDeviceMotion.cpp
@@ +134,5 @@
>      if (NS_SUCCEEDED(rv) && bvalue == false)
>        mEnabled = false;
>    }
> +
> +  for (int i=0; i<NUM_SENSOR_TYPE; i++) {

This file appears to use spaces around operators like = and <.

@@ +181,3 @@
>  {
> +  for (int i=0; i<NUM_SENSOR_TYPE; i++) {
> +    if (IsSensorEnabled(i))

This check seems unnecessary
(Assignee)

Comment 4

5 years ago
Created attachment 608921 [details] [diff] [review]
patch 1/2.  This makes EnableDeviceMotion finer grain. v.2
Attachment #608867 - Attachment is obsolete: true
Attachment #608921 - Flags: review?(mwu)
Attachment #608867 - Flags: review?(mwu)

Comment 5

5 years ago
Comment on attachment 608921 [details] [diff] [review]
patch 1/2.  This makes EnableDeviceMotion finer grain. v.2

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

Looks good to me.

::: dom/base/nsGlobalWindow.h
@@ +539,5 @@
>    virtual nsresult DispatchSyncPopState();
>  
> +  virtual void EnableDeviceSensor(PRUint32 aType);
> +  virtual void DisableDeviceSensor(PRUint32 aType);
> +  nsTArray<PRUint32> mEnabledSensors;

This could probably be private/protected.

::: dom/system/nsDeviceMotion.h
@@ +74,5 @@
> +  // sensor -> window listener
> +  nsTArray<nsTArray<nsIDOMWindow*>* > mWindowListeners;
> +  
> +  // window -> sensortype enabled
> +  nsDataHashtable<nsUint32HashKey, nsTArray<PRUint32 > > mSensorsEnabled;

Remove
Attachment #608921 - Flags: review?(mwu) → review+

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/246d43704fc7
https://hg.mozilla.org/mozilla-central/rev/df108e67c87a
https://hg.mozilla.org/mozilla-central/rev/b3b3928b2c0f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14

Updated

5 years ago
Depends on: 740252
You need to log in before you can comment on or make changes to this bug.