extensions: Destroy ExtensionMessageFilter on IO thread.

extensions: Destroy ExtensionMessageFilter on IO thread.

Avoid a crash in tests caused by
WeakPtr<ExtensionMessageFilter>s being used on the IO thread
while the filter is destroyed on the UI thread.

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

[Daniel Erat, 2014-06-02 19:37:08]

yoz@, mind approving for extensions/?

[James Cook, 2014-06-02 19:50:43]

LGTM with comments

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
File extensions/browser/extension_message_filter.cc (right):

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
extensions/browser/extension_message_filter.cc:34:
ExtensionMessageFilter::~ExtensionMessageFilter() {}
DCHECK_CURRENTLY_ON(BrowserThread::IO)

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
extensions/browser/extension_message_filter.cc:57:
BrowserThread::DeleteOnIOThread::Destruct(this);
I would document either here or in the header why it has to be destroyed on the
IO thread.

Optional alternative: If you think we're slathering this with too many comments
to cover up a code smell, consider splitting into two message filters, one that
just handles IO thread messages.

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
File extensions/browser/extension_message_filter.h (right):

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
extensions/browser/extension_message_filter.h:89:
base::WeakPtrFactory<ExtensionMessageFilter> weak_ptr_factory_;
Document that these weak pointers are bound to the IO thread.

[Yoyo Zhou, 2014-06-02 19:54:26]

LGTM

[Wez, 2014-06-02 20:01:31]

LGTM!

[Daniel Erat, 2014-06-02 20:07:58]

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
File extensions/browser/extension_message_filter.cc (right):

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
extensions/browser/extension_message_filter.cc:34:
ExtensionMessageFilter::~ExtensionMessageFilter() {}
On 2014/06/02 19:50:43, James Cook wrote:
> DCHECK_CURRENTLY_ON(BrowserThread::IO)

Done.

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
extensions/browser/extension_message_filter.cc:57:
BrowserThread::DeleteOnIOThread::Destruct(this);
On 2014/06/02 19:50:43, James Cook wrote:
> I would document either here or in the header why it has to be destroyed on
the
> IO thread.
> 
> Optional alternative: If you think we're slathering this with too many
comments
> to cover up a code smell, consider splitting into two message filters, one
that
> just handles IO thread messages.

Done.

(i went with the easy/lazy approach of adding comments)

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
File extensions/browser/extension_message_filter.h (right):

https://codereview.chromium.org/309123002/diff/1/extensions/browser/extension...
extensions/browser/extension_message_filter.h:89:
base::WeakPtrFactory<ExtensionMessageFilter> weak_ptr_factory_;
On 2014/06/02 19:50:43, James Cook wrote:
> Document that these weak pointers are bound to the IO thread.

Done.

[Daniel Erat, 2014-06-02 20:08:03]

The CQ bit was checked by derat@chromium.org

[commit-bot: I haz the power, 2014-06-02 20:09:06]

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

[commit-bot: I haz the power, 2014-06-03 00:03:58]

Change committed as 274382