Monitor Profile destruction from ChromeExtensionMessageFilter.

Monitor Profile destruction from ChromeExtensionMessageFilter.

BUG=391861
R=asargent@chromium.org
TBR=jochen@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282109

[asargent_no_longer_on_chrome, 2014-07-08 23:55:55]

lgtm

[not at google - send to devlin, 2014-07-09 00:10:41]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-07-09 00:15:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kalman@chromium.org/377833009/1

[commit-bot: I haz the power, 2014-07-09 08:17:25]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  mac_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_triggered...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[commit-bot: I haz the power, 2014-07-09 08:20:15]

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

[commit-bot: I haz the power, 2014-07-09 08:20:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[not at google - send to devlin, 2014-07-09 15:50:39]

TBR-ing jochen.

[not at google - send to devlin, 2014-07-09 15:50:42]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-07-09 15:52:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kalman@chromium.org/377833009/1

[not at google - send to devlin, 2014-07-09 16:23:21]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-07-09 16:23:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kalman@chromium.org/377833009/20001

[not at google - send to devlin, 2014-07-09 17:09:42]

https://codereview.chromium.org/377833009/diff/40001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_extension_message_filter.cc (right):

https://codereview.chromium.org/377833009/diff/40001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_extension_message_filter.cc:126: if
(BrowserThread::CurrentlyOn(BrowserThread::UI)) {
Antony - I needed to add this, it was crashing.

[not at google - send to devlin, 2014-07-09 17:09:44]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-07-09 17:11:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kalman@chromium.org/377833009/40001

[asargent_no_longer_on_chrome, 2014-07-09 17:58:39]

https://codereview.chromium.org/377833009/diff/40001/chrome/browser/renderer_...
File chrome/browser/renderer_host/chrome_extension_message_filter.cc (right):

https://codereview.chromium.org/377833009/diff/40001/chrome/browser/renderer_...
chrome/browser/renderer_host/chrome_extension_message_filter.cc:126: if
(BrowserThread::CurrentlyOn(BrowserThread::UI)) {
On 2014/07/09 17:09:41, kalman wrote:
> Antony - I needed to add this, it was crashing.

Ok, makes sense.

[commit-bot: I haz the power, 2014-07-09 20:26:49]

Change committed as 282109

[not at google - send to devlin, 2014-08-08 17:50:07]

coming back to this to merge, I'm a little surprised there wasn't a problem with
double-deletion, since profile destruction is calling "delete this" but it's
refcounted, so, surely elsewhere the refcounting is being deleted?

well... nothing has complained so far. huh.

[asargent_no_longer_on_chrome, 2014-08-08 18:01:17]

Huh - I guess this works because of the custom BrowserMessageFilterTraits
that BrowserMessageFilter uses with RefCountedThreadSafe, which instead of
getting the default behavior of  DefaultRefCountedThreadSafeTraits that
just calls DeleteInternal (which does "delete this"), it instead calls back
our OnDestruct where we do the same thing. (I guess we have this because
some BrowserMessageFilter's need to do more complicated work before
destructing themselves?)



On Fri, Aug 8, 2014 at 10:50 AM, <kalman@chromium.org> wrote:

> coming back to this to merge, I'm a little surprised there wasn't a
> problem with
> double-deletion, since profile destruction is calling "delete this" but
> it's
> refcounted, so, surely elsewhere the refcounting is being deleted?
>
> well... nothing has complained so far. huh.
>
> https://codereview.chromium.org/377833009/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[not at google - send to devlin, 2014-08-08 19:43:35]

On 2014/08/08 18:01:17, Antony Sargent wrote:
> Huh - I guess this works because of the custom BrowserMessageFilterTraits
> that BrowserMessageFilter uses with RefCountedThreadSafe, which instead of
> getting the default behavior of  DefaultRefCountedThreadSafeTraits that
> just calls DeleteInternal (which does "delete this"), it instead calls back
> our OnDestruct where we do the same thing. (I guess we have this because
> some BrowserMessageFilter's need to do more complicated work before
> destructing themselves?)

oh how odd! but that explains it. ok I feel better about this now, thanks.

> 
> 
> 
> On Fri, Aug 8, 2014 at 10:50 AM, <mailto:kalman@chromium.org> wrote:
> 
> > coming back to this to merge, I'm a little surprised there wasn't a
> > problem with
> > double-deletion, since profile destruction is calling "delete this" but
> > it's
> > refcounted, so, surely elsewhere the refcounting is being deleted?
> >
> > well... nothing has complained so far. huh.
> >
> > https://codereview.chromium.org/377833009/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.