Revert 122916 - 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/)


Review URL: http://codereview.chromium.org/9419036

TBR=raymes@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9433019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@122921 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 2 insertions, 45 deletions

diff --git a/ui/base/clipboard/clipboard.h b/ui/base/clipboard/clipboard.h
index 05bbb4a..23f000f 100644
--- a/ui/base/clipboard/clipboard.h
+++ b/ui/base/clipboard/clipboard.h
@@ -14,7 +14,6 @@
 #include "base/process.h"
 #include "base/shared_memory.h"
 #include "base/string16.h"
-#include "base/threading/thread_checker.h"
 #include "ui/base/ui_export.h"
 
 #if defined(TOOLKIT_USES_GTK)
@@ -47,7 +46,7 @@ class NSString;
 
 namespace ui {
 
-class UI_EXPORT Clipboard : public base::ThreadChecker {
+class UI_EXPORT Clipboard {
  public:
   // MIME type constants.
   static const char kMimeTypeText[];
diff --git a/ui/base/clipboard/clipboard_aurax11.cc b/ui/base/clipboard/clipboard_aurax11.cc
index bac440c..ee3975d 100644
--- a/ui/base/clipboard/clipboard_aurax11.cc
+++ b/ui/base/clipboard/clipboard_aurax11.cc
@@ -447,18 +447,15 @@ bool Clipboard::FormatType::Equals(const FormatType& other) const {
 }
 
 Clipboard::Clipboard() {
-  DCHECK(CalledOnValidThread());
   // Make sure clipboard is created.
   GetClipboard();
 }
 
 Clipboard::~Clipboard() {
-  DCHECK(CalledOnValidThread());
   DeleteClipboard();
 }
 
 void Clipboard::WriteObjects(const ObjectMap& objects) {
-  DCHECK(CalledOnValidThread());
   for (ObjectMap::const_iterator iter = objects.begin();
        iter != objects.end(); ++iter) {
     DispatchObject(static_cast<ObjectType>(iter->first), iter->second);
@@ -468,7 +465,6 @@ void Clipboard::WriteObjects(const ObjectMap& objects) {
 
 bool Clipboard::IsFormatAvailable(const FormatType& format,
                                   Buffer buffer) const {
-  DCHECK(CalledOnValidThread());
   DCHECK(IsValidBuffer(buffer));
   AuraClipboard* clipboard = GetClipboard();
   if (GetPlainTextFormatType().Equals(format))
@@ -491,7 +487,6 @@ bool Clipboard::IsFormatAvailable(const FormatType& format,
 
 void Clipboard::ReadAvailableTypes(Buffer buffer, std::vector<string16>* types,
     bool* contains_filenames) const {
-  DCHECK(CalledOnValidThread());
   if (!types || !contains_filenames) {
     NOTREACHED();
     return;
@@ -518,12 +513,10 @@ void Clipboard::ReadAvailableTypes(Buffer buffer, std::vector<string16>* types,
 }
 
 void Clipboard::ReadText(Buffer buffer, string16* result) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadText(result);
 }
 
 void Clipboard::ReadAsciiText(Buffer buffer, std::string* result) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadAsciiText(result);
 }
 
@@ -532,29 +525,24 @@ void Clipboard::ReadHTML(Buffer buffer,
                          std::string* src_url,
                          uint32* fragment_start,
                          uint32* fragment_end) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadHTML(markup, src_url, fragment_start, fragment_end);
 }
 
 SkBitmap Clipboard::ReadImage(Buffer buffer) const {
-  DCHECK(CalledOnValidThread());
   return GetClipboard()->ReadImage();
 }
 
 void Clipboard::ReadCustomData(Buffer buffer,
                                const string16& type,
                                string16* result) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadCustomData(type, result);
 }
 
 void Clipboard::ReadBookmark(string16* title, std::string* url) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadBookmark(title, url);
 }
 
 void Clipboard::ReadFile(FilePath* file) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadFile(file);
 }
 
@@ -563,12 +551,10 @@ void Clipboard::ReadFiles(std::vector<FilePath>* files) const {
 }
 
 void Clipboard::ReadData(const FormatType& format, std::string* result) const {
-  DCHECK(CalledOnValidThread());
   GetClipboard()->ReadData(format.ToString(), result);
 }
 
 uint64 Clipboard::GetSequenceNumber(Buffer buffer) {
-  DCHECK(CalledOnValidThread());
   return GetClipboard()->GetNumClipboardEntries();
 }
 
diff --git a/ui/base/clipboard/clipboard_gtk.cc b/ui/base/clipboard/clipboard_gtk.cc
index a3df8ef..a63bb8e 100644
--- a/ui/base/clipboard/clipboard_gtk.cc
+++ b/ui/base/clipboard/clipboard_gtk.cc
@@ -210,18 +210,15 @@ bool Clipboard::FormatType::Equals(const FormatType& other) const {
 }
 
 Clipboard::Clipboard() : clipboard_data_(NULL) {
-  DCHECK(CalledOnValidThread());
   clipboard_ = gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
   primary_selection_ = gtk_clipboard_get(GDK_SELECTION_PRIMARY);
 }
 
 Clipboard::~Clipboard() {
-  DCHECK(CalledOnValidThread());
   gtk_clipboard_store(clipboard_);
 }
 
 void Clipboard::WriteObjects(const ObjectMap& objects) {
-  DCHECK(CalledOnValidThread());
   clipboard_data_ = new TargetMap();
 
   for (ObjectMap::const_iterator iter = objects.begin();
@@ -236,7 +233,6 @@ void Clipboard::WriteObjects(const ObjectMap& objects) {
 // location", for example), we additionally stick it in the X clipboard. This
 // matches other linux browsers.
 void Clipboard::DidWriteURL(const std::string& utf8_text) {
-  DCHECK(CalledOnValidThread());
   gtk_clipboard_set_text(primary_selection_, utf8_text.c_str(),
                          utf8_text.length());
 }
@@ -351,7 +347,6 @@ void Clipboard::WriteData(const FormatType& format,
 // and does not always refresh the cache when it is appropriate.
 bool Clipboard::IsFormatAvailable(const Clipboard::FormatType& format,
                                   Clipboard::Buffer buffer) const {
-  DCHECK(CalledOnValidThread());
   GtkClipboard* clipboard = LookupBackingClipboard(buffer);
   if (clipboard == NULL)
     return false;
@@ -401,7 +396,6 @@ bool Clipboard::IsFormatAvailable(const Clipboard::FormatType& format,
 void Clipboard::ReadAvailableTypes(Clipboard::Buffer buffer,
                                    std::vector<string16>* types,
                                    bool* contains_filenames) const {
-  DCHECK(CalledOnValidThread());
   if (!types || !contains_filenames) {
     NOTREACHED();
     return;
@@ -432,7 +426,6 @@ void Clipboard::ReadAvailableTypes(Clipboard::Buffer buffer,
 
 
 void Clipboard::ReadText(Clipboard::Buffer buffer, string16* result) const {
-  DCHECK(CalledOnValidThread());
   GtkClipboard* clipboard = LookupBackingClipboard(buffer);
   if (clipboard == NULL)
     return;
@@ -450,7 +443,6 @@ void Clipboard::ReadText(Clipboard::Buffer buffer, string16* result) const {
 
 void Clipboard::ReadAsciiText(Clipboard::Buffer buffer,
                               std::string* result) const {
-  DCHECK(CalledOnValidThread());
   GtkClipboard* clipboard = LookupBackingClipboard(buffer);
   if (clipboard == NULL)
     return;
@@ -466,7 +458,6 @@ void Clipboard::ReadAsciiText(Clipboard::Buffer buffer,
 }
 
 void Clipboard::ReadFile(FilePath* file) const {
-  DCHECK(CalledOnValidThread());
   *file = FilePath();
 }
 
@@ -475,7 +466,6 @@ void Clipboard::ReadFile(FilePath* file) const {
 void Clipboard::ReadHTML(Clipboard::Buffer buffer, string16* markup,
                          std::string* src_url, uint32* fragment_start,
                          uint32* fragment_end) const {
-  DCHECK(CalledOnValidThread());
   markup->clear();
   if (src_url)
     src_url->clear();
@@ -516,7 +506,6 @@ void Clipboard::ReadHTML(Clipboard::Buffer buffer, string16* markup,
 }
 
 SkBitmap Clipboard::ReadImage(Buffer buffer) const {
-  DCHECK(CalledOnValidThread());
   ScopedGObject<GdkPixbuf>::Type pixbuf(
       gtk_clipboard_wait_for_image(clipboard_));
   if (!pixbuf.get())
@@ -537,7 +526,6 @@ SkBitmap Clipboard::ReadImage(Buffer buffer) const {
 void Clipboard::ReadCustomData(Buffer buffer,
                                const string16& type,
                                string16* result) const {
-  DCHECK(CalledOnValidThread());
   GtkClipboard* clipboard = LookupBackingClipboard(buffer);
   if (!clipboard)
     return;
@@ -558,7 +546,6 @@ void Clipboard::ReadBookmark(string16* title, std::string* url) const {
 }
 
 void Clipboard::ReadData(const FormatType& format, std::string* result) const {
-  DCHECK(CalledOnValidThread());
   GtkSelectionData* data =
       gtk_clipboard_wait_for_contents(clipboard_, format.ToGdkAtom());
   if (!data)
@@ -570,7 +557,6 @@ void Clipboard::ReadData(const FormatType& format, std::string* result) const {
 }
 
 uint64 Clipboard::GetSequenceNumber(Buffer buffer) {
-  DCHECK(CalledOnValidThread());
   if (buffer == BUFFER_STANDARD)
     return SelectionChangeObserver::GetInstance()->clipboard_sequence_number();
   else
diff --git a/ui/base/clipboard/clipboard_mac.mm b/ui/base/clipboard/clipboard_mac.mm
index 661d057..c4671a1 100644
--- a/ui/base/clipboard/clipboard_mac.mm
+++ b/ui/base/clipboard/clipboard_mac.mm
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -79,15 +79,12 @@ Clipboard::FormatType Clipboard::FormatType::Deserialize(
 }
 
 Clipboard::Clipboard() {
-  DCHECK(CalledOnValidThread());
 }
 
 Clipboard::~Clipboard() {
-  DCHECK(CalledOnValidThread());
 }
 
 void Clipboard::WriteObjects(const ObjectMap& objects) {
-  DCHECK(CalledOnValidThread());
   NSPasteboard* pb = GetPasteboard();
   [pb declareTypes:[NSArray array] owner:nil];
 
@@ -212,7 +209,6 @@ void Clipboard::WriteWebSmartPaste() {
 }
 
 uint64 Clipboard::GetSequenceNumber(Buffer buffer) {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
 
   NSPasteboard* pb = GetPasteboard();
@@ -221,7 +217,6 @@ uint64 Clipboard::GetSequenceNumber(Buffer buffer) {
 
 bool Clipboard::IsFormatAvailable(const FormatType& format,
                                   Buffer buffer) const {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
 
   NSPasteboard* pb = GetPasteboard();
@@ -239,7 +234,6 @@ bool Clipboard::IsFormatAvailable(const FormatType& format,
 void Clipboard::ReadAvailableTypes(Clipboard::Buffer buffer,
                                    std::vector<string16>* types,
                                    bool* contains_filenames) const {
-  DCHECK(CalledOnValidThread());
   types->clear();
   if (IsFormatAvailable(Clipboard::GetPlainTextFormatType(), buffer))
     types->push_back(UTF8ToUTF16(kMimeTypeText));
@@ -258,7 +252,6 @@ void Clipboard::ReadAvailableTypes(Clipboard::Buffer buffer,
 }
 
 void Clipboard::ReadText(Clipboard::Buffer buffer, string16* result) const {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
   NSPasteboard* pb = GetPasteboard();
   NSString* contents = [pb stringForType:NSStringPboardType];
@@ -270,7 +263,6 @@ void Clipboard::ReadText(Clipboard::Buffer buffer, string16* result) const {
 
 void Clipboard::ReadAsciiText(Clipboard::Buffer buffer,
                               std::string* result) const {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
   NSPasteboard* pb = GetPasteboard();
   NSString* contents = [pb stringForType:NSStringPboardType];
@@ -284,7 +276,6 @@ void Clipboard::ReadAsciiText(Clipboard::Buffer buffer,
 void Clipboard::ReadHTML(Clipboard::Buffer buffer, string16* markup,
                          std::string* src_url, uint32* fragment_start,
                          uint32* fragment_end) const {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
 
   // TODO(avi): src_url?
@@ -313,7 +304,6 @@ void Clipboard::ReadHTML(Clipboard::Buffer buffer, string16* markup,
 }
 
 SkBitmap Clipboard::ReadImage(Buffer buffer) const {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
 
   scoped_nsobject<NSImage> image(
@@ -344,7 +334,6 @@ SkBitmap Clipboard::ReadImage(Buffer buffer) const {
 void Clipboard::ReadCustomData(Buffer buffer,
                                const string16& type,
                                string16* result) const {
-  DCHECK(CalledOnValidThread());
   DCHECK_EQ(buffer, BUFFER_STANDARD);
 
   NSPasteboard* pb = GetPasteboard();
@@ -356,7 +345,6 @@ void Clipboard::ReadCustomData(Buffer buffer,
 }
 
 void Clipboard::ReadBookmark(string16* title, std::string* url) const {
-  DCHECK(CalledOnValidThread());
   NSPasteboard* pb = GetPasteboard();
 
   if (title) {
@@ -376,7 +364,6 @@ void Clipboard::ReadBookmark(string16* title, std::string* url) const {
 }
 
 void Clipboard::ReadFile(FilePath* file) const {
-  DCHECK(CalledOnValidThread());
   if (!file) {
     NOTREACHED();
     return;
@@ -409,7 +396,6 @@ void Clipboard::ReadFiles(std::vector<FilePath>* files) const {
 }
 
 void Clipboard::ReadData(const FormatType& format, std::string* result) const {
-  DCHECK(CalledOnValidThread());
   NSPasteboard* pb = GetPasteboard();
   NSData* data = [pb dataForType:format.ToNSString()];
   if ([data length])