Tied up BlimpNavigationController to NavigationFeature

blimp/client/core/contents/blimp_navigation_controller_impl.cc

Index: blimp/client/core/contents/blimp_navigation_controller_impl.cc
diff --git a/blimp/client/core/contents/blimp_navigation_controller_impl.cc b/blimp/client/core/contents/blimp_navigation_controller_impl.cc
index 23f0857ba1040c152fe8e3b4233c56d7ce6ee994..f4b06c48ef1fefbaf56caa5e976e73514d4bdaab 100644
--- a/blimp/client/core/contents/blimp_navigation_controller_impl.cc
+++ b/blimp/client/core/contents/blimp_navigation_controller_impl.cc
@@ -10,33 +10,83 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "blimp/client/core/contents/blimp_navigation_controller_delegate.h"
 
+namespace {
+// TODO(shaktisahu): NavigationFeature currently needs a tab_id. Remove this
+// later when it is fully integrated with BlimpClientContext.
+const int kDummyTabId = 0;

[Kevin M, 2016/07/29 21:05:32]

We should use a shared constant instead of redefining this for every feature.

[shaktisahu, 2016/07/29 22:57:30]

Acknowledged.

+}  // namespace
+
 namespace blimp {
 namespace client {
 
 BlimpNavigationControllerImpl::BlimpNavigationControllerImpl(
-    BlimpNavigationControllerDelegate* delegate)
-    : delegate_(delegate), weak_ptr_factory_(this) {}
+    BlimpNavigationControllerDelegate* delegate,
+    NavigationFeature* feature)
+    : navigation_feature_(feature), delegate_(delegate) {
+  if (navigation_feature_)

[Kevin M, 2016/07/29 21:05:32]

Why would this be non-null?

If there's no reason, then DCHECK the ptr.

[shaktisahu, 2016/07/29 22:57:30]

Yea, I should DCHECK. But currently the hookup is not complete till
BlimpClientSession integration and I am passing null for |feature|.

+    navigation_feature_->SetDelegate(kDummyTabId, this);
+}
 
 BlimpNavigationControllerImpl::~BlimpNavigationControllerImpl() = default;
 
+void BlimpNavigationControllerImpl::SetNavigationFeatureForTesting(
+    NavigationFeature* feature) {
+  navigation_feature_ = feature;
+  navigation_feature_->SetDelegate(kDummyTabId, this);
+}
+
 void BlimpNavigationControllerImpl::LoadURL(const GURL& url) {
   current_url_ = url;
-  // Temporary trick to ensure that the delegate is not invoked before this
-  // method has finished executing. This enables tests to test the
-  // asynchronous nature of the API.
-  // TODO(shaktisahu): Remove this after integration with NavigationFeature.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(&BlimpNavigationControllerImpl::NotifyDelegateURLLoaded,
-                 weak_ptr_factory_.GetWeakPtr(), url));
+  navigation_feature_->NavigateToUrlText(kDummyTabId, current_url_.spec());
+}
+
+void BlimpNavigationControllerImpl::Reload() {
+  navigation_feature_->Reload(kDummyTabId);
+}
+
+bool BlimpNavigationControllerImpl::CanGoBack() const {
+  // TODO(shaktisahu): Implement the client-side logic.

[Kevin M, 2016/07/29 21:05:32]

Replace these TODOs with NOTIMPLEMENTED()

[shaktisahu, 2016/07/29 22:57:30]

Done.

+  return false;
+}
+
+bool BlimpNavigationControllerImpl::CanGoForward() const {
+  // TODO(shaktisahu): Implement the client-side logic.
+  return false;
+}
+
+void BlimpNavigationControllerImpl::GoBack() {
+  navigation_feature_->GoBack(kDummyTabId);
+}
+
+void BlimpNavigationControllerImpl::GoForward() {
+  navigation_feature_->GoForward(kDummyTabId);
 }
 
 const GURL& BlimpNavigationControllerImpl::GetURL() {
   return current_url_;
 }
 
-void BlimpNavigationControllerImpl::NotifyDelegateURLLoaded(const GURL& url) {
-  delegate_->NotifyURLLoaded(url);
+void BlimpNavigationControllerImpl::OnUrlChanged(int tab_id, const GURL& url) {
+  delegate_->OnNavigationStateChanged();
+}
+
+void BlimpNavigationControllerImpl::OnFaviconChanged(int tab_id,
+                                                     const SkBitmap& favicon) {
+  delegate_->OnNavigationStateChanged();
+}
+
+void BlimpNavigationControllerImpl::OnTitleChanged(int tab_id,
+                                                   const std::string& title) {
+  delegate_->OnNavigationStateChanged();
+}
+
+void BlimpNavigationControllerImpl::OnLoadingChanged(int tab_id, bool loading) {
+  delegate_->OnNavigationStateChanged();
+}
+
+void BlimpNavigationControllerImpl::OnPageLoadStatusUpdate(int tab_id,
+                                                           bool completed) {
+  delegate_->OnNavigationStateChanged();
 }
 
 }  // namespace client