Convert StartPageService and ZoomController to use the new navigation callbacks.

chrome/browser/ui/app_list/start_page_service.cc

 // Copyright 2013 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 "chrome/browser/ui/app_list/start_page_service.h"
 
 #include <stddef.h>
 
 #include <string>
 
@@ skipping 22 matching lines @@
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "components/prefs/pref_service.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/zoom/zoom_controller.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/speech_recognition_session_preamble.h"
 #include "content/public/browser/web_contents.h"
@@ skipping 497 matching lines @@
 void StartPageService::Shutdown() {
   UnloadContents();
 #if defined(OS_CHROMEOS)
   audio_status_.reset();
 #endif
 
   speech_auth_helper_.reset();
   network_change_observer_.reset();
 }
 
-void StartPageService::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& /*details*/,
-    const content::FrameNavigateParams& /*params*/) {
-  // Set the zoom level in DidNavigateMainFrame, as this is the earliest point
+void StartPageService::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() || !navigation_handle->HasCommitted())

[Dan Beam, 2017/01/27 20:33:21]

should this !IsInMainFrame() be after the error check if DidFailProvisionalLoad
applied to all frames, not just the main one?

[jam, 2017/01/27 21:28:57]

per the other comment, these two methods realy only deal with the main frame.

+    return;
+
+  if (navigation_handle->IsErrorPage()) {
+    // This avoids displaying a "Webpage Blocked" error or similar (which can
+    // happen if the URL is blacklisted by enterprise policy).
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::Bind(&StartPageService::UnloadContents,
+                   weak_factory_.GetWeakPtr()));
+    return;
+  }
+
+  // Set the zoom level in DidFinishNavigation, as this is the earliest point
   // at which it can be done and not be affected by the ZoomController's
-  // DidNavigateMainFrame handler.
+  // DidFinishNavigation handler.
   //
   // Use a temporary zoom level for this web contents (aka isolated zoom
   // mode) so changes to its zoom aren't reflected in any preferences.
   zoom::ZoomController::FromWebContents(contents_.get())
       ->SetZoomMode(zoom::ZoomController::ZOOM_MODE_ISOLATED);
   // Set to have a zoom level of 0, which corresponds to 100%, so the
   // contents aren't affected by the browser's default zoom level.
   zoom::ZoomController::FromWebContents(contents_.get())->SetZoomLevel(0);
 }
 
-void StartPageService::DidFailProvisionalLoad(

[Dan Beam, 2017/01/27 20:33:21]

did this only apply to the main page?  did this get called without a commit?

[jam, 2017/01/27 21:28:57]

The chrome apps page has no frames, so the IsInMainFrame() doesn't matter.
In the old api, a navigation is provisional until it either commits or fails.
I'm not sure what your question means?

-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    int error_code,
-    const base::string16& error_description,
-    bool was_ignored_by_handler) {
-  // This avoids displaying a "Webpage Blocked" error or similar (which can
-  // happen if the URL is blacklisted by enterprise policy).
-  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-                                   base::Bind(&StartPageService::UnloadContents,
-                                              weak_factory_.GetWeakPtr()));
-}
-
 void StartPageService::WebUILoaded() {
   // There's a race condition between the WebUI loading, and calling its JS
   // functions. Specifically, calling LoadContents() doesn't mean that the page
   // has loaded, but several code paths make this assumption. This function
   // allows us to defer calling JS functions until after the page has finished
   // loading.
   webui_finished_loading_ = true;
   for (const auto& cb : pending_webui_callbacks_)
     cb.Run();
   pending_webui_callbacks_.clear();
@@ skipping 85 matching lines @@
 
   // Check for a new doodle.
   content::BrowserThread::PostDelayedTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&StartPageService::FetchDoodleJson,
                  weak_factory_.GetWeakPtr()),
       recheck_delay);
 }
 
 }  // namespace app_list