NavigationController: merge Reload*() to one Reload() with ReloadType

content/public/browser/navigation_controller.h

Index: content/public/browser/navigation_controller.h
diff --git a/content/public/browser/navigation_controller.h b/content/public/browser/navigation_controller.h
index 3d3975235d74b2d187ef0fb4baa83b1dadfe208d..b35b1c90a0d09b67ee1f03b35e988c62110347a1 100644
--- a/content/public/browser/navigation_controller.h
+++ b/content/public/browser/navigation_controller.h
@@ -17,6 +17,7 @@
 #include "build/build_config.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_request_id.h"
+#include "content/public/browser/reload_type.h"
 #include "content/public/browser/restore_type.h"
 #include "content/public/browser/session_storage_namespace.h"
 #include "content/public/browser/site_instance.h"
@@ -349,6 +350,14 @@ class NavigationController {
   // the offset is out of bounds.
   virtual void GoToOffset(int offset) = 0;
 
+  // Reloads the current entry under the speicified ReloadType. This will

[Charlie Reis, 2016/12/05 19:34:00]

nit: specified
nit: We don't need the second sentence, since the TODO covers that.

[Takashi Toyoshima, 2016/12/06 08:10:42]

Done.

+  // replace Reload* methods below.
+  // TODO(toyoshim): Change all callers to use this new Reload(), and remove
+  // old Reload* methods below. One motivation of this change is Reload(bool)
+  // interface is just confusing because in some contexts, the bool could be to
+  // specify bypassing cache. http://crbug.com/670232
+  virtual void Reload(bool check_for_repost, ReloadType reload_type) = 0;

[Charlie Reis, 2016/12/05 19:34:00]

I'm wondering if it would be worth reversing the order of the parameters, since
|reload_type| seems like the more fundamental one for affecting behavior.  (On
the flip side, I see that ReloadInternal is already set up the other way, so
this is a smaller delta.)

I'll leave it up to you, for which one you think is clearer to callers.

[Takashi Toyoshima, 2016/12/06 08:10:42]

One pros of current order is that we can set a default type on finalizing this
cleanup.

It would be

virtual void Reload(bool check_for_repost, ReloadType reload_type =
ReloadType::NORMAL);

I assume NORMAL in the final stage mean MAIN_RESOURCE of today.

If adding a default type isn't good idea here, I completely agreed that reversed
order is better.

[Charlie Reis, 2016/12/08 23:41:23]

I'm generally not a fan of default arguments, but it could be useful in a case
like this if the vast majority of cases use the default.  I'm ok with leaving
the order as is if we're considering that.

+
   // Reloads the current entry. If |check_for_repost| is true and the current
   // entry has POST data the user is prompted to see if they really want to
   // reload the page. In nearly all cases pass in true.  If a transient entry