Updates to PrerenderingOffliner and PrerenderingLoader interfaces to push PrerenderManager knowledge

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 b6d834be1f87df73b59c7582489df0e665eed38d..1693d5bb6a06eb52810592c8289b4c6376567784 100644
--- a/chrome/browser/android/offline_pages/prerendering_offliner.cc
+++ b/chrome/browser/android/offline_pages/prerendering_offliner.cc
@@ -4,23 +4,54 @@
 
 #include "chrome/browser/android/offline_pages/prerendering_offliner.h"
 
+#include "base/bind.h"
+#include "components/offline_pages/background/save_page_request.h"
+#include "content/public/browser/browser_context.h"
+
 namespace offline_pages {
 
-PrerenderingOffliner::PrerenderingOffliner(const OfflinerPolicy* policy,
-                                           PrerenderManager* prerender_manager,
-                                           OfflinePageModel* offline_page_model)
-    : loader_(new PrerenderingLoader(prerender_manager)) {}
+PrerenderingOffliner::PrerenderingOffliner(
+    content::BrowserContext* browser_context,
+    const OfflinerPolicy* policy,
+    OfflinePageModel* offline_page_model)
+    : browser_context_(browser_context),
+      offline_page_model_(offline_page_model),
+      weak_ptr_factory_(this) {}
 
 PrerenderingOffliner::~PrerenderingOffliner() {}
 
+void PrerenderingOffliner::OnLoadPageDone(
+    const Offliner::CompletionStatus load_status,
+    content::WebContents* contents) {
+  // TODO(dougarnett): Implement save attempt and running CompletionCallback.
+}
+
 bool PrerenderingOffliner::LoadAndSave(const SavePageRequest& request,
                                        const CompletionCallback& callback) {
-  // TODO(dougarnett): implement.
-  return false;
+  if (!offline_page_model_->CanSavePage(request.url()))
+    return false;
+
+  // Kick off load page attempt.
+  return (GetOrCreateLoader()->LoadPage(

[fgorski, 2016/05/10 03:52:43]

The outer () are not necessary, and they don't improve readability of this code.

[dougarnett, 2016/05/10 15:30:19]

Done.

+      request.url(), base::Bind(&PrerenderingOffliner::OnLoadPageDone,
+                                weak_ptr_factory_.GetWeakPtr())));
 }
 
 void PrerenderingOffliner::Cancel() {
-  loader_->StopLoading();
+  GetOrCreateLoader()->StopLoading();
+}
+
+PrerenderingLoader* PrerenderingOffliner::GetOrCreateLoader() {
+  if (!loader_) {
+    loader_ = std::unique_ptr<PrerenderingLoader>(

[fgorski, 2016/05/10 03:52:43]

I think a more concise form would be:

loader_.reset(new PrerenderingLoader(browser_context_));

What do you think?

And below in 54, I think assignment is the right choice.

+        new PrerenderingLoader(browser_context_));
+  }
+  return loader_.get();
+}
+
+void PrerenderingOffliner::SetLoaderForTesting(
+    std::unique_ptr<PrerenderingLoader> loader) {

[fgorski, 2016/05/10 03:52:43]

If you want to prevent accidentally setting the loader twice in your tests you
can add:
DCHECK(!loader_);

[dougarnett, 2016/05/10 15:30:19]

Done.

+  loader_ = std::move(loader);
 }
 
 }  // namespace offline_pages