Bug 1204518 - Fix warnings in widget/gonk/. r=mwu.
authorNicholas Nethercote <nnethercote@mozilla.com>
Mon, 14 Sep 2015 18:08:56 -0700
changeset 295107 8dbe5f85a00892167aac0e68dc58354b0f3e9ca9
parent 295106 583b3c9b6e0e81126099a6d0b2efda174744a0f5
child 295108 a748cf7b0c31adc60074e85d3fbb92f1d2fdff89
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmwu
bugs1204518
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1204518 - Fix warnings in widget/gonk/. r=mwu. Warnings fixed include the following. - Several cases where macros (|LOG| and |ALOGE|) were redefined. I just did a simple #undef to fix these. - In GonkMemoryPressureMonitoring.cpp, "the address of NuwaMarkCurrentThread will never be NULL". - In OrientationObserver.cpp, several signed/unsigned comparison warnings. - Several warnings about variables that are unused or set but not used: in InputDispatcher.cpp, InputReader.cpp. Also in SpriteController, where several loops were all but empty if HAVE_ANDROID_OS is undefined; for these I moved the HAVE_ANDROID_OS check outside the loop. The patch also disallows the introduction of new warnings by removing the ALLOW_COMPILER_WARNINGS flag.
widget/gonk/GeckoTouchDispatcher.cpp
widget/gonk/GonkMemoryPressureMonitoring.cpp
widget/gonk/GonkPermission.cpp
widget/gonk/OrientationObserver.cpp
widget/gonk/libui/InputDispatcher.cpp
widget/gonk/libui/InputReader.cpp
widget/gonk/libui/Keyboard.cpp
widget/gonk/libui/SpriteController.cpp
widget/gonk/moz.build
widget/gonk/nsAppShell.cpp
--- a/widget/gonk/GeckoTouchDispatcher.cpp
+++ b/widget/gonk/GeckoTouchDispatcher.cpp
@@ -33,16 +33,17 @@
 #include "nsAppShell.h"
 #include "nsDebug.h"
 #include "nsThreadUtils.h"
 #include "nsWindow.h"
 #include <sys/types.h>
 #include <unistd.h>
 #include <utils/Timers.h>
 
