Notify DataUseTabModel of navigations and tab closures

chrome/browser/android/data_usage/data_use_tab_helper.cc

Index: chrome/browser/android/data_usage/data_use_tab_helper.cc
diff --git a/chrome/browser/android/data_usage/data_use_tab_helper.cc b/chrome/browser/android/data_usage/data_use_tab_helper.cc
index 4d1dabf543be59c124f0ca49ada74834c1113c7e..849017a22265fb06c35e1d675f1a99a9d53a68e1 100644
--- a/chrome/browser/android/data_usage/data_use_tab_helper.cc
+++ b/chrome/browser/android/data_usage/data_use_tab_helper.cc
@@ -11,12 +11,9 @@
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
 #include "ui/base/page_transition_types.h"
 
-namespace content {
-class RenderFrameHost;
-}
-
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(DataUseTabHelper);
 
 DataUseTabHelper::~DataUseTabHelper() {}
@@ -26,32 +23,40 @@ DataUseTabHelper::DataUseTabHelper(content::WebContents* web_contents)
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 }
 
-void DataUseTabHelper::ReadyToCommitNavigation(
+void DataUseTabHelper::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  if (!navigation_handle->IsInMainFrame())

[bengr, 2015/11/23 18:26:25]

I suppose this makes sense. Add a TODO to consider the case of a page that
provides a navigation bar and loads pages in a sub-frame.

[tbansal1, 2015/11/23 19:58:08]

Done.

+    return;
 
   // Notify the DataUseUITabModel.
   chrome::android::DataUseUITabModel* data_use_ui_tab_model =
       chrome::android::DataUseUITabModelFactory::GetForBrowserContext(
           Profile::FromBrowserContext(web_contents()->GetBrowserContext()));
-  if (data_use_ui_tab_model) {
+  int32_t tab_id = SessionTabHelper::IdForTab(web_contents());

[bengr, 2015/11/23 18:26:25]

Why not make tab_id a SessionID::id_type and include session_id.h?

[tbansal1, 2015/11/23 19:58:08]

Done.

+  if (data_use_ui_tab_model && tab_id >= 0) {
     data_use_ui_tab_model->ReportBrowserNavigation(
         navigation_handle->GetURL(),
         ui::PageTransitionFromInt(navigation_handle->GetPageTransition()),
-        SessionTabHelper::IdForTab(web_contents()));
+        tab_id);
   }
 }
 
-void DataUseTabHelper::FrameDeleted(
+void DataUseTabHelper::RenderFrameDeleted(
     content::RenderFrameHost* render_frame_host) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
+  // Check if it is a main frame.
+  if (render_frame_host->GetParent() != nullptr ||
+      render_frame_host->IsRenderFrameLive()) {

[bengr, 2015/11/23 18:26:25]

Why do you only proceed if the render frame is not live?

[tbansal1, 2015/11/23 19:58:08]

Done.

+    return;
+  }
+
   // Notify the DataUseUITabModel.
   chrome::android::DataUseUITabModel* data_use_ui_tab_model =
       chrome::android::DataUseUITabModelFactory::GetForBrowserContext(
           Profile::FromBrowserContext(web_contents()->GetBrowserContext()));
-  if (data_use_ui_tab_model) {
-    data_use_ui_tab_model->ReportTabClosure(
-        SessionTabHelper::IdForTab(web_contents()));
-  }
+  int32_t tab_id = SessionTabHelper::IdForTab(web_contents());

[bengr, 2015/11/23 18:26:25]

SessionID::id_type?

[tbansal1, 2015/11/23 19:58:08]

Done.

+  if (data_use_ui_tab_model && tab_id >= 0)
+    data_use_ui_tab_model->ReportTabClosure(tab_id);
 }