Navigation: move RestoreType and ReloadType into a separate file

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 2b21fd9296ce3828ee0b2417a9174fc788e022e6..d43de0405319b8dec45d27e93b263c0c5cca2d94 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -115,30 +115,16 @@ void SetPageStateIfEmpty(NavigationEntryImpl* entry) {
     entry->SetPageState(PageState::CreateFromURL(entry->GetURL()));
 }
 
-NavigationEntryImpl::RestoreType ControllerRestoreTypeToEntryType(

[Charlie Reis, 2016/08/11 23:58:43]

I admit this does seem a bit unnecessary.  I think I'm ok changing RestoreType
in this CL as as well, even though it's unrelated.

-    NavigationController::RestoreType type) {
-  switch (type) {
-    case NavigationController::RESTORE_CURRENT_SESSION:
-      return NavigationEntryImpl::RESTORE_CURRENT_SESSION;
-    case NavigationController::RESTORE_LAST_SESSION_EXITED_CLEANLY:
-      return NavigationEntryImpl::RESTORE_LAST_SESSION_EXITED_CLEANLY;
-    case NavigationController::RESTORE_LAST_SESSION_CRASHED:
-      return NavigationEntryImpl::RESTORE_LAST_SESSION_CRASHED;
-  }
-  NOTREACHED();
-  return NavigationEntryImpl::RESTORE_CURRENT_SESSION;
-}
-
 // Configure all the NavigationEntries in entries for restore. This resets
 // the transition type to reload and makes sure the content state isn't empty.
 void ConfigureEntriesForRestore(
     std::vector<std::unique_ptr<NavigationEntryImpl>>* entries,
-    NavigationController::RestoreType type) {
+    RestoreType type) {
   for (size_t i = 0; i < entries->size(); ++i) {
     // Use a transition type of reload so that we don't incorrectly increase
     // the typed count.
     (*entries)[i]->SetTransitionType(ui::PAGE_TRANSITION_RELOAD);
-    (*entries)[i]->set_restore_type(ControllerRestoreTypeToEntryType(type));
+    (*entries)[i]->set_restore_type(type);
     // NOTE(darin): This code is only needed for backwards compat.
     SetPageStateIfEmpty((*entries)[i].get());
   }
@@ -239,7 +225,7 @@ NavigationControllerImpl::NavigationControllerImpl(
       needs_reload_(false),
       is_initial_navigation_(true),
       in_navigate_to_pending_entry_(false),
-      pending_reload_(NO_RELOAD),
+      pending_reload_(ReloadType::NONE),
       get_timestamp_callback_(base::Bind(&base::Time::Now)),
       screenshot_manager_(new NavigationEntryScreenshotManager(this)) {
   DCHECK(browser_context_);
@@ -281,29 +267,29 @@ void NavigationControllerImpl::Restore(
 }
 
 void NavigationControllerImpl::Reload(bool check_for_repost) {
-  ReloadType type = RELOAD;
+  ReloadType type = ReloadType::NORMAL;
   if (base::FeatureList::IsEnabled(
         features::kNonValidatingReloadOnNormalReload)) {
-    type = RELOAD_MAIN_RESOURCE;
+    type = ReloadType::MAIN_RESOURCE;
   }
   ReloadInternal(check_for_repost, type);
 }
 void NavigationControllerImpl::ReloadToRefreshContent(bool check_for_repost) {
-  ReloadType type = RELOAD;
+  ReloadType type = ReloadType::NORMAL;
   if (base::FeatureList::IsEnabled(
         features::kNonValidatingReloadOnRefreshContent)) {
-    type = RELOAD_MAIN_RESOURCE;
+    type = ReloadType::MAIN_RESOURCE;
   }
   ReloadInternal(check_for_repost, type);
 }
 void NavigationControllerImpl::ReloadBypassingCache(bool check_for_repost) {
-  ReloadInternal(check_for_repost, RELOAD_BYPASSING_CACHE);
+  ReloadInternal(check_for_repost, ReloadType::BYPASSING_CACHE);
 }
 void NavigationControllerImpl::ReloadOriginalRequestURL(bool check_for_repost) {
-  ReloadInternal(check_for_repost, RELOAD_ORIGINAL_REQUEST_URL);
+  ReloadInternal(check_for_repost, ReloadType::ORIGINAL_REQUEST_URL);
 }
 void NavigationControllerImpl::ReloadDisableLoFi(bool check_for_repost) {
-  ReloadInternal(check_for_repost, RELOAD_DISABLE_LOFI_MODE);
+  ReloadInternal(check_for_repost, ReloadType::DISABLE_LOFI_MODE);
 }
 
 void NavigationControllerImpl::ReloadInternal(bool check_for_repost,
@@ -379,7 +365,7 @@ void NavigationControllerImpl::ReloadInternal(bool check_for_repost,
 
       // Mark the reload type as NO_RELOAD, so navigation will not be considered
       // a reload in the renderer.
-      reload_type = NavigationController::NO_RELOAD;
+      reload_type = ReloadType::NONE;
 
       nav_entry->set_should_replace_entry(true);
       pending_entry_ = nav_entry;
@@ -402,16 +388,16 @@ void NavigationControllerImpl::ReloadInternal(bool check_for_repost,
 }
 
 void NavigationControllerImpl::CancelPendingReload() {
-  DCHECK(pending_reload_ != NO_RELOAD);
-  pending_reload_ = NO_RELOAD;
+  DCHECK(pending_reload_ != ReloadType::NONE);
+  pending_reload_ = ReloadType::NONE;
 }
 
 void NavigationControllerImpl::ContinuePendingReload() {
-  if (pending_reload_ == NO_RELOAD) {
+  if (pending_reload_ == ReloadType::NONE) {
     NOTREACHED();
   } else {
     ReloadInternal(false, pending_reload_);
-    pending_reload_ = NO_RELOAD;
+    pending_reload_ = ReloadType::NONE;
   }
 }
 
@@ -421,8 +407,8 @@ bool NavigationControllerImpl::IsInitialNavigation() const {
 
 bool NavigationControllerImpl::IsInitialBlankNavigation() const {
   // TODO(creis): Once we create a NavigationEntry for the initial blank page,
-  // we'll need to check for entry count 1 and restore_type RESTORE_NONE (to
-  // exclude the cloned tab case).
+  // we'll need to check for entry count 1 and restore_type NONE (to exclude
+  // the cloned tab case).
   return IsInitialNavigation() && GetEntryCount() == 0;
 }
 
@@ -445,7 +431,7 @@ void NavigationControllerImpl::LoadEntry(
   // end up leaving the current page.  The new page load could for example
   // result in a download or a 'no content' response (e.g., a mailto: URL).
   SetPendingEntry(std::move(entry));
-  NavigateToPendingEntry(NO_RELOAD);
+  NavigateToPendingEntry(ReloadType::NONE);
 }
 
 void NavigationControllerImpl::SetPendingEntry(
@@ -608,7 +594,7 @@ void NavigationControllerImpl::GoToIndex(int index) {
       ui::PageTransitionFromInt(
           entries_[pending_entry_index_]->GetTransitionType() |
           ui::PAGE_TRANSITION_FORWARD_BACK));
-  NavigateToPendingEntry(NO_RELOAD);
+  NavigateToPendingEntry(ReloadType::NONE);
 }
 
 void NavigationControllerImpl::GoToOffset(int offset) {
@@ -801,12 +787,11 @@ bool NavigationControllerImpl::RendererDidNavigate(
 
   // If there is a pending entry at this point, it should have a SiteInstance,
   // except for restored entries.
-  DCHECK(pending_entry_index_ == -1 ||
-         pending_entry_->site_instance() ||
-         pending_entry_->restore_type() != NavigationEntryImpl::RESTORE_NONE);
-  if (pending_entry_ &&
-      pending_entry_->restore_type() != NavigationEntryImpl::RESTORE_NONE)
-    pending_entry_->set_restore_type(NavigationEntryImpl::RESTORE_NONE);
+  DCHECK(pending_entry_index_ == -1 || pending_entry_->site_instance() ||
+         pending_entry_->restore_type() != RestoreType::NONE);
+  if (pending_entry_ && pending_entry_->restore_type() != RestoreType::NONE) {
+    pending_entry_->set_restore_type(RestoreType::NONE);
+  }
 
   // The renderer tells us whether the navigation replaces the current entry.
   details->did_replace_entry = params.should_replace_current_entry;
@@ -819,26 +804,26 @@ bool NavigationControllerImpl::RendererDidNavigate(
                                               params.was_within_same_page, rfh);
 
   switch (details->type) {
-    case NAVIGATION_TYPE_NEW_PAGE:
+    case NavigationType::NEW_PAGE:

[Charlie Reis, 2016/08/11 23:58:43]

What's the motivation for changing this one?  I'm not sure why making it an enum
class is an improvement, and it seems unnecessary for the Reload work.  I'd
prefer to avoid unnecessary churn on this unless there's a reason for it.

[Takashi Toyoshima, 2016/09/07 12:07:44]

My understanding is "enum class" is encouraged to be used in new code, and is
better to replace existing "enum" when we modify.

But if we need to have a separate header file for each enum, this won't be
touched in this CL anyway.

       RendererDidNavigateToNewPage(rfh, params, details->is_in_page,
                                    details->did_replace_entry);
       break;
-    case NAVIGATION_TYPE_EXISTING_PAGE:
+    case NavigationType::EXISTING_PAGE:
       details->did_replace_entry = details->is_in_page;
       RendererDidNavigateToExistingPage(rfh, params);
       break;
-    case NAVIGATION_TYPE_SAME_PAGE:
+    case NavigationType::SAME_PAGE:
       RendererDidNavigateToSamePage(rfh, params);
       break;
-    case NAVIGATION_TYPE_NEW_SUBFRAME:
+    case NavigationType::NEW_SUBFRAME:
       RendererDidNavigateNewSubframe(rfh, params, details->is_in_page,
                                      details->did_replace_entry);
       break;
-    case NAVIGATION_TYPE_AUTO_SUBFRAME:
+    case NavigationType::AUTO_SUBFRAME:
       if (!RendererDidNavigateAutoSubframe(rfh, params))
         return false;
       break;
-    case NAVIGATION_TYPE_NAV_IGNORE:
+    case NavigationType::NAV_IGNORE:
       // If a pending navigation was in progress, this canceled it.  We should
       // discard it and make sure it is removed from the URL bar.  After that,
       // there is nothing we can do with this navigation, so we just return to
@@ -946,7 +931,7 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // A new entry. We may or may not have a pending entry for the page, and
     // this may or may not be the main frame.
     if (!rfh->GetParent()) {
-      return NAVIGATION_TYPE_NEW_PAGE;
+      return NavigationType::NEW_PAGE;
     }
 
     // When this is a new subframe navigation, we should have a committed page
@@ -955,10 +940,10 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // written into the popup by script on the main page). For these cases,
     // there isn't any navigation stuff we can do, so just ignore it.
     if (!GetLastCommittedEntry())
-      return NAVIGATION_TYPE_NAV_IGNORE;
+      return NavigationType::NAV_IGNORE;
 
     // Valid subframe navigation.
-    return NAVIGATION_TYPE_NEW_SUBFRAME;
+    return NavigationType::NEW_SUBFRAME;
   }
 
   // Cross-process location.replace navigations should be classified as New with
@@ -970,7 +955,7 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
   if (!rfh->GetParent() && GetLastCommittedEntry() &&
       GetLastCommittedEntry()->site_instance() != rfh->GetSiteInstance() &&
       params.should_replace_current_entry) {
-    return NAVIGATION_TYPE_NEW_PAGE;
+    return NavigationType::NEW_PAGE;
   }
 
   // We only clear the session history when navigating to a new page.
@@ -980,11 +965,11 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // All manual subframes would be did_create_new_entry and handled above, so
     // we know this is auto.
     if (GetLastCommittedEntry()) {
-      return NAVIGATION_TYPE_AUTO_SUBFRAME;
+      return NavigationType::AUTO_SUBFRAME;
     } else {
       // We ignore subframes created in non-committed pages; we'd appreciate if
       // people stopped doing that.
-      return NAVIGATION_TYPE_NAV_IGNORE;
+      return NavigationType::NAV_IGNORE;
     }
   }
 
@@ -997,11 +982,11 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // stuff that we can do, so ignore it here as well.
     NavigationEntry* last_committed = GetLastCommittedEntry();
     if (!last_committed)
-      return NAVIGATION_TYPE_NAV_IGNORE;
+      return NavigationType::NAV_IGNORE;
 
     // This is history.replaceState(), history.reload(), or a client-side
     // redirect.
-    return NAVIGATION_TYPE_EXISTING_PAGE;
+    return NavigationType::EXISTING_PAGE;
   }
 
   if (pending_entry_ && pending_entry_index_ == -1 &&
@@ -1012,7 +997,7 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // thus set params.did_create_new_entry to false. In that case, we must
     // treat it as NEW since the SiteInstance doesn't match the entry.
     if (GetLastCommittedEntry()->site_instance() != rfh->GetSiteInstance())
-      return NAVIGATION_TYPE_NEW_PAGE;
+      return NavigationType::NEW_PAGE;
 
     // Otherwise, this happens when you press enter in the URL bar to reload. We
     // will create a pending entry, but Blink will convert it to a reload since
@@ -1021,14 +1006,14 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // want to just ignore the pending entry and go back to where we were (the
     // "existing entry").
     // TODO(creis,avi): Eliminate SAME_PAGE in https://crbug.com/536102.
-    return NAVIGATION_TYPE_SAME_PAGE;
+    return NavigationType::SAME_PAGE;
   }
 
   if (params.intended_as_new_entry) {
     // This was intended to be a navigation to a new entry but the pending entry
     // got cleared in the meanwhile. Classify as EXISTING_PAGE because we may or
     // may not have a pending entry.
-    return NAVIGATION_TYPE_EXISTING_PAGE;
+    return NavigationType::EXISTING_PAGE;
   }
 
   if (params.url_is_unreachable && failed_pending_entry_id_ != 0 &&
@@ -1037,7 +1022,7 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     // of an error, this is the case of the user trying to retry a failed load
     // by pressing return. Classify as EXISTING_PAGE because we probably don't
     // have a pending entry.
-    return NAVIGATION_TYPE_EXISTING_PAGE;
+    return NavigationType::EXISTING_PAGE;
   }
 
   // Now we know that the notification is for an existing page. Find that entry.
@@ -1045,12 +1030,12 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
   if (existing_entry_index == -1) {
     // The renderer has committed a navigation to an entry that no longer
     // exists. Because the renderer is showing that page, resurrect that entry.
-    return NAVIGATION_TYPE_NEW_PAGE;
+    return NavigationType::NEW_PAGE;
   }
 
   // Since we weeded out "new" navigations above, we know this is an existing
   // (back/forward) navigation.
-  return NAVIGATION_TYPE_EXISTING_PAGE;
+  return NavigationType::EXISTING_PAGE;
 }
 
 void NavigationControllerImpl::RendererDidNavigateToNewPage(
@@ -1475,7 +1460,8 @@ void NavigationControllerImpl::CopyStateFrom(
     session_storage_namespace_map_[it->first] = source_namespace->Clone();
   }
 
-  FinishRestore(source.last_committed_entry_index_, RESTORE_CURRENT_SESSION);
+  FinishRestore(source.last_committed_entry_index_,
+                RestoreType::CURRENT_SESSION);
 
   // Copy the max page id map from the old tab to the new tab.  This ensures
   // that new and existing navigations in the tab's current SiteInstances
@@ -1780,10 +1766,9 @@ void NavigationControllerImpl::NavigateToPendingEntry(ReloadType reload_type) {
   // page from loading (which would normally happen during the navigation).
   if (pending_entry_index_ != -1 &&
       pending_entry_index_ == last_committed_entry_index_ &&
-      (entries_[pending_entry_index_]->restore_type() ==
-          NavigationEntryImpl::RESTORE_NONE) &&
+      (entries_[pending_entry_index_]->restore_type() == RestoreType::NONE) &&
       (entries_[pending_entry_index_]->GetTransitionType() &
-          ui::PAGE_TRANSITION_FORWARD_BACK)) {
+       ui::PAGE_TRANSITION_FORWARD_BACK)) {
     delegate_->Stop();
 
     // If an interstitial page is showing, we want to close it to get back
@@ -1984,10 +1969,10 @@ void NavigationControllerImpl::LoadIfNecessary() {
   // Explicitly use NavigateToPendingEntry so that the renderer uses the
   // cached state.
   if (pending_entry_) {
-    NavigateToPendingEntry(NO_RELOAD);
+    NavigateToPendingEntry(ReloadType::NONE);
   } else if (last_committed_entry_index_ != -1) {
     pending_entry_index_ = last_committed_entry_index_;
-    NavigateToPendingEntry(NO_RELOAD);
+    NavigateToPendingEntry(ReloadType::NONE);
   } else {
     // If there is something to reload, the successful reload will clear the
     // |needs_reload_| flag. Otherwise, just do it here.