Remove never reached area in GeoNotifier

Source/modules/geolocation/GeoNotifier.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/geolocation/GeoNotifier.h"
 
 #include "modules/geolocation/Geolocation.h"
 #include "modules/geolocation/PositionCallback.h"
 #include "modules/geolocation/PositionError.h"
@@ skipping 38 matching lines @@
 }
 
 void GeoNotifier::setUseCachedPosition()
 {
     m_useCachedPosition = true;
     m_timer.startOneShot(0, FROM_HERE);
 }
 
 void GeoNotifier::runSuccessCallback(Geoposition* position)
 {
-    // If we are here and the Geolocation permission is not approved, something has
-    // gone horribly wrong.
-    if (!m_geolocation->isAllowed())
-        CRASH();

[Michael van Ouwerkerk, 2014/07/17 10:15:00]

I'd have prefered to just replace this with ASSERT(m_geolocation->isAllowed());

While it is true that currently this is dead code, there are multiple paths to
this method. Also, a lot of code in Geolocation is asynchronous. This means that
it seems possible for regressions to occur, and an ASSERT is the easiest way to
read the code and verify the behavior.

Are you removing this to get rid of the m_geolocation member? It seems an
unfortunate side effect to lose code readability and robustness.

[kihong, 2014/07/17 11:47:55]

There are only two way to get here.(from makeCachedPositionCallbacks() or
endPosition())
And allowance checking have been done right before calling runSuccessCallback()
synchronously.
So, I don't think there is possible to get here without allowance.
But if you think we need this, I have no problem with reverting or changing to
ASSERT.

[Michael van Ouwerkerk, 2014/07/17 12:15:34]

Your change is correct. If the purpose is the break the cycle, then this seems
like a reasonable change. I was just expressing regret at losing some
readability of the code.

[kihong, 2014/07/17 12:21:10]

I got it and thanks for your kind reviewing. :)

-
     m_successCallback->handleEvent(position);
 }
 
 void GeoNotifier::runErrorCallback(PositionError* error)
 {
     if (m_errorCallback)
         m_errorCallback->handleEvent(error);
 }
 
 void GeoNotifier::startTimer()
@@ skipping 26 matching lines @@
         m_geolocation->requestUsesCachedPosition(this);
         return;
     }
 
     if (m_errorCallback)
         m_errorCallback->handleEvent(PositionError::create(PositionError::TIMEOUT, "Timeout expired"));
     m_geolocation->requestTimedOut(this);
 }
 
 } // namespace WebCore