Convert ContentTranslateDriver to use the new navigation callbacks.

components/translate/content/browser/content_translate_driver.cc

 // Copyright 2014 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 "components/translate/content/browser/content_translate_driver.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_manager.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/page_navigator.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/referrer.h"
 #include "net/http/http_status_code.h"
 #include "url/gurl.h"
 
 namespace {
 
 // The maximum number of attempts we'll do to see if the page has finshed
@@ skipping 176 matching lines @@
   // infobars being removed. Since the translation initiation process might add
   // an infobar, it must be done after that.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::Bind(&ContentTranslateDriver::InitiateTranslation,
                  weak_pointer_factory_.GetWeakPtr(),
                  translate_manager_->GetLanguageState().original_language(),
                  0));
 }
 
-void ContentTranslateDriver::DidNavigateAnyFrame(
-    content::RenderFrameHost* render_frame_host,
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
+void ContentTranslateDriver::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->HasCommitted())

[groby-ooo-7-16, 2017/02/07 18:31:08]

Why do we need to check HasCommitted()? Isn't that implied in DidFinish?

[jam, 2017/02/07 18:35:17]

If the response is a download or 204/205, then the navigation finishes but
doesn't commit (i.e. the previous page is the one that's in the tab)

[jam, 2017/02/07 18:37:05]

btw thanks for reminding me, I wanted to add some comments to NH to write this
out. I'll do that in a followup

+    return;
+
   // Let the LanguageState clear its state.
   const bool reload =
-      ui::PageTransitionCoreTypeIs(details.entry->GetTransitionType(),
-                                   ui::PAGE_TRANSITION_RELOAD) ||
-      details.type == content::NAVIGATION_TYPE_SAME_PAGE;
+      navigation_handle->GetReloadType() != content::ReloadType::NONE ||

[groby-ooo-7-16, 2017/02/07 18:31:08]

I assume ReloadType()!=NONE is the equivalent of
PageTransitionCoreTypeIs()==PAGE_TRANSITION_RELOAD? 

(Sidebar: I sure wish NavigationHandle did just have an IsReload() member...)

[jam, 2017/02/07 18:35:17]

Correct
I hear you (I had to do this in a couple of other places as well and I did find
it a little awkward). The tradeoff is that the class is already pretty large, so
adding redundant helpers could be a slippery slope.

+      navigation_handle->IsSamePage();
   translate_manager_->GetLanguageState().DidNavigate(
-      details.is_in_page, details.is_main_frame, reload);
+      navigation_handle->IsSamePage(), navigation_handle->IsInMainFrame(),

[groby-ooo-7-16, 2017/02/07 18:31:08]

Should this pass through the |navigation_handle| instead?

[jam, 2017/02/07 18:35:17]

That's calling out to shared code with ios, so it doesn't use any content types.

+      reload);
 }
 
 void ContentTranslateDriver::OnPageAway(int page_seq_no) {
   pages_.erase(page_seq_no);
 }
 
 // mojom::ContentTranslateDriver implementation.
 void ContentTranslateDriver::RegisterPage(
     mojom::PagePtr page,
     const LanguageDetectionDetails& details,
@@ skipping 22 matching lines @@
   if (cancelled)
     return;
 
   translate_manager_->PageTranslated(
       original_lang, translated_lang, error_type);
   for (auto& observer : observer_list_)
     observer.OnPageTranslated(original_lang, translated_lang, error_type);
 }
 
 }  // namespace translate