Reset geolocation permission embargo for an origin when it becomes DSE

Reset geolocation permission embargo for an origin when it becomes DSE

If an origin becomes the default search engine (DSE) it should have any
embargo status on it reset.

BUG=730377
Review-Url: https://codereview.chromium.org/2941753002
Cr-Commit-Position: refs/heads/master@{#479606}
Committed: https://chromium.googlesource.com/chromium/src/+/19a5b3587d246544ad1927e6c891e00df2061c49

[benwells, 2017-06-14 13:00:30]

The CQ bit was checked by benwells@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 13:00:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-14 14:56:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-14 14:56:44]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[benwells, 2017-06-15 00:27:59]

benwells@chromium.org changed reviewers:
+ dominickn@chromium.org

[dominickn, 2017-06-15 00:34:54]

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
File
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc
(right):

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:393:
EXPECT_EQ(auto_blocker
It might be more robust to check the embargo state via
PermissionManager::GetPermissionStatus since that's the primary external
interface.

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:397:
PermissionStatusSource::MULTIPLE_DISMISSALS);
You should explicitly check the PermissionResult::content_setting is
CONTENT_SETTING_BLOCK rather than the source (which is really just for UI
purposes).

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:404:
.source,
You should explicitly check the PermissionResult::content_setting is
CONTENT_SETTING_ASK.

[benwells, 2017-06-15 03:59:24]

The CQ bit was checked by benwells@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-15 03:59:35]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[benwells, 2017-06-15 04:00:40]

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
File
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc
(right):

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:393:
EXPECT_EQ(auto_blocker
On 2017/06/15 00:34:53, dominickn wrote:
> It might be more robust to check the embargo state via
> PermissionManager::GetPermissionStatus since that's the primary external
> interface.

As discussed it makes the tests not work, as the search geo stuff hooks in too
soon for embargo to take effect.

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:397:
PermissionStatusSource::MULTIPLE_DISMISSALS);
On 2017/06/15 00:34:53, dominickn wrote:
> You should explicitly check the PermissionResult::content_setting is
> CONTENT_SETTING_BLOCK rather than the source (which is really just for UI
> purposes).

Checked as well.

https://codereview.chromium.org/2941753002/diff/1/chrome/browser/android/sear...
chrome/browser/android/search_geolocation/search_geolocation_service_unittest.cc:404:
.source,
On 2017/06/15 00:34:53, dominickn wrote:
> You should explicitly check the PermissionResult::content_setting is
> CONTENT_SETTING_ASK.

Checked as well.

[dominickn, 2017-06-15 04:08:51]

lgtm

[commit-bot: I haz the power, 2017-06-15 04:41:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-15 04:41:18]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-06-15 04:42:51]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2017-06-15 04:43:12]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-15 04:47:58]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497501771605100,
"parent_rev": "5825e6768fa612c2185540a66c02250fa031c28c", "commit_rev":
"19a5b3587d246544ad1927e6c891e00df2061c49"}

[commit-bot: I haz the power, 2017-06-15 04:48:07]

Description was changed from

==========
Reset geolocation permission embargo for an origin when it becomes DSE

If an origin becomes the default search engine (DSE) it should have any
embargo status on it reset.

BUG=730377
==========

to

==========
Reset geolocation permission embargo for an origin when it becomes DSE

If an origin becomes the default search engine (DSE) it should have any
embargo status on it reset.

BUG=730377

Review-Url: https://codereview.chromium.org/2941753002
Cr-Commit-Position: refs/heads/master@{#479606}
Committed:
https://chromium.googlesource.com/chromium/src/+/19a5b3587d246544ad1927e6c891...
==========

[commit-bot: I haz the power, 2017-06-15 04:48:10]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/19a5b3587d246544ad1927e6c891...