Convert ExtensionLoaderHandler to use the new navigation callbacks.

chrome/browser/ui/webui/extensions/extension_loader_handler.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 "chrome/browser/ui/webui/extensions/extension_loader_handler.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/path_util.h"
 #include "chrome/browser/extensions/unpacked_installer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/file_highlighter.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_constants.h"
 #include "third_party/re2/src/re2/re2.h"
 #include "ui/base/l10n/l10n_util.h"
@@ skipping 130 matching lines @@
       content::BrowserThread::GetBlockingPool(),
       FROM_HERE,
       base::Bind(&ReadFileToString, file_path.Append(kManifestFilename)),
       base::Bind(&ExtensionLoaderHandler::AddFailure,
                  weak_ptr_factory_.GetWeakPtr(),
                  file_path,
                  error,
                  line));
 }
 
-void ExtensionLoaderHandler::DidStartNavigationToPendingEntry(
-    const GURL& url,
-    content::ReloadType reload_type) {
+void ExtensionLoaderHandler::ReadyToCommitNavigation(

[Devlin, 2017/01/21 02:01:17]

The cheatsheet linked in the bug and the comments in WebContentsObserver seem to
indicate that DidStartNavigationToPendingEntry is replaced by DidStartNavigation
- why's this differ?

[jam, 2017/01/21 05:30:56]

Generally that's the replacement. In this case, the method needs the transition
type. That's not available in DidStartNavigation. that could be done, but only
with PlzNavigate, so I filed 683402 to track making this getter available early
once PlzNavigate is the only code path. So in the meantime, this is as early as
possible to get the transition type.

Note I've manually tested this cl and the other one, with and without
PlzNavigate, to make sure the user behavior stays the same.

[jam, 2017/01/21 07:17:54]

actually, turns out that my change for ContentFaviconDriver necessitated
plumbing more reload information to NavigationHandle which is available at
construction.

hold off reviewing this, when that other cl lands I can change this and the
other extensions cl to use DidStartNavigation.

+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() || navigation_handle->IsSamePage())

[Devlin, 2017/01/21 02:01:17]

Why do we need the IsInMainFrame() check?  And won't IsSamePage() always be
false for PAGE_TRANSITION_RELOAD? (which, separately, is a little confusing,
since it is the same page, but that's a different discussion...)

[jam, 2017/01/21 05:30:56]

To keep the same behavior as before, since DidStartNavigationToPendingEntry
fired only for main frames while DidStartNavigation also fires for subframes.
Yep that's true; I had kept it in to keep the same behavior as before but you're
right it's redundant. Removed.

+    return;
+
   // In the event of a page reload, we ensure that the frontend is not notified
   // until the UI finishes loading, so we set |ui_ready_| to false. This is
   // balanced in HandleDisplayFailures, which is called when the frontend is
   // ready to receive failure notifications.
-  if (reload_type != content::ReloadType::NONE)
+  bool is_reload = ui::PageTransitionCoreTypeIs(
+      navigation_handle->GetPageTransition(), ui::PAGE_TRANSITION_RELOAD);
+  if (is_reload)
     ui_ready_ = false;
 }
 
 void ExtensionLoaderHandler::AddFailure(
     const base::FilePath& file_path,
     const std::string& error,
     size_t line_number,
     const std::string& manifest) {
   failed_paths_.push_back(file_path);
   base::FilePath prettified_path = path_util::PrettifyPath(file_path);
@@ skipping 17 matching lines @@
     NotifyFrontendOfFailure();
 }
 
 void ExtensionLoaderHandler::NotifyFrontendOfFailure() {
   web_ui()->CallJavascriptFunctionUnsafe(
       "extensions.ExtensionLoader.notifyLoadFailed", failures_);
   failures_.Clear();
 }
 
 }  // namespace extensions