Move ContentTranslateDriver reload logic to DidFinishNavigation.

The reload detection in NavigationEntryCommitted depends on stale code
that is being removed, and using a NavigationHandle in
DidFinishNavigation is a more modern approach. This CL updates the
check accordingly so that we can remove NAVIGATION_TYPE_SAME_ENTRY.

Some duplication occurs due to https://crbug.com/1064974.

Bug: 536102, 1064974
Change-Id: I7b908770cfceebc696bff9e900da78c28d1b2f5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628652Reviewed-by: Scott Little <sclittle@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845440}

components/translate/content/browser/content_translate_driver.cc

@@ -180,29 +180,22 @@ void ContentTranslateDriver::OpenUrlInNewTab(const GURL& url) {
   navigation_controller_->GetWebContents()->OpenURL(params);
 }
 
-// content::WebContentsObserver methods
-
-void ContentTranslateDriver::NavigationEntryCommitted(
-    const content::LoadCommittedDetails& load_details) {
+void ContentTranslateDriver::InitiateTranslationIfReload(
+    content::NavigationHandle* navigation_handle) {
   // Check whether this is a reload: When doing a page reload, the
   // TranslateLanguageDetermined IPC is not sent so the translation needs to be
   // explicitly initiated.
 
-  content::NavigationEntry* entry =
-      web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry) {
-    NOTREACHED();
-    return;
-  }
-
   // If the navigation happened while offline don't show the translate
   // bar since there will be nothing to translate.
-  if (load_details.http_status_code == 0 ||
-      load_details.http_status_code == net::HTTP_INTERNAL_SERVER_ERROR) {
+  int response_code =
+      navigation_handle->GetResponseHeaders()
+          ? navigation_handle->GetResponseHeaders()->response_code()
+          : 0;
+  if (response_code == 0 || response_code == net::HTTP_INTERNAL_SERVER_ERROR)
     return;
-  }
 
-  if (!load_details.is_main_frame &&
+  if (!navigation_handle->IsInMainFrame() &&
       translate_manager_->GetLanguageState()->translation_declined()) {
     // Some sites (such as Google map) may trigger sub-frame navigations
     // when the user interacts with the page.  We don't want to show a new
@@ -211,13 +204,11 @@ void ContentTranslateDriver::NavigationEntryCommitted(
   }
 
   // If not a reload, return.
-  if (!ui::PageTransitionCoreTypeIs(entry->GetTransitionType(),
-                                    ui::PAGE_TRANSITION_RELOAD) &&
-      load_details.type != content::NAVIGATION_TYPE_SAME_ENTRY) {
+  if (navigation_handle->GetReloadType() == content::ReloadType::NONE)
     return;
-  }
 
-  if (entry->GetTransitionType() & ui::PAGE_TRANSITION_FORWARD_BACK) {
+  if (navigation_handle->GetPageTransition() &
+      ui::PAGE_TRANSITION_FORWARD_BACK) {
     // Workaround for http://crbug.com/653051: back navigation sometimes have
     // the reload core type. Once http://crbug.com/669008 got resolved, we
     // could revisit here for a thorough solution.
@@ -245,11 +236,14 @@ void ContentTranslateDriver::NavigationEntryCommitted(
           translate_manager_->GetLanguageState()->original_language(), 0));
 }
 
+// content::WebContentsObserver methods
 void ContentTranslateDriver::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->HasCommitted())
     return;
 
+  InitiateTranslationIfReload(navigation_handle);
+
   if (navigation_handle->IsInMainFrame())
     finish_navigation_time_ = base::TimeTicks::Now();
 

components/translate/content/browser/content_translate_driver.h

@@ -95,8 +95,6 @@ class ContentTranslateDriver : public TranslateDriver,
   void OpenUrlInNewTab(const GURL& url) override;
 
   // content::WebContentsObserver implementation.
-  void NavigationEntryCommitted(
-      const content::LoadCommittedDetails& load_details) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
 
@@ -131,6 +129,9 @@ class ContentTranslateDriver : public TranslateDriver,
  private:
   void OnPageAway(int page_seq_no);
 
+  void InitiateTranslationIfReload(
+      content::NavigationHandle* navigation_handle);
+
   // The navigation controller of the tab we are associated with.
   content::NavigationController* navigation_controller_;
 

components/translate/content/browser/per_frame_content_translate_driver.cc

@@ -202,28 +202,22 @@ void PerFrameContentTranslateDriver::RevertFrame(
     frame_agent->RevertTranslation();
 }
 
-// content::WebContentsObserver methods
-void PerFrameContentTranslateDriver::NavigationEntryCommitted(
-    const content::LoadCommittedDetails& load_details) {
+void PerFrameContentTranslateDriver::InitiateTranslationIfReload(
+    content::NavigationHandle* navigation_handle) {
   // Check whether this is a reload: When doing a page reload, the
   // TranslateLanguageDetermined IPC is not sent so the translation needs to be
   // explicitly initiated.
 
-  content::NavigationEntry* entry =
-      web_contents()->GetController().GetLastCommittedEntry();
-  if (!entry) {
-    NOTREACHED();
-    return;
-  }
-
   // If the navigation happened while offline don't show the translate
   // bar since there will be nothing to translate.
-  if (load_details.http_status_code == 0 ||
-      load_details.http_status_code == net::HTTP_INTERNAL_SERVER_ERROR) {
+  int response_code =
+      navigation_handle->GetResponseHeaders()
+          ? navigation_handle->GetResponseHeaders()->response_code()
+          : 0;
+  if (response_code == 0 || response_code == net::HTTP_INTERNAL_SERVER_ERROR)
     return;
-  }
 
-  if (!load_details.is_main_frame &&
+  if (!navigation_handle->IsInMainFrame() &&
       translate_manager()->GetLanguageState()->translation_declined()) {
     // Some sites (such as Google map) may trigger sub-frame navigations
     // when the user interacts with the page.  We don't want to show a new
@@ -232,13 +226,11 @@ void PerFrameContentTranslateDriver::NavigationEntryCommitted(
   }
 
   // If not a reload, return.
-  if (!ui::PageTransitionCoreTypeIs(entry->GetTransitionType(),
-                                    ui::PAGE_TRANSITION_RELOAD) &&
-      load_details.type != content::NAVIGATION_TYPE_SAME_ENTRY) {
+  if (navigation_handle->GetReloadType() == content::ReloadType::NONE)
     return;
-  }
 
-  if (entry->GetTransitionType() & ui::PAGE_TRANSITION_FORWARD_BACK) {
+  if (navigation_handle->GetPageTransition() &
+      ui::PAGE_TRANSITION_FORWARD_BACK) {
     // Workaround for http://crbug.com/653051: back navigation sometimes have
     // the reload core type. Once http://crbug.com/669008 got resolved, we
     // could revisit here for a thorough solution.
@@ -252,8 +244,9 @@ void PerFrameContentTranslateDriver::NavigationEntryCommitted(
 
   if (!translate_manager()
            ->GetLanguageState()
-           ->page_level_translation_critiera_met())
+           ->page_level_translation_critiera_met()) {
     return;
+  }
 
   // Note that we delay it as the ordering of the processing of this callback
   // by WebContentsObservers is undefined and might result in the current
@@ -267,11 +260,14 @@ void PerFrameContentTranslateDriver::NavigationEntryCommitted(
           translate_manager()->GetLanguageState()->original_language(), 0));
 }
 
+// content::WebContentsObserver methods
 void PerFrameContentTranslateDriver::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->HasCommitted())
     return;
 
+  InitiateTranslationIfReload(navigation_handle);
+
   if (navigation_handle->IsInMainFrame())
     finish_navigation_time_ = base::TimeTicks::Now();
 

components/translate/content/browser/per_frame_content_translate_driver.h

@@ -52,8 +52,6 @@ class PerFrameContentTranslateDriver : public ContentTranslateDriver {
   void RevertTranslation(int page_seq_no) override;
 
   // content::WebContentsObserver implementation.
-  void NavigationEntryCommitted(
-      const content::LoadCommittedDetails& load_details) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
   void DOMContentLoaded(content::RenderFrameHost* render_frame_host) override;
@@ -84,6 +82,9 @@ class PerFrameContentTranslateDriver : public ContentTranslateDriver {
 
   void StartLanguageDetection();
 
+  void InitiateTranslationIfReload(
+      content::NavigationHandle* navigation_handle);
+
   void TranslateFrame(const std::string& translate_script,
                       const std::string& source_lang,
                       const std::string& target_lang,