diff options
author | darin@chromium.org <darin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-09-28 18:12:55 +0000 |
---|---|---|
committer | darin@chromium.org <darin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-09-28 18:12:55 +0000 |
commit | 13f383ff5fc8ff095501794d4ce758f0067ff9b5 (patch) | |
tree | ceb1d08f38759bf8805ede009924a990bae496e5 /chrome/browser/printing | |
parent | f61f331824ac9c98684a886be0d918aefe69d3ce (diff) | |
download | chromium_src-13f383ff5fc8ff095501794d4ce758f0067ff9b5.zip chromium_src-13f383ff5fc8ff095501794d4ce758f0067ff9b5.tar.gz chromium_src-13f383ff5fc8ff095501794d4ce758f0067ff9b5.tar.bz2 |
Assert that thread-safe reference counting is used with
cross-thread NewRunnableMethod.
This assertion caught such an error in VisitedLinkMaster!
My approach, modify RunnableMethodTraits<T> to assert that
when ReleaseCallee happens on a different thread from
RetainCallee that the type supports thread-safe reference
counting. I do this by adding a static method to both
RefCounted<T> and RefCountedThreadSafe<T>.
This results in a little ugliness in cases where people
implement AddRef and Release by hand (to make the no-ops).
There may be a nicer way to deal with those few cases.
R=brettw
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/251012
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27379 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/printing')
-rw-r--r-- | chrome/browser/printing/print_job.h | 11 | ||||
-rw-r--r-- | chrome/browser/printing/print_job_unittest.cc | 10 | ||||
-rw-r--r-- | chrome/browser/printing/print_job_worker.cc | 3 | ||||
-rw-r--r-- | chrome/browser/printing/print_job_worker.h | 1 | ||||
-rw-r--r-- | chrome/browser/printing/print_job_worker_owner.h | 6 | ||||
-rw-r--r-- | chrome/browser/printing/printer_query.h | 10 |
6 files changed, 7 insertions, 34 deletions
diff --git a/chrome/browser/printing/print_job.h b/chrome/browser/printing/print_job.h index 45d5d12..cf4badc 100644 --- a/chrome/browser/printing/print_job.h +++ b/chrome/browser/printing/print_job.h @@ -8,7 +8,6 @@ #include "base/basictypes.h" #include "base/gfx/native_widget_types.h" #include "base/message_loop.h" -#include "base/ref_counted.h" #include "chrome/browser/printing/print_job_worker_owner.h" #include "chrome/common/notification_registrar.h" @@ -32,9 +31,8 @@ class PrinterQuery; // any state change. While printing, the PrintJobManager instance keeps a // reference to the job to be sure it is kept alive. All the code in this class // runs in the UI thread. -class PrintJob : public base::RefCountedThreadSafe<PrintJob>, +class PrintJob : public PrintJobWorkerOwner, public NotificationObserver, - public PrintJobWorkerOwner, public MessageLoop::DestructionObserver { public: // Create a empty PrintJob. When initializing with this constructor, @@ -52,13 +50,6 @@ class PrintJob : public base::RefCountedThreadSafe<PrintJob>, const NotificationDetails& details); // PrintJobWorkerOwner - virtual void AddRef() { - return base::RefCountedThreadSafe<PrintJob>::AddRef(); - } - virtual void Release() { - return base::RefCountedThreadSafe<PrintJob>::Release(); - } - virtual void GetSettingsDone(const PrintSettings& new_settings, PrintingContext::Result result); virtual PrintJobWorker* DetachWorker(PrintJobWorkerOwner* new_owner); diff --git a/chrome/browser/printing/print_job_unittest.cc b/chrome/browser/printing/print_job_unittest.cc index fe98964..3ef795c 100644 --- a/chrome/browser/printing/print_job_unittest.cc +++ b/chrome/browser/printing/print_job_unittest.cc @@ -33,12 +33,6 @@ class TestPrintJobWorker : public printing::PrintJobWorker { class TestOwner : public printing::PrintJobWorkerOwner { public: - virtual void AddRef() { - EXPECT_FALSE(true); - } - virtual void Release() { - EXPECT_FALSE(true); - } virtual void GetSettingsDone(const printing::PrintSettings& new_settings, printing::PrintingContext::Result result) { EXPECT_FALSE(true); @@ -104,9 +98,9 @@ TEST(PrintJobTest, SimplePrint) { volatile bool check = false; scoped_refptr<printing::PrintJob> job(new TestPrintJob(&check)); EXPECT_EQ(MessageLoop::current(), job->message_loop()); - TestOwner owner; + scoped_refptr<TestOwner> owner(new TestOwner); TestSource source; - job->Initialize(&owner, &source); + job->Initialize(owner, &source); job->Stop(); job = NULL; EXPECT_TRUE(check); diff --git a/chrome/browser/printing/print_job_worker.cc b/chrome/browser/printing/print_job_worker.cc index caf5ad3..375c2407 100644 --- a/chrome/browser/printing/print_job_worker.cc +++ b/chrome/browser/printing/print_job_worker.cc @@ -275,9 +275,6 @@ void PrintJobWorker::OnFailure() { } // namespace printing -RunnableMethodTraits<printing::PrintJobWorker>::RunnableMethodTraits() { -} - void RunnableMethodTraits<printing::PrintJobWorker>::RetainCallee( printing::PrintJobWorker* obj) { DCHECK(!owner_.get()); diff --git a/chrome/browser/printing/print_job_worker.h b/chrome/browser/printing/print_job_worker.h index 9ad2c39..fce8912 100644 --- a/chrome/browser/printing/print_job_worker.h +++ b/chrome/browser/printing/print_job_worker.h @@ -98,7 +98,6 @@ class PrintJobWorker : public base::Thread { template <> struct RunnableMethodTraits<printing::PrintJobWorker> { - RunnableMethodTraits(); void RetainCallee(printing::PrintJobWorker* obj); void ReleaseCallee(printing::PrintJobWorker* obj); private: diff --git a/chrome/browser/printing/print_job_worker_owner.h b/chrome/browser/printing/print_job_worker_owner.h index 1810eca..e1ba719 100644 --- a/chrome/browser/printing/print_job_worker_owner.h +++ b/chrome/browser/printing/print_job_worker_owner.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_PRINTING_PRINT_JOB_WORKER_OWNER_H__ #define CHROME_BROWSER_PRINTING_PRINT_JOB_WORKER_OWNER_H__ +#include "base/ref_counted.h" #include "printing/printing_context.h" class MessageLoop; @@ -14,12 +15,11 @@ namespace printing { class PrintJobWorker; class PrintSettings; -class PrintJobWorkerOwner { +class PrintJobWorkerOwner : + public base::RefCountedThreadSafe<PrintJobWorkerOwner> { public: virtual ~PrintJobWorkerOwner() { } - virtual void AddRef() = 0; - virtual void Release() = 0; // Finishes the initialization began by PrintJobWorker::Init(). Creates a // new PrintedDocument if necessary. Solely meant to be called by diff --git a/chrome/browser/printing/printer_query.h b/chrome/browser/printing/printer_query.h index 6d6fd88..b0502c8 100644 --- a/chrome/browser/printing/printer_query.h +++ b/chrome/browser/printing/printer_query.h @@ -6,7 +6,6 @@ #define CHROME_BROWSER_PRINTING_PRINTER_QUERY_H_ #include "base/scoped_ptr.h" -#include "base/ref_counted.h" #include "chrome/browser/printing/print_job_worker_owner.h" class CancelableTask; @@ -21,8 +20,7 @@ namespace printing { class PrintJobWorker; // Query the printer for settings. -class PrinterQuery : public base::RefCountedThreadSafe<PrinterQuery>, - public PrintJobWorkerOwner { +class PrinterQuery : public PrintJobWorkerOwner { public: // GetSettings() UI parameter. enum GetSettingsAskParam { @@ -34,12 +32,6 @@ class PrinterQuery : public base::RefCountedThreadSafe<PrinterQuery>, virtual ~PrinterQuery(); // PrintJobWorkerOwner - virtual void AddRef() { - return base::RefCountedThreadSafe<PrinterQuery>::AddRef(); - } - virtual void Release() { - return base::RefCountedThreadSafe<PrinterQuery>::Release(); - } virtual void GetSettingsDone(const PrintSettings& new_settings, PrintingContext::Result result); virtual PrintJobWorker* DetachWorker(PrintJobWorkerOwner* new_owner); |