Fix Android strict-mode violation in GeoLocation info bar.

chrome/browser/geolocation/chrome_geolocation_permission_context_android.cc

 // Copyright (c) 2012 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 "chrome/browser/geolocation/chrome_geolocation_permission_context_android.h"
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/android/google_location_settings_helper.h"
+#include "chrome/browser/content_settings/permission_request_id.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 
 ChromeGeolocationPermissionContextAndroid::
     ChromeGeolocationPermissionContextAndroid(Profile* profile)
     : ChromeGeolocationPermissionContext(profile),
       google_location_settings_helper_(
           GoogleLocationSettingsHelper::Create()) {
 }
 
 ChromeGeolocationPermissionContextAndroid::
     ~ChromeGeolocationPermissionContextAndroid() {
 }
 
+void ChromeGeolocationPermissionContextAndroid::CallSuperDecidePermission(

[bulach, 2013/09/12 09:18:29]

thanks for the explanation!
how about "ProceedDecidePermissionFlow"? more below..

[acleung1, 2013/09/17 00:08:45]

Done.

+    const PermissionRequestID& id,
+    const GURL& requesting_frame,
+    const GURL& embedder,
+    base::Callback<void(bool)> callback) {
+  // Super class implementation expects everything in UI thread instead.
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  ChromeGeolocationPermissionContext::DecidePermission(
+      id, requesting_frame, embedder, callback);
+}
+
+void ChromeGeolocationPermissionContextAndroid::CheckMasterLocation(
+    const PermissionRequestID& id,
+    const GURL& requesting_frame,
+    const GURL& embedder,
+    base::Callback<void(bool)> callback) {
+  // Check to see if the feature in its entirety has been disabled.
+  // This must happen before other services (e.g. tabs, extensions)
+  // get an opportunity to allow the geolocation request.
+  bool enabled =
+      google_location_settings_helper_->IsMasterLocationSettingEnabled();
+
+  // Regardless of what the master setting is, we still need to
+  // go back to the UI thread for the rest of the bookkeeping.

[bulach, 2013/09/12 09:18:29]

Perhaps a more detailed explanation would help clarify:

// The flow for geolocation permission on android is:
// - ChromeGeolocationPermissionContextAndroid::DecidePermission
// intercepts the flow in the UI thread, and posts task
// to the blocking pool to CheckMasterLocation (in order to
// avoid strict-mode violation).
// - At this point the master location permission is either:
// -- enabled, in which we case it proceeds the normal flow
// via ChromeGeolocationPermissionContext (which may create infobars, etc.).
// -- disabled, in which case the permission is already decided.
//
// In either case, ChromeGeolocationPermissionContext expects these
// in the UI thread.

[acleung1, 2013/09/17 00:08:45]

Done.

+  base::Closure ui_closure;
+  if (enabled) {
+    ui_closure = base::Bind(
+        &ChromeGeolocationPermissionContextAndroid::CallSuperDecidePermission,

[bulach, 2013/09/12 09:18:29]

so if you agree with the comment above, I think "ProceedDecidePermissionFlow"
would be clearer..

[acleung1, 2013/09/17 00:08:45]

Done.

+        base::Unretained(this), id, requesting_frame, embedder, callback);
+  } else {
+    ui_closure = base::Bind(
+        &ChromeGeolocationPermissionContextAndroid::PermissionDecided,
+        base::Unretained(this), id,
+        requesting_frame, embedder, callback, false);
+  }
+
+  // This method is executed from the BlockingPool, post the result
+  // back to the UI thread.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,ui_closure);

[bulach, 2013/09/12 09:18:29]

nit: space after comma, s/,ui_clo/, ui_clo/

[acleung1, 2013/09/17 00:08:45]

Done.

+}
+
 void ChromeGeolocationPermissionContextAndroid::DecidePermission(
     const PermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     base::Callback<void(bool)> callback) {
+
+  // Called on the UI thread. However, do the work on a separate thread
+  // to avoid strict mode violation.
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  // Check to see if the feature in its entirety has been disabled.
-  // This must happen before other services (e.g. tabs, extensions)
-  // get an opportunity to allow the geolocation request.
-  if (!google_location_settings_helper_->IsMasterLocationSettingEnabled()) {
-    PermissionDecided(id, requesting_frame, embedder, callback, false);
-    return;
-  }
-
-  ChromeGeolocationPermissionContext::DecidePermission(id, requesting_frame,
-                                                       embedder, callback);
+  content::BrowserThread::PostBlockingPoolTask(FROM_HERE,
+      base::Bind(
+          &ChromeGeolocationPermissionContextAndroid::CheckMasterLocation,
+  base::Unretained(this), id, requesting_frame, embedder, callback));

[bulach, 2013/09/12 09:18:29]

nit: indent

[acleung1, 2013/09/17 00:08:45]

Done.

 }
 
 void ChromeGeolocationPermissionContextAndroid::PermissionDecided(
     const PermissionRequestID& id,
     const GURL& requesting_frame,
     const GURL& embedder,
     base::Callback<void(bool)> callback,
     bool allowed) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   // If Google Apps Location setting is turned off then we ignore
   // the 'allow' website setting for this site and instead show
   // the infobar to go back to the 'settings' to turn it back on.
   if (allowed &&
       !google_location_settings_helper_->IsGoogleAppsLocationSettingEnabled()) {
     QueueController()->CreateInfoBarRequest(id, requesting_frame, embedder,
                                             callback);

[bulach, 2013/10/16 10:47:11]

here, I think this could pass the boolean and string obtained above down to the
infobar... that way, the infobar is entirely in the UI thread, and all the
orchestration happens here.

[acleung1, 2013/12/12 23:38:30]

I don't think that's going to solve the issue we have. There are two strict mode
violations. The first one  involves the "Allow Label" which the info bar needs
to know when it is being constructed.

[acleung1, 2013/12/12 23:38:30]

I dont' think this will solve our problem because there are two strict mode
violations this is trying to fix. One of them being the Allow Label that is
needed when the infobar is being constructed. I don't really see a good way to
delay it to here.

     return;
   }
 
   ChromeGeolocationPermissionContext::PermissionDecided(
       id, requesting_frame, embedder, callback, allowed);
 }