summaryrefslogtreecommitdiffstats
path: root/chrome/browser/printing
diff options
context:
space:
mode:
authordarin@chromium.org <darin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-09-28 18:12:55 +0000
committerdarin@chromium.org <darin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-09-28 18:12:55 +0000
commit13f383ff5fc8ff095501794d4ce758f0067ff9b5 (patch)
treeceb1d08f38759bf8805ede009924a990bae496e5 /chrome/browser/printing
parentf61f331824ac9c98684a886be0d918aefe69d3ce (diff)
downloadchromium_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.h11
-rw-r--r--chrome/browser/printing/print_job_unittest.cc10
-rw-r--r--chrome/browser/printing/print_job_worker.cc3
-rw-r--r--chrome/browser/printing/print_job_worker.h1
-rw-r--r--chrome/browser/printing/print_job_worker_owner.h6
-rw-r--r--chrome/browser/printing/printer_query.h10
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);