Closed
Bug 734855
Opened 12 years ago
Closed 12 years ago
Device Orientation - Make EnableDeviceMotion finer grain
Categories
(Core :: DOM: Geolocation, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: dougt, Assigned: dougt)
References
Details
Attachments
(2 files, 1 obsolete file)
21.67 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
26.57 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
Assignee: nobody → doug.turner
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #608867 -
Flags: review?(mwu)
Assignee | ||
Comment 2•12 years ago
|
||
since this class is really does more than device motion, lets rename the fucker.
Attachment #608869 -
Flags: review?(mwu)
Updated•12 years ago
|
Attachment #608869 -
Flags: review?(mwu) → review+
Comment 3•12 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•12 years ago
|
||
Attachment #608867 -
Attachment is obsolete: true
Attachment #608921 -
Flags: review?(mwu)
Attachment #608867 -
Flags: review?(mwu)
Comment 5•12 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•12 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
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•