Reset the clipboard writer before destroying the clipboard message filter.

Reset the clipboard writer before destroying the clipboard message filter.

We cannot commit them at this point because we are running on the IO thread.

BUG=465586
Committed: https://crrev.com/3dae1709c63b598c867e0108eb8a4e0535b3c1ac
Cr-Commit-Position: refs/heads/master@{#319838}

[Martin Barbella, 2015-03-10 01:21:48]

mbarbella@chromium.org changed reviewers:
+ creis@chromium.org

[Martin Barbella, 2015-03-10 01:21:49]

Charlie, could you PTAL or suggest another reviewer?

This is currently a blocker for IPC fuzzing since it's coming up every few test
cases, but usually just as a SIGABRT.

[Charlie Reis, 2015-03-10 03:10:39]

creis@chromium.org changed reviewers:
+ dcheng@chromium.org

[Charlie Reis, 2015-03-10 03:10:39]

@dcheng is your man for clipboard stuff.

[dcheng, 2015-03-10 03:17:43]

LGTM but I'm not an owner.

[Charlie Reis, 2015-03-10 03:28:47]

Thanks.  I'm happy to look for owners approval.

https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/clipboard_message_filter.cc (right):

https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/clipboard_message_filter.cc:94:
clipboard_writer_->Reset();
Hmm, how is this different than before?  clipboard_writer_ is a scoped_ptr and
the last member of the class, so it would be the first to be deleted.  What am I
missing?

[dcheng, 2015-03-10 03:36:02]

https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/clipboard_message_filter.cc (right):

https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/clipboard_message_filter.cc:94:
clipboard_writer_->Reset();
On 2015/03/10 03:28:47, Charlie Reis wrote:
> Hmm, how is this different than before?  clipboard_writer_ is a scoped_ptr and
> the last member of the class, so it would be the first to be deleted.  What am
I
> missing?

ScopedClipboardWriter flushes internal data to the clipboard when it is
destroyed. It also has a Reset() member function, which clears that internal
data (which turns ~ScopedClipboardWriter into a no-op).

We are calling SCW::Reset here, not scoped_ptr::reset (the naming collision is a
bit unfortunate).

[Charlie Reis, 2015-03-10 04:15:40]

On 2015/03/10 03:36:02, dcheng wrote:
>
https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
> File content/browser/renderer_host/clipboard_message_filter.cc (right):
> 
>
https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
> content/browser/renderer_host/clipboard_message_filter.cc:94:
> clipboard_writer_->Reset();
> On 2015/03/10 03:28:47, Charlie Reis wrote:
> > Hmm, how is this different than before?  clipboard_writer_ is a scoped_ptr
and
> > the last member of the class, so it would be the first to be deleted.  What
am
> I
> > missing?
> 
> ScopedClipboardWriter flushes internal data to the clipboard when it is
> destroyed. It also has a Reset() member function, which clears that internal
> data (which turns ~ScopedClipboardWriter into a no-op).
> 
> We are calling SCW::Reset here, not scoped_ptr::reset (the naming collision is
a
> bit unfortunate).

Ah!  Reset vs reset is the bit I was missing.  :)

content/ LGTM.

[Martin Barbella, 2015-03-10 04:16:27]

On 2015/03/10 04:15:40, Charlie Reis wrote:
> On 2015/03/10 03:36:02, dcheng wrote:
> >
>
https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
> > File content/browser/renderer_host/clipboard_message_filter.cc (right):
> > 
> >
>
https://codereview.chromium.org/987583005/diff/1/content/browser/renderer_hos...
> > content/browser/renderer_host/clipboard_message_filter.cc:94:
> > clipboard_writer_->Reset();
> > On 2015/03/10 03:28:47, Charlie Reis wrote:
> > > Hmm, how is this different than before?  clipboard_writer_ is a scoped_ptr
> and
> > > the last member of the class, so it would be the first to be deleted. 
What
> am
> > I
> > > missing?
> > 
> > ScopedClipboardWriter flushes internal data to the clipboard when it is
> > destroyed. It also has a Reset() member function, which clears that internal
> > data (which turns ~ScopedClipboardWriter into a no-op).
> > 
> > We are calling SCW::Reset here, not scoped_ptr::reset (the naming collision
is
> a
> > bit unfortunate).
> 
> Ah!  Reset vs reset is the bit I was missing.  :)
> 
> content/ LGTM.

Thanks!

[Martin Barbella, 2015-03-10 04:16:31]

The CQ bit was checked by mbarbella@chromium.org

[commit-bot: I haz the power, 2015-03-10 04:16:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/987583005/1

[commit-bot: I haz the power, 2015-03-10 05:41:01]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-03-10 05:41:49]

Patchset 1 (id:??) landed as
https://crrev.com/3dae1709c63b598c867e0108eb8a4e0535b3c1ac
Cr-Commit-Position: refs/heads/master@{#319838}