chromeos: Destruct DriveResourceMetadata on the blocking pool

chrome/browser/chromeos/drive/drive_test_util.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_TEST_UTIL_H_
 #define CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_TEST_UTIL_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/chromeos/drive/drive_resource_metadata.h"
 #include "chrome/browser/chromeos/drive/search_metadata.h"
@@ skipping 151 matching lines @@
 void CopyResultsFromCloseFileCallbackAndQuit(DriveFileError* out_error,
                                              DriveFileError error);
 
 // Loads a test json file as root ("/drive") element from a test file stored
 // under chrome/test/data/chromeos. Returns true on success.
 bool LoadChangeFeed(const std::string& relative_path,
                     ChangeListLoader* change_list_loader,
                     bool is_delta_feed,
                     int64 root_feed_changestamp);
 
-// DriveCache has private destructor, so it is impossible to delete the
-// instance directly. This method delete it correctly.
-void DeleteDriveCache(DriveCache* drive_cache);
+// Helper to destroy objects which needs Destroy() to be called on destruction.

[hidehiko, 2013/03/18 13:16:28]

How about adding comment that it is necessary to invoke reset() explicitly in
TearDown rather than waiting for the destruction of test fixture?

[hashimoto, 2013/03/18 13:40:10]

It's not necessary as long as the blocking task runner and the UI thread (needed
to DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)) ) lives longer than the
object being destructed.

reset() is called manually in DriveResourceMetadataTest::TearDown because the
metadata object is destructed earlier than the UI thread. (protected member is
placed before private members)
In other tests where reset() is called manually, I was simply lazy.

If you like it, I'd change the code to stop calling reset() manually when
possible.

[hidehiko, 2013/03/18 13:57:44]

I understand.
Could you add short comment about it is necessary to be destroyed before threads
are died?

As for manual method() invocation, I'm fine with either way.

On 2013/03/18 13:40:10, hashimoto wrote:

[hashimoto, 2013/03/18 15:15:34]

Added a note.

+template<typename T>
+struct DestroyHelperForTests {

[hidehiko, 2013/03/18 13:16:28]

ditto.

[hashimoto, 2013/03/18 13:40:10]

Done.

+  // scoped_ptr for T which calls Destroy() and RunBlockingPoolTask() on
+  // destruction.
+  typedef scoped_ptr<T, DestroyHelperForTests<T> > scoped_ptr;
+
+  void operator()(T* object) const {
+    if (object)
+      object->Destroy();
+    google_apis::test_util::RunBlockingPoolTask();  // Finish destruction.
+  }
+};
 
 }  // namespace test_util
 }  // namespace drive
 
 #endif  // CHROME_BROWSER_CHROMEOS_DRIVE_DRIVE_TEST_UTIL_H_