Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(43)

Issue 11590002: Destroy GeolocationInfobarQueueController on UI thread. (Closed)

Created:
8 years ago by John Knottenbelt
Modified:
7 years, 11 months ago
Reviewers:
bulach
CC:
chromium-reviews
Visibility:
Public.

Description

Destroy GeolocationInfobarQueueController on UI thread. ChromeGeolocationPermissionContext is RefcountedThreadsafe and may lose it's last reference from either the UI thread or the IO thread. GeolocationInfobarQueueController must be destroyed on the UI thread, however. Depends on https://codereview.chromium.org/11587003/ TEST= BUG=127751 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176112

Patch Set 1 #

Patch Set 2 : Rebase. #

Total comments: 2

Patch Set 3 : Fix nit. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -4 lines) Patch
M chrome/browser/geolocation/chrome_geolocation_permission_context.h View 1 2 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/geolocation/chrome_geolocation_permission_context.cc View 6 chunks +18 lines, -2 lines 0 comments Download
M chrome/browser/geolocation/chrome_geolocation_permission_context_factory.cc View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
John Knottenbelt
7 years, 11 months ago (2013-01-08 16:07:29 UTC) #1
bulach
lgtm, thanks! tiny nit: https://codereview.chromium.org/11590002/diff/2001/chrome/browser/geolocation/chrome_geolocation_permission_context.h File chrome/browser/geolocation/chrome_geolocation_permission_context.h (right): https://codereview.chromium.org/11590002/diff/2001/chrome/browser/geolocation/chrome_geolocation_permission_context.h#newcode91 chrome/browser/geolocation/chrome_geolocation_permission_context.h:91: geolocation_infobar_queue_controller_; nit: I think this ...
7 years, 11 months ago (2013-01-10 10:38:44 UTC) #2
John Knottenbelt
https://codereview.chromium.org/11590002/diff/2001/chrome/browser/geolocation/chrome_geolocation_permission_context.h File chrome/browser/geolocation/chrome_geolocation_permission_context.h (right): https://codereview.chromium.org/11590002/diff/2001/chrome/browser/geolocation/chrome_geolocation_permission_context.h#newcode91 chrome/browser/geolocation/chrome_geolocation_permission_context.h:91: geolocation_infobar_queue_controller_; On 2013/01/10 10:38:44, bulach wrote: > nit: I ...
7 years, 11 months ago (2013-01-10 15:16:26 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jknotten@chromium.org/11590002/8002
7 years, 11 months ago (2013-01-10 15:16:40 UTC) #4
commit-bot: I haz the power
Retried try job too often on ios_dbg_simulator for step(s) sync_unit_tests, unit_tests
7 years, 11 months ago (2013-01-10 15:48:50 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jknotten@chromium.org/11590002/8002
7 years, 11 months ago (2013-01-10 16:10:37 UTC) #6
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 11 months ago (2013-01-10 16:16:21 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jknotten@chromium.org/11590002/8002
7 years, 11 months ago (2013-01-10 16:44:44 UTC) #8
commit-bot: I haz the power
7 years, 11 months ago (2013-01-10 18:33:26 UTC) #9
Message was sent while issue was closed.
Change committed as 176112

Powered by Google App Engine
This is Rietveld 408576698