Change all platforms except Windows to access the clipboard solely from the UI thread

Clipboard access is not threadsafe in general. Previously clipboard reads were occuring on the IO thread while writes were on the UI thread, leading to a race condition in the clipboard code. This fixes that issue by placing all clipboard accesses on the UI thread and adding thread checks to ensure that all accesses occur from the same thread.

Windows is an exception to this rule where clipboard reads need to occur on the IO thread to avoid deadlocks with NPAPI plugins. This is ok because the windows clipboard is threadsafe. 

BUG=114648
TEST=Tested clipboard by hand on linux. Ran trybots (all passed except flakiness on linux_chromeos). Ran new PPAPI test that was triggering the crash (http://codereview.chromium.org/9212066/)


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=122916

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=123088

[dcheng, 2012-02-17 19:43:21]

Should this change apply to the Clipboard::Write* methods as well?

https://chromiumcodereview.appspot.com/9419036/diff/2002/content/browser/rend...
File content/browser/renderer_host/clipboard_message_filter.cc (right):

https://chromiumcodereview.appspot.com/9419036/diff/2002/content/browser/rend...
content/browser/renderer_host/clipboard_message_filter.cc:53: *thread =
BrowserThread::UI;
Doesn't this change result in us posting a task from the UI thread to the UI
thread on non-Windows platforms? See lines 89-127. Should we change that to
avoid posting a task redundantly?

https://chromiumcodereview.appspot.com/9419036/diff/2002/ui/base/clipboard/cl...
File ui/base/clipboard/clipboard.h (right):

https://chromiumcodereview.appspot.com/9419036/diff/2002/ui/base/clipboard/cl...
ui/base/clipboard/clipboard.h:256: void ReadFiles(std::vector<FilePath>* files)
const;
Why was this line moved?

[raymes, 2012-02-17 23:14:03]

https://chromiumcodereview.appspot.com/9419036/diff/2002/content/browser/rend...
File content/browser/renderer_host/clipboard_message_filter.cc (right):

https://chromiumcodereview.appspot.com/9419036/diff/2002/content/browser/rend...
content/browser/renderer_host/clipboard_message_filter.cc:53: *thread =
BrowserThread::UI;
On 2012/02/17 19:43:22, dcheng wrote:
> Doesn't this change result in us posting a task from the UI thread to the UI
> thread on non-Windows platforms? See lines 89-127. Should we change that to
> avoid posting a task redundantly?

Sounds good, I was considering this but wasn't sure. Updated in the new patch
set.

https://chromiumcodereview.appspot.com/9419036/diff/2002/ui/base/clipboard/cl...
File ui/base/clipboard/clipboard.h (right):

https://chromiumcodereview.appspot.com/9419036/diff/2002/ui/base/clipboard/cl...
ui/base/clipboard/clipboard.h:256: void ReadFiles(std::vector<FilePath>* files)
const;
On 2012/02/17 19:43:22, dcheng wrote:
> Why was this line moved?

AFAICT this function is only used internally in the clipboard implementations
and there is no implementation on linux. So any usage of this function
externally would result in a linker error on linux. Alternatively I could #ifdef
it but noone is using it anyway. Let me know your thoughts.

[jam, 2012-02-18 02:13:11]

content lgtm

http://codereview.chromium.org/9419036/diff/8002/content/browser/renderer_hos...
File content/browser/renderer_host/clipboard_message_filter.cc (right):

http://codereview.chromium.org/9419036/diff/8002/content/browser/renderer_hos...
content/browser/renderer_host/clipboard_message_filter.cc:44: // thread-safe, so
all clipboard calls should be serviced from the UI thread.
"thread-safe," can be on the previous line

[raymes, 2012-02-21 18:00:43]

https://chromiumcodereview.appspot.com/9419036/diff/2002/ui/base/clipboard/cl...
File ui/base/clipboard/clipboard.h (right):

https://chromiumcodereview.appspot.com/9419036/diff/2002/ui/base/clipboard/cl...
ui/base/clipboard/clipboard.h:256: void ReadFiles(std::vector<FilePath>* files)
const;
I removed this change as not to conflict with your deletion CL.
On 2012/02/17 23:14:03, raymes wrote:
> On 2012/02/17 19:43:22, dcheng wrote:
> > Why was this line moved?
> 
> AFAICT this function is only used internally in the clipboard implementations
> and there is no implementation on linux. So any usage of this function
> externally would result in a linker error on linux. Alternatively I could
#ifdef
> it but noone is using it anyway. Let me know your thoughts.

https://chromiumcodereview.appspot.com/9419036/diff/8002/content/browser/rend...
File content/browser/renderer_host/clipboard_message_filter.cc (right):

https://chromiumcodereview.appspot.com/9419036/diff/8002/content/browser/rend...
content/browser/renderer_host/clipboard_message_filter.cc:44: // thread-safe, so
all clipboard calls should be serviced from the UI thread.
On 2012/02/18 02:13:11, John Abd-El-Malek wrote:
> "thread-safe," can be on the previous line

Done.

[dcheng, 2012-02-21 19:13:53]

lgtm

[Ben Goodger (Google), 2012-02-21 22:05:21]

lgtm

[commit-bot: I haz the power, 2012-02-21 22:07:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/9419036/9001

[commit-bot: I haz the power, 2012-02-21 23:50:15]

Change committed as 122916

[commit-bot: I haz the power, 2012-02-22 17:21:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/9419036/11005

[commit-bot: I haz the power, 2012-02-22 17:21:03]

Can't apply patch for file ui/base/clipboard/clipboard_aurax11.cc.
While running patch -p1 --forward --force;
patching file ui/base/clipboard/clipboard_aurax11.cc
Hunk #1 FAILED at 447.
Hunk #2 succeeded at 435 (offset -30 lines).
Hunk #3 succeeded at 456 (offset -32 lines).
Hunk #4 succeeded at 479 (offset -36 lines).
Hunk #5 FAILED at 529.
Hunk #6 succeeded at 511 (offset -44 lines).
2 out of 6 hunks FAILED -- saving rejects to file
ui/base/clipboard/clipboard_aurax11.cc.rej

[commit-bot: I haz the power, 2012-02-22 18:10:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/raymes@chromium.org/9419036/15001

[commit-bot: I haz the power, 2012-02-22 19:54:04]

Change committed as 123088