+#undef LOG
 #define LOG(args...)                                            \
   __android_log_print(ANDROID_LOG_INFO, "Gonk" , ## args)
 
 // uncomment to print log resample data
 // #define LOG_RESAMPLE_DATA 1
 
 namespace mozilla {
 
--- a/widget/gonk/GonkMemoryPressureMonitoring.cpp
+++ b/widget/gonk/GonkMemoryPressureMonitoring.cpp
@@ -116,18 +116,16 @@ public:
   }
 
   NS_IMETHOD Run()
   {
     MOZ_ASSERT(!NS_IsMainThread());
 
 #ifdef MOZ_NUWA_PROCESS
     if (IsNuwaProcess()) {
-      NS_ASSERTION(NuwaMarkCurrentThread != nullptr,
-                   "NuwaMarkCurrentThread is undefined!");
       NuwaMarkCurrentThread(nullptr, nullptr);
     }
 #endif
 
     int lowMemFd = open("/sys/kernel/mm/lowmemkiller/notify_trigger_active",
                         O_RDONLY | O_CLOEXEC);
     NS_ENSURE_STATE(lowMemFd != -1);
     ScopedClose autoClose(lowMemFd);
--- a/widget/gonk/GonkPermission.cpp
+++ b/widget/gonk/GonkPermission.cpp
@@ -25,16 +25,17 @@
 #include "mozilla/dom/TabParent.h"
 #include "mozilla/SyncRunnable.h"
 #include "nsIAppsService.h"
 #include "mozIApplication.h"
 #include "nsThreadUtils.h"
 
 #undef LOG
 #include <android/log.h>
+#undef ALOGE
 #define ALOGE(args...)  __android_log_print(ANDROID_LOG_ERROR, "gonkperm" , ## args)
 
 using namespace android;
 using namespace mozilla;
 
 // Checking permissions needs to happen on the main thread, but the
 // binder callback is called on a special binder thread, so we use
 // this runnable for that.
--- a/widget/gonk/OrientationObserver.cpp
+++ b/widget/gonk/OrientationObserver.cpp
@@ -38,18 +38,18 @@ struct OrientationMapping {
 
 static OrientationMapping sOrientationMappings[] = {
   {nsIScreen::ROTATION_0_DEG,   eScreenOrientation_PortraitPrimary},
   {nsIScreen::ROTATION_180_DEG, eScreenOrientation_PortraitSecondary},
   {nsIScreen::ROTATION_90_DEG,  eScreenOrientation_LandscapePrimary},
   {nsIScreen::ROTATION_270_DEG, eScreenOrientation_LandscapeSecondary},
 };
 
-const static int sDefaultLandscape = 2;
-const static int sDefaultPortrait = 0;
+const static uint32_t sDefaultLandscape = 2;
+const static uint32_t sDefaultPortrait = 0;
 
 static uint32_t sOrientationOffset = 0;
 
 static already_AddRefed<nsIScreen>
 GetPrimaryScreen()
 {
   nsCOMPtr<nsIScreenManager> screenMgr =
     do_GetService("@mozilla.org/gfx/screenmanager;1");
@@ -103,17 +103,17 @@ DetectDefaultOrientation()
  * @param aOrientation DOM orientation e.g.
  *        dom::eScreenOrientation_PortraitPrimary.
  * @param aResult output nsIScreen rotation e.g. nsIScreen::ROTATION_0_DEG.
  * @return NS_OK on success. NS_ILLEGAL_VALUE on failure.
  */
 static nsresult
 ConvertToScreenRotation(ScreenOrientationInternal aOrientation, uint32_t *aResult)
 {
-  for (int i = 0; i < ArrayLength(sOrientationMappings); i++) {
+  for (uint32_t i = 0; i < ArrayLength(sOrientationMappings); i++) {
     if (aOrientation & sOrientationMappings[i].mDomOrientation) {
       // Shift the mappings in sOrientationMappings so devices with default
       // landscape orientation map landscape-primary to 0 degree and so forth.
       int adjusted = (i + sOrientationOffset) %
                      ArrayLength(sOrientationMappings);
       *aResult = sOrientationMappings[adjusted].mScreenRotation;
       return NS_OK;
     }
@@ -129,17 +129,17 @@ ConvertToScreenRotation(ScreenOrientatio
  * @param aRotation nsIScreen rotation e.g. nsIScreen::ROTATION_0_DEG.
  * @param aResult output DOM orientation e.g.
  *        dom::eScreenOrientation_PortraitPrimary.
  * @return NS_OK on success. NS_ILLEGAL_VALUE on failure.
  */
 nsresult
 ConvertToDomOrientation(uint32_t aRotation, ScreenOrientationInternal *aResult)
 {
-  for (int i = 0; i < ArrayLength(sOrientationMappings); i++) {
+  for (uint32_t i = 0; i < ArrayLength(sOrientationMappings); i++) {
     if (aRotation == sOrientationMappings[i].mScreenRotation) {
       // Shift the mappings in sOrientationMappings so devices with default
       // landscape orientation map 0 degree to landscape-primary and so forth.
       int adjusted = (i + sOrientationOffset) %
                      ArrayLength(sOrientationMappings);
       *aResult = sOrientationMappings[adjusted].mDomOrientation;
       return NS_OK;
     }
@@ -208,17 +208,17 @@ OrientationObserver::Notify(const hal::S
   }
 
   uint32_t currRotation;
   if(NS_FAILED(screen->GetRotation(&currRotation))) {
     return;
   }
 
   int rotation = mOrientation->OnSensorChanged(aSensorData, static_cast<int>(currRotation));
-  if (rotation < 0 || rotation == currRotation) {
+  if (rotation < 0 || uint32_t(rotation) == currRotation) {
     return;
   }
 
   ScreenOrientationInternal orientation;
   if (NS_FAILED(ConvertToDomOrientation(rotation, &orientation))) {
     return;
   }
 
--- a/widget/gonk/libui/InputDispatcher.cpp
+++ b/widget/gonk/libui/InputDispatcher.cpp
@@ -1107,18 +1107,16 @@ int32_t InputDispatcher::findTouchedWind
         const MotionEntry* entry, Vector<InputTarget>& inputTargets, nsecs_t* nextWakeupTime,
         bool* outConflictingPointerActions) {
     enum InjectionPermission {
         INJECTION_PERMISSION_UNKNOWN,
         INJECTION_PERMISSION_GRANTED,
         INJECTION_PERMISSION_DENIED
     };
 
-    nsecs_t startTime = now();
-
     // For security reasons, we defer updating the touch state until we are sure that
     // event injection will be allowed.
     //
     // FIXME In the original code, screenWasOff could never be set to true.
     //       The reason is that the POLICY_FLAG_WOKE_HERE
     //       and POLICY_FLAG_BRIGHT_HERE flags were set only when preprocessing raw
     //       EV_KEY, EV_REL and EV_ABS events.  As it happens, the touch event was
     //       actually enqueued using the policyFlags that appeared in the final EV_SYN
@@ -2236,30 +2234,28 @@ void InputDispatcher::synthesizeCancelat
         startDispatchCycleLocked(currentTime, connection);
     }
 }
 
 InputDispatcher::MotionEntry*
 InputDispatcher::splitMotionEvent(const MotionEntry* originalMotionEntry, BitSet32 pointerIds) {
     ALOG_ASSERT(pointerIds.value != 0);
 
-    uint32_t splitPointerIndexMap[MAX_POINTERS];
     PointerProperties splitPointerProperties[MAX_POINTERS];
     PointerCoords splitPointerCoords[MAX_POINTERS];
 
     uint32_t originalPointerCount = originalMotionEntry->pointerCount;
     uint32_t splitPointerCount = 0;
 
     for (uint32_t originalPointerIndex = 0; originalPointerIndex < originalPointerCount;
             originalPointerIndex++) {
         const PointerProperties& pointerProperties =
                 originalMotionEntry->pointerProperties[originalPointerIndex];
         uint32_t pointerId = uint32_t(pointerProperties.id);
         if (pointerIds.hasBit(pointerId)) {
-            splitPointerIndexMap[splitPointerCount] = originalPointerIndex;
             splitPointerProperties[splitPointerCount].copyFrom(pointerProperties);
             splitPointerCoords[splitPointerCount].copyFrom(
                     originalMotionEntry->pointerCoords[originalPointerIndex]);
             splitPointerCount += 1;
         }
     }
 
     if (splitPointerCount != pointerIds.count()) {
--- a/widget/gonk/libui/InputReader.cpp
+++ b/widget/gonk/libui/InputReader.cpp
@@ -1587,21 +1587,25 @@ void MultiTouchMotionAccumulator::clearS
             mSlots[i].clear();
         }
     }
     mCurrentSlot = initialSlot;
 }
 
 void MultiTouchMotionAccumulator::process(const RawEvent* rawEvent) {
     if (rawEvent->type == EV_ABS) {
+#if DEBUG_POINTERS
         bool newSlot = false;
+#endif
         if (mUsingSlotsProtocol) {
             if (rawEvent->code == ABS_MT_SLOT) {
                 mCurrentSlot = rawEvent->value;
+#if DEBUG_POINTERS
                 newSlot = true;
+#endif
             }
         } else if (mCurrentSlot < 0) {
             mCurrentSlot = 0;
         }
 
         if (mCurrentSlot < 0 || size_t(mCurrentSlot) >= mSlotCount) {
 #if DEBUG_POINTERS
             if (newSlot) {
@@ -4645,28 +4649,25 @@ bool TouchInputMapper::preparePointerGes
                 mCurrentFingerIdBits, positions);
     }
 
     // Pick a new active touch id if needed.
     // Choose an arbitrary pointer that just went down, if there is one.
     // Otherwise choose an arbitrary remaining pointer.
     // This guarantees we always have an active touch id when there is at least one pointer.
     // We keep the same active touch id for as long as possible.
-    bool activeTouchChanged = false;
     int32_t lastActiveTouchId = mPointerGesture.activeTouchId;
     int32_t activeTouchId = lastActiveTouchId;
     if (activeTouchId < 0) {
         if (!mCurrentFingerIdBits.isEmpty()) {
-            activeTouchChanged = true;
             activeTouchId = mPointerGesture.activeTouchId =
                     mCurrentFingerIdBits.firstMarkedBit();
             mPointerGesture.firstTouchTime = when;
         }
     } else if (!mCurrentFingerIdBits.hasBit(activeTouchId)) {
-        activeTouchChanged = true;
         if (!mCurrentFingerIdBits.isEmpty()) {
             activeTouchId = mPointerGesture.activeTouchId =
                     mCurrentFingerIdBits.firstMarkedBit();
         } else {
             activeTouchId = mPointerGesture.activeTouchId = -1;
         }
     }
 
@@ -4752,17 +4753,16 @@ bool TouchInputMapper::preparePointerGes
                     if (speed > bestSpeed) {
                         bestId = id;
                         bestSpeed = speed;
                     }
                 }
             }
             if (bestId >= 0 && bestId != activeTouchId) {
                 mPointerGesture.activeTouchId = activeTouchId = bestId;
-                activeTouchChanged = true;
 #if DEBUG_GESTURES
                 ALOGD("Gestures: BUTTON_CLICK_OR_DRAG switched pointers, "
                         "bestId=%d, bestSpeed=%0.3f", bestId, bestSpeed);
 #endif
             }
         }
 
         if (activeTouchId >= 0 && mLastFingerIdBits.hasBit(activeTouchId)) {
--- a/widget/gonk/libui/Keyboard.cpp
+++ b/widget/gonk/libui/Keyboard.cpp
@@ -238,17 +238,16 @@ static int32_t toggleLockedMetaState(int
     if (down) {
         return oldMetaState;
     } else {
         return oldMetaState ^ mask;
     }
 }
 
 int32_t updateMetaState(int32_t keyCode, bool down, int32_t oldMetaState) {
-    int32_t mask;
     switch (keyCode) {
     case AKEYCODE_ALT_LEFT:
         return setEphemeralMetaState(AMETA_ALT_LEFT_ON, down, oldMetaState);
     case AKEYCODE_ALT_RIGHT:
         return setEphemeralMetaState(AMETA_ALT_RIGHT_ON, down, oldMetaState);
     case AKEYCODE_SHIFT_LEFT:
         return setEphemeralMetaState(AMETA_SHIFT_LEFT_ON, down, oldMetaState);
     case AKEYCODE_SHIFT_RIGHT:
--- a/widget/gonk/libui/SpriteController.cpp
+++ b/widget/gonk/libui/SpriteController.cpp
@@ -136,40 +136,38 @@ void SpriteController::doUpdateSprites()
             updates.push(SpriteUpdate(sprite, sprite->getStateLocked()));
             sprite->resetDirtyLocked();
         }
         mLocked.invalidatedSprites.clear();
     } // release lock
 
     // Create missing surfaces.
     bool surfaceChanged = false;
+#ifdef HAVE_ANDROID_OS
     for (size_t i = 0; i < numSprites; i++) {
         SpriteUpdate& update = updates.editItemAt(i);
-
-#ifdef HAVE_ANDROID_OS
         if (update.state.surfaceControl == NULL && update.state.wantSurfaceVisible()) {
             update.state.surfaceWidth = update.state.icon.bitmap.width();
             update.state.surfaceHeight = update.state.icon.bitmap.height();
             update.state.surfaceDrawn = false;
             update.state.surfaceVisible = false;
             update.state.surfaceControl = obtainSurface(
                     update.state.surfaceWidth, update.state.surfaceHeight);
             if (update.state.surfaceControl != NULL) {
                 update.surfaceChanged = surfaceChanged = true;
             }
         }
+    }
 #endif
-    }
 
     // Resize sprites if needed, inside a global transaction.
+#ifdef HAVE_ANDROID_OS
     bool haveGlobalTransaction = false;
     for (size_t i = 0; i < numSprites; i++) {
         SpriteUpdate& update = updates.editItemAt(i);
-
-#ifdef HAVE_ANDROID_OS
         if (update.state.surfaceControl != NULL && update.state.wantSurfaceVisible()) {
             int32_t desiredWidth = update.state.icon.bitmap.width();
             int32_t desiredHeight = update.state.icon.bitmap.height();
             if (update.state.surfaceWidth < desiredWidth
                     || update.state.surfaceHeight < desiredHeight) {
                 if (!haveGlobalTransaction) {
                     SurfaceComposerClient::openGlobalTransaction();
                     haveGlobalTransaction = true;
@@ -192,18 +190,18 @@ void SpriteController::doUpdateSprites()
                             ALOGE("Error %d hiding sprite surface after resize.", status);
                         } else {
                             update.state.surfaceVisible = false;
                         }
                     }
                 }
             }
         }
+    }
 #endif
-    }
 #ifdef HAVE_ANDROID_OS
     if (haveGlobalTransaction) {
         SurfaceComposerClient::closeGlobalTransaction();
     }
 #endif
 
     // Redraw sprites if needed.
     for (size_t i = 0; i < numSprites; i++) {
@@ -253,26 +251,25 @@ void SpriteController::doUpdateSprites()
                     update.state.surfaceDrawn = true;
                     update.surfaceChanged = surfaceChanged = true;
                 }
             }
         }
 #endif
     }
 
+#ifdef HAVE_ANDROID_OS
     // Set sprite surface properties and make them visible.
     bool haveTransaction = false;
     for (size_t i = 0; i < numSprites; i++) {
         SpriteUpdate& update = updates.editItemAt(i);
-
         bool wantSurfaceVisibleAndDrawn = update.state.wantSurfaceVisible()
                 && update.state.surfaceDrawn;
         bool becomingVisible = wantSurfaceVisibleAndDrawn && !update.state.surfaceVisible;
         bool becomingHidden = !wantSurfaceVisibleAndDrawn && update.state.surfaceVisible;
-#ifdef HAVE_ANDROID_OS
         if (update.state.surfaceControl != NULL && (becomingVisible || becomingHidden
                 || (wantSurfaceVisibleAndDrawn && (update.state.dirty & (DIRTY_ALPHA
                         | DIRTY_POSITION | DIRTY_TRANSFORMATION_MATRIX | DIRTY_LAYER
                         | DIRTY_VISIBILITY | DIRTY_HOTSPOT))))) {
             status_t status;
             if (!haveTransaction) {
                 SurfaceComposerClient::openGlobalTransaction();
                 haveTransaction = true;
@@ -332,41 +329,41 @@ void SpriteController::doUpdateSprites()
                 if (status) {
                     ALOGE("Error %d hiding sprite surface.", status);
                 } else {
                     update.state.surfaceVisible = false;
                     update.surfaceChanged = surfaceChanged = true;
                 }
             }
         }
+    }
 #endif
-    }
 
 #ifdef HAVE_ANDROID_OS
     if (haveTransaction) {
         SurfaceComposerClient::closeGlobalTransaction();
     }
 #endif
 
+#ifdef HAVE_ANDROID_OS
     // If any surfaces were changed, write back the new surface properties to the sprites.
     if (surfaceChanged) { // acquire lock
         AutoMutex _l(mLock);
 
         for (size_t i = 0; i < numSprites; i++) {
             const SpriteUpdate& update = updates.itemAt(i);
 
-#ifdef HAVE_ANDROID_OS
             if (update.surfaceChanged) {
                 update.sprite->setSurfaceLocked(update.state.surfaceControl,
                         update.state.surfaceWidth, update.state.surfaceHeight,
                         update.state.surfaceDrawn, update.state.surfaceVisible);
             }
-#endif
         }
     } // release lock
+#endif
 
     // Clear the sprite update vector outside the lock.  It is very important that
     // we do not clear sprite references inside the lock since we could be releasing
     // the last remaining reference to the sprite here which would result in the
     // sprite being deleted and the lock being reacquired by the sprite destructor
     // while already held.
     updates.clear();
 }
