Remove clipboard argument from ScopedClipboardWriter constructor.

Remove clipboard argument from ScopedClipboardWriter constructor.

Unfortunately, early binding to a clipboard on one thread makes it hard
to implement some cleanups in the clipboard IPC handlers for writing
data. Since all existing callers simply pass in the clipboard for the
current thread, this patch removes the constructor parameter in favor of
having ScopedClipboardWriter use it itself internally.

BUG=319285
Committed: https://crrev.com/3749e4e5ae82c3ad4e379d347a115fb2a8ab489b
Cr-Commit-Position: refs/heads/master@{#295344}

[dcheng, 2014-09-17 06:19:37]

dcheng@chromium.org changed reviewers:
+ jamesr@chromium.org, sky@chromium.org

[dcheng, 2014-09-17 06:25:28]

This is a pretty straightforward CL, with the exception of a rather ugly hack in
ScopedClipboardWriterGlue.

In the past, there were certain test binaries that would use a different
clipboard implementation, which is why ScopedClipboardWriterGlue supported
writing to the clipboard using ui::Clipboard directly or going through
content::ClipboardClient. To signal which path to use, we'd make sure only one
of the fields was ever set to a non-NULL value. If clipboard_ was NULL, then
~ScopedClipboardWriterGlue would handle flushing data and the base destructor
would be a no-op. If clipboard_ was not NULL, then ~ScopedCliipboardWriterGlue
would be a no-op and the base destructor would handle flushing the data.

Now, there are no test binaries that ever pass a NULL
ClipboardClient::WriteContext and a non-NULL Clipboard to SCWG. So I've updated
SCWG to unconditionally flush data in its destructor. In order to prevent its
base class dtor from also trying to flush the data though, I clear the ObjectMap
in ~SCWG. This is pretty hacky, but given that SCWG will go away very soon
(https://codereview.chromium.org/574273002 gets rid of it), I think that's OK.

[dcheng, 2014-09-17 06:25:54]

On 2014/09/17 at 06:25:28, dcheng (OOO) wrote:
> This is a pretty straightforward CL, with the exception of a rather ugly hack
in ScopedClipboardWriterGlue.
> 
> In the past, there were certain test binaries that would use a different
clipboard implementation, which is why ScopedClipboardWriterGlue supported
writing to the clipboard using ui::Clipboard directly or going through
content::ClipboardClient. To signal which path to use, we'd make sure only one
of the fields was ever set to a non-NULL value. If clipboard_ was NULL, then
~ScopedClipboardWriterGlue would handle flushing data and the base destructor
would be a no-op. If clipboard_ was not NULL, then ~ScopedCliipboardWriterGlue
would be a no-op and the base destructor would handle flushing the data.
> 
> Now, there are no test binaries that ever pass a NULL
ClipboardClient::WriteContext and a non-NULL Clipboard to SCWG. So I've updated
SCWG to unconditionally flush data in its destructor. In order to prevent its
base class dtor from also trying to flush the data though, I clear the ObjectMap
in ~SCWG. This is pretty hacky, but given that SCWG will go away very soon
(https://codereview.chromium.org/574273002 gets rid of it), I think that's OK.

+jamesr for the content/renderer changes, and +sky for everything else.

[jamesr, 2014-09-17 06:57:01]

c/r/ lgtm

+erg

[sky, 2014-09-17 15:41:51]

LGTM

https://codereview.chromium.org/558913003/diff/40001/ui/base/clipboard/scoped...
File ui/base/clipboard/scoped_clipboard_writer.h (right):

https://codereview.chromium.org/558913003/diff/40001/ui/base/clipboard/scoped...
ui/base/clipboard/scoped_clipboard_writer.h:74: Clipboard::ObjectMap objects_;
style guide says members should be private.

[dcheng, 2014-09-17 17:10:49]

https://codereview.chromium.org/558913003/diff/40001/ui/base/clipboard/scoped...
File ui/base/clipboard/scoped_clipboard_writer.h (right):

https://codereview.chromium.org/558913003/diff/40001/ui/base/clipboard/scoped...
ui/base/clipboard/scoped_clipboard_writer.h:74: Clipboard::ObjectMap objects_;
On 2014/09/17 15:41:50, sky wrote:
> style guide says members should be private.

Yeah, ScopedClipboardWriterGlue directly modifies this atm. That will be fixed
soon by the CL I linked in my initial email.

[dcheng, 2014-09-17 17:16:01]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-09-17 17:16:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/558913003/40001

[Elliot Glaysher, 2014-09-17 17:32:59]

erg@chromium.org changed reviewers:
+ erg@chromium.org

[Elliot Glaysher, 2014-09-17 17:33:00]

lgtm

[commit-bot: I haz the power, 2014-09-17 18:40:55]

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

[commit-bot: I haz the power, 2014-09-17 18:40:56]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dcheng, 2014-09-17 19:22:20]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-09-17 19:22:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/558913003/40001

[dcheng, 2014-09-17 19:53:03]

The CQ bit was unchecked by dcheng@chromium.org

[dcheng, 2014-09-17 20:25:05]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-09-17 20:26:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/558913003/60001

[commit-bot: I haz the power, 2014-09-17 21:23:54]

Committed patchset #4 (id:60001) as ed1fad2bbfce5501f9061af7017619c061f313b3

[commit-bot: I haz the power, 2014-09-17 21:24:34]

Patchset 4 (id:??) landed as
https://crrev.com/3749e4e5ae82c3ad4e379d347a115fb2a8ab489b
Cr-Commit-Position: refs/heads/master@{#295344}