PlzNavigate: add cancel navigation logic for uncommitted requests

content/browser/frame_host/navigation_request.cc

Index: content/browser/frame_host/navigation_request.cc
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
index fb46ae59b3d9c10ad87d021dd60dd5010efb342f..635ea1df51f753df892550974fb2702bd253b1b9 100644
--- a/content/browser/frame_host/navigation_request.cc
+++ b/content/browser/frame_host/navigation_request.cc
@@ -13,25 +13,52 @@ namespace content {
 
 namespace {
 
+// The next available browser-global navigation request ID.
+int64 next_navigation_request_id_ = 0;

[(Do not use) nasko, 2014/08/28 15:55:05]

nit: make this one static

[carlosk, 2014/08/29 15:54:43]

I added the static.

But from what I read pre-pending static to a global variable makes it
inaccessible from other compilation units. But doesn't declaring it inside an
anonymous namespace achieves the same goal?

Also, is there any styleguide rule or just practical rule against me just
defining this as a static variable inside the scope of the function that sets
new IDs? Ex:

void foo() {
  (...)
  static int new_id_source = 0;
  my_id = ++new_id_source;
  (...)
}

[nasko, 2014/09/02 14:10:56]

This is a good point. Use whichever you prefer.
 
I don't know of a style rule about that. 

[carlosk, 2014/09/02 15:30:09]

Acknowledged.

+
 void OnBeginNavigation(const NavigationRequestInfo& info,
                        scoped_refptr<ResourceRequestBody> request_body,
+                       int64 navigation_request_id,
                        int64 frame_node_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  ResourceDispatcherHostImpl::Get()->NavigationRequest(
-      info, request_body, frame_node_id);
+  ResourceDispatcherHostImpl::Get()->StartNavigationRequest(
+      info, request_body, navigation_request_id, frame_node_id);
+}
+
+void CancelNavigationRequest(int64 navigation_request_id,
+                             int64 frame_node_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  ResourceDispatcherHostImpl::Get()->CancelNavigationRequest(
+      navigation_request_id, frame_node_id);
 }
 
 }  // namespace
 
+NavigationRequest::Observer::Observer() : navigation_request_(NULL) {}
+
+NavigationRequest::Observer::Observer(
+    NavigationRequest* nv) : navigation_request_(nv) {
+  navigation_request_->SetSingleObserver(this);
+}
+
+NavigationRequest::Observer::~Observer() {
+  if (navigation_request_)
+    navigation_request_->SetSingleObserver(NULL);

[clamy, 2014/08/28 12:17:01]

This should be replaced by a call to RemoveObserver.

+}
+
 NavigationRequest::NavigationRequest(const NavigationRequestInfo& info,
                                      int64 frame_node_id)
-  : info_(info),
-    frame_node_id_(frame_node_id) {
+  : navigation_request_id_(++next_navigation_request_id_),
+    info_(info),
+    frame_node_id_(frame_node_id),
+    single_observer_(NULL) {
 }
 
 NavigationRequest::~NavigationRequest() {
-  // TODO(clamy): Cancel the corresponding request in ResourceDispatcherHost if
-  // it has not commited yet.
+  if (single_observer_) {
+    single_observer_->RequestDestroyed(this);
+    single_observer_->set_navigation_request(NULL);

[clamy, 2014/08/28 12:17:01]

nit: it may actually be unnecessary to set the the navigation request to NULL
here. However, in that case, the implementation of RequestDestroyed should set
it to null.

[carlosk, 2014/08/29 15:54:43]

Agreed it should be moved out of here. It is necessary to do it though because
if the observer is destroyed after the request it will try to un-register itself
using that pointer (it it's not set to NULL).

+  }
 }
 
 void NavigationRequest::BeginNavigation(
@@ -40,7 +67,32 @@ void NavigationRequest::BeginNavigation(
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&OnBeginNavigation, info_, request_body, frame_node_id_));
+      base::Bind(&OnBeginNavigation,
+                 info_,
+                 request_body,
+                 navigation_request_id_,
+                 frame_node_id_));
+}
+
+void NavigationRequest::CancelNavigation() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&CancelNavigationRequest,
+                 navigation_request_id_, frame_node_id_));
+  if (single_observer_ != NULL)
+    single_observer_->RequestCanceled(this);
+}
+
+void NavigationRequest::SetSingleObserver(
+    NavigationRequest::Observer* observer) {
+  DCHECK((single_observer_ != NULL && observer == NULL)
+     || (single_observer_ == NULL && observer != NULL));
+  if (single_observer_) {
+    single_observer_->set_navigation_request(NULL);
+  }
+  single_observer_ = observer;
 }
 
 }  // namespace content