Integrates the SnapshotController into the PrerenderingLoader to provide logic for determining when…

chrome/browser/android/offline_pages/prerendering_loader.cc

 // Copyright 2016 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.
 
 #include "chrome/browser/android/offline_pages/prerendering_loader.h"
 
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace offline_pages {
 
 PrerenderingLoader::PrerenderingLoader(content::BrowserContext* browser_context)
     : state_(State::IDLE), browser_context_(browser_context) {
   adapter_.reset(new PrerenderAdapter(this));
+  snapshot_controller_.reset(
+      new SnapshotController(base::ThreadTaskRunnerHandle::Get(), this));
 }
 
 PrerenderingLoader::~PrerenderingLoader() {
   CancelPrerender();
 }
 
 bool PrerenderingLoader::LoadPage(const GURL& url,
                                   const LoadPageCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (!IsIdle()) {
@@ skipping 11 matching lines @@
       content::WebContents::CreateParams(browser_context_)));
   content::SessionStorageNamespace* sessionStorageNamespace =
       session_contents_->GetController().GetDefaultSessionStorageNamespace();
   gfx::Size renderWindowSize = session_contents_->GetContainerBounds().size();
   bool accepted = adapter_->StartPrerender(
       browser_context_, url, sessionStorageNamespace, renderWindowSize);
   if (!accepted)
     return false;
 
   DCHECK(adapter_->IsActive());
+  snapshot_controller_->Reset();

[Pete Williamson, 2016/06/09 00:22:51]

Why reset the controller here?  I would guess that this is when we are asking
the prerenderer to load a page, not a callback telling us that the prerenderer
successfully loaded.

[dougarnett, 2016/06/09 00:42:33]

This is to prepare (or arm or clear or Reset()) the state of the controller to
be ready to determine when to snapshot. A subsequent Stop() call on it will
freeze it until it is Reset again.

[Pete Williamson, 2016/06/09 00:49:05]

Ah, this is a bit confusing, since 'reset'-ing a smart pointer is how you clear
it!  At the very least we should comment here, better still  might be to rename
the reset method.

[dougarnett, 2016/06/09 15:12:27]

Added comment. 

I wasn't part of the SnapshotController review so not sure what API naming
discussion might have occurred but dimich@ is reviewer here and so he might
comment on its naming. I agree it is a bit confusing when contained in
unique_ptr. 

   callback_ = callback;
   state_ = State::PENDING;
   return true;
 }
 
 void PrerenderingLoader::StopLoading() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   CancelPrerender();
 }
 
@@ skipping 19 matching lines @@
 }
 
 void PrerenderingLoader::OnPrerenderStart() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(state_ == State::PENDING);
   state_ = State::LOADING;
 }
 
 void PrerenderingLoader::OnPrerenderStopLoading() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // TODO(dougarnett): Implement/integrate to delay policy here.
-  HandleLoadEvent();
+  DCHECK(!IsIdle());
+  if (!IsLoaded()) {
+    DCHECK(adapter_->GetWebContents());
+    snapshot_controller_->DocumentOnLoadCompletedInMainFrame();

[Pete Williamson, 2016/06/09 00:22:51]

(For my education as a reviewer of this code):
What is the difference between OnPrerendererStopLoading() and
OnPrerendererDomContentLoaded()?  Do we get both in some cases?

Also does the snapshot_controller have some kind of built in Start + N seconds
functionality where it takes a snapshot in case we never get the page loaded
event, or are we relying on the prerender stop loading event for our timeout?

It looks like this is set up for single snapshot, right?  Do we anticipate being
able to move to several snapshots someday, or does the implementation of the
prerenderer make that not feasible?

[dougarnett, 2016/06/09 00:42:33]

This code is single shot at the moment however the SnapshotController has
handshaking to support multiple ones. We can decide whether to utilize that here
down the road (let's get single shot working first!).

The SnapshotController currently sets a 7 second delay on the DocumentAvailable
(or DomContentLoaded) event and no delay for the OnLoadCompleted (or
OnPrerenderStopLoading) event. So we expect to get the HandleLoadEvent() call
once for whichever happens first: OnPrerenderStopLoading or
OnPrerenderDomContentLoaded+7secs

[Pete Williamson, 2016/06/09 00:49:05]

OK, code is good as is, then, but let's document this.

[dougarnett, 2016/06/09 15:12:27]

Done - let me know if not clear enough.

+  }
 }
 
 void PrerenderingLoader::OnPrerenderDomContentLoaded() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // TODO(dougarnett): Implement/integrate to delay policy here.
-  HandleLoadEvent();
+  DCHECK(!IsIdle());
+  if (!IsLoaded()) {
+    if (!adapter_->GetWebContents()) {
+      // Without a WebContents object at this point, we are done.
+      HandleLoadingStopped();
+    } else {
+      // Engage the logic to detect when to snapshot.
+      snapshot_controller_->DocumentAvailableInMainFrame();
+    }
+  }
 }
 
 void PrerenderingLoader::OnPrerenderStop() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   HandleLoadingStopped();
 }
 
+void PrerenderingLoader::StartSnapshot() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(!IsIdle());
+  DCHECK(!IsLoaded());
+  HandleLoadEvent();
+}
+
 void PrerenderingLoader::HandleLoadEvent() {
   // If still loading, check if the load succeeded or not, then update
   // the internal state (LOADED for success or IDLE for failure) and post
   // callback.
   // Note: it is possible to receive a load event (e.g., if timeout-based)
   // after the request has completed via another path (e.g., canceled) so
   // the Loader may be idle at this point.
 
   if (IsIdle() || IsLoaded())
     return;
 
   content::WebContents* web_contents = adapter_->GetWebContents();
   if (web_contents) {
+    snapshot_controller_->Stop();
     state_ = State::LOADED;
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(callback_, Offliner::RequestStatus::LOADED, web_contents));
   } else {
     // No WebContents means that the load failed (and it stopped).
     HandleLoadingStopped();
   }
 }
 
@@ skipping 12 matching lines @@
     DVLOG(1) << "Load failed: " << adapter_->GetFinalStatus();
     adapter_->DestroyActive();
   }
   // Request status depends on whether we are still loading (failed) or
   // did load and then loading was stopped (cancel - from prerender stack).
   Offliner::RequestStatus request_status =
       IsLoaded() ? Offliner::RequestStatus::CANCELED
                  : Offliner::RequestStatus::FAILED;
   // TODO(dougarnett): For failure, determine from final status if retry-able
   // and report different failure statuses if retry-able or not.
+  snapshot_controller_->Stop();
   session_contents_.reset(nullptr);
   state_ = State::IDLE;
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(callback_, request_status, nullptr));
 }
 
 void PrerenderingLoader::CancelPrerender() {
   if (adapter_->IsActive()) {
     adapter_->DestroyActive();
   }
+  snapshot_controller_->Stop();
   session_contents_.reset(nullptr);
   if (!IsLoaded() && !IsIdle()) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(callback_, Offliner::RequestStatus::CANCELED, nullptr));
   }
   state_ = State::IDLE;
 }
 
 }  // namespace offline_pages