Fix the issue with adding/removing event listeners for Device Motion/Orientation.

Source/modules/device_orientation/DeviceMotionController.cpp

 /*
  * Copyright 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Samsung Electronics. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *  * Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  *  * Redistributions in binary form must reproduce the above copyright
@@ skipping 11 matching lines @@
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "modules/device_orientation/DeviceMotionController.h"
 
 #include "RuntimeEnabledFeatures.h"
 #include "core/dom/Document.h"
+#include "core/frame/DOMWindow.h"
 #include "core/page/Page.h"
 #include "modules/device_orientation/DeviceMotionData.h"
 #include "modules/device_orientation/DeviceMotionDispatcher.h"
 #include "modules/device_orientation/DeviceMotionEvent.h"
 
 namespace WebCore {
 
 DeviceMotionController::DeviceMotionController(Document& document)
     : DeviceSensorEventController(document)
     , DOMWindowLifecycleObserver(document.domWindow())
@@ skipping 44 matching lines @@
 {
     DeviceMotionDispatcher::instance().removeDeviceMotionController(this);
 }
 
 bool DeviceMotionController::isNullEvent(Event* event)
 {
     DeviceMotionEvent* motionEvent = toDeviceMotionEvent(event);
     return !motionEvent->deviceMotionData()->canProvideEventData();
 }
 
-void DeviceMotionController::didAddEventListener(DOMWindow*, const AtomicString& eventType)
+void DeviceMotionController::didAddEventListener(DOMWindow* window, const AtomicString& eventType)
 {
-    if (eventType == EventTypeNames::devicemotion && RuntimeEnabledFeatures::deviceMotionEnabled()) {
+    if (eventType == EventTypeNames::devicemotion
+        && RuntimeEnabledFeatures::deviceMotionEnabled()
+        && window->hasEventListeners(EventTypeNames::devicemotion)) {

[Inactive, 2014/04/17 14:38:20]

How can this new condition ever be false?

[Inactive, 2014/04/17 14:43:17]

Unless I am mistaken, when didAddEventListener() is called, we always have at
least 1 listener so window->hasEventListeners(EventTypeNames::devicemotion)
would always be true, right? What we would like to know if there is only 1
listener.

Admittedly, I don't know if it is an issue to call startUpdating() too many
times. The most important is that we fix the didRemoveEventListener() case so
that we don't call stopUpdating() prematurely. 

[timvolodine, 2014/04/17 15:34:13]

I think you are right. Also it cannot hurt to call startUpdating() multiple
times (as it will be a noop after the first time). So removed this check here.
done.

+
         if (page() && page()->visibilityState() == PageVisibilityStateVisible)
             startUpdating();
+
         m_hasEventListener = true;
     }
 }
 
-void DeviceMotionController::didRemoveEventListener(DOMWindow*, const AtomicString& eventType)
+void DeviceMotionController::didRemoveEventListener(DOMWindow* window, const AtomicString& eventType)
 {
-    if (eventType == EventTypeNames::devicemotion) {
+    if (eventType == EventTypeNames::devicemotion
+        && RuntimeEnabledFeatures::deviceMotionEnabled()
+        && !window->hasEventListeners(EventTypeNames::devicemotion)) {

[Inactive, 2014/04/17 14:38:20]

Right, much better than my "counter" idea :)

[Peter Beverloo, 2014/04/17 14:48:17]

nit: you could early return here.

[timvolodine, 2014/04/17 15:34:13]

Done.

[timvolodine, 2014/04/17 15:34:13]

;)

+
         stopUpdating();
         m_hasEventListener = false;
     }
 }
 
 void DeviceMotionController::didRemoveAllEventListeners(DOMWindow*)
 {
     stopUpdating();
     m_hasEventListener = false;
 }
 
 } // namespace WebCore