--- a/widget/gonk/moz.build
+++ b/widget/gonk/moz.build
@@ -72,19 +72,16 @@ SOURCES += [
     'nsWindow.cpp',
     'OrientationObserver.cpp',
     'ProcessOrientation.cpp',
     'WidgetTraceEvent.cpp'
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
-# XXX: We should fix these warnings.
-ALLOW_COMPILER_WARNINGS = True
-
 FINAL_LIBRARY = 'xul'
 
 LOCAL_INCLUDES += [
     '/dom/system/android',
     '/gfx/skia/skia/include/config',
     '/gfx/skia/skia/include/core',
     '/image',
     '/widget',
--- a/widget/gonk/nsAppShell.cpp
+++ b/widget/gonk/nsAppShell.cpp
@@ -70,16 +70,17 @@
 #include "mozilla/Preferences.h"
 #include "GeckoProfiler.h"
 
 // Defines kKeyMapping and GetKeyNameIndex()
 #include "GonkKeyMapping.h"
 #include "mozilla/layers/CompositorParent.h"
 #include "GeckoTouchDispatcher.h"
 
+#undef LOG
 #define LOG(args...)                                            \
     __android_log_print(ANDROID_LOG_INFO, "Gonk" , ## args)
 #ifdef VERBOSE_LOG_ENABLED
 # define VERBOSE_LOG(args...)                           \
     __android_log_print(ANDROID_LOG_INFO, "Gonk" , ## args)
 #else
 # define VERBOSE_LOG(args...)                   \
     (void)0