PrerenderingLoader initial integration with PrerenderManager/PrerenderHandle

chrome/browser/android/offline_pages/prerendering_offliner.cc

Index: chrome/browser/android/offline_pages/prerendering_offliner.cc
diff --git a/chrome/browser/android/offline_pages/prerendering_offliner.cc b/chrome/browser/android/offline_pages/prerendering_offliner.cc
index c38afcb7ab8f248ff3751aa2ecabf301cdf3c91c..b654c75a6b5620ebe6bf03e7d525daa88a111cb0 100644
--- a/chrome/browser/android/offline_pages/prerendering_offliner.cc
+++ b/chrome/browser/android/offline_pages/prerendering_offliner.cc
@@ -20,9 +20,8 @@ PrerenderingOffliner::PrerenderingOffliner(
 
 PrerenderingOffliner::~PrerenderingOffliner() {}
 
-void PrerenderingOffliner::OnLoadPageDone(
-    const Offliner::CompletionStatus load_status,
-    content::WebContents* contents) {
+void PrerenderingOffliner::OnLoadPageDone(Offliner::RequestStatus load_status,
+                                          content::WebContents* contents) {
   // TODO(dougarnett): Implement save attempt and running CompletionCallback.
 }
 
@@ -41,6 +40,12 @@ void PrerenderingOffliner::Cancel() {
   GetOrCreateLoader()->StopLoading();
 }
 
+void PrerenderingOffliner::SetLoaderForTesting(
+    std::unique_ptr<PrerenderingLoader> loader) {
+  DCHECK(!loader_);
+  loader_ = std::move(loader);

[pasko, 2016/05/20 19:04:53]

std::move is a cast to an rvalue, and avoiding a cast seems beneficial, so how
about:

loader_.swap(loader)

?

[dougarnett, 2016/05/20 22:21:12]

I don't know about that. This is the pattern I see for move-ing ownership and
swap seems confusing to me wrt that intent. It is not hard to find examples of
this pattern in online C++ guides. Do you have examples or doc pointers on swap
being preferred?

[pasko, 2016/05/23 20:05:41]

There is no official recommendation to use one or the other. My intuition leans
towards .swap(), but now that I think about it, this may be slightly more
expensive than std::move, so I would understand why std::move would be
preferable now.

I saw examples of both in the code. If the code around .../android/offline_pages
prefers std::move, then it is preferable to be consistent and use std::move here
as well.

Thank you for background!

+}
+
 PrerenderingLoader* PrerenderingOffliner::GetOrCreateLoader() {
   if (!loader_) {
     loader_.reset(new PrerenderingLoader(browser_context_));
@@ -48,10 +53,4 @@ PrerenderingLoader* PrerenderingOffliner::GetOrCreateLoader() {
   return loader_.get();
 }
 
-void PrerenderingOffliner::SetLoaderForTesting(
-    std::unique_ptr<PrerenderingLoader> loader) {
-  DCHECK(!loader_);
-  loader_ = std::move(loader);
-}
-
 }  // namespace offline_pages