PlzNavigate: identify same-page browser-initiated navigation.

url/url_util.h

Index: url/url_util.h
diff --git a/url/url_util.h b/url/url_util.h
index e424c503757edbf2002f081bb2cc8499ffaebcd1..2ee7e57657260b0f31d0463db9d5a2f0e71f4c12 100644
--- a/url/url_util.h
+++ b/url/url_util.h
@@ -176,6 +176,13 @@ URL_EXPORT bool DomainIs(base::StringPiece canonicalized_host,
 // cheap, as it must re-parse the host to verify.
 URL_EXPORT bool HostIsIPAddress(base::StringPiece host);
 
+// Fragment --------------------------------------------------------------------
+
+// Checks whether or not two URLs are differing only in the fragment (the part
+// after the # character). Returns false when the new URL has no fragment.
+URL_EXPORT bool IsFragmentAddedOrUpdated(const GURL& old_url,

[brettw, 2017/01/26 20:44:46]

Several points:

Regarding the naming: This code uses the term "ref" for the reference fragment
rather than "fragment". The naming of "changed" implies a temporal change which
may make sense in terms of the caller in navigator_impl.cc but makes less sense
to me in terms of a function taking two parameters.

This function is being used to compute whether the navigation is in-page. If the
2nd parameter has no ref or the URLs are equal, it doesn't imply to me that the
navigation is *not* in page. This seems like pretty strange semantics to me. Are
you sure this is what you want? If it is, this seems to be something specific to
the rules of how Blink sends us navigation messages rather than an intrinsic
property of URLs, and this exact behavior would be better implemented in the
navigation layer.

KURL has a function equalIgnoringFragmentIdentifier. This seems very easy to
reason about. I would be happy adding a GURL::EqualsIgnoringRef function and
then you wrap that in the navigation layer if there are extra rules you want to
implement on top of that about navigating from refs to no refs.



url_util is at a conceptually lower layer than GURL: it provides the common
backing for GURL and KURL. Functions dealing with GURL should go on the GURL
object.



Implementation:

The implementation is pretty heavyweight, involving reallocating and
recanonicalizing two URLs just for comparison. At this low layer, I think the
operations should be more optimized. To do this, call
Parsed::CountCharactersBefore(REF, <not_sure>) and compare the string pieces of
the computed prefix to see if the URLs are equivalent ignoring the ref.

+                                         const GURL& new_url);
+
 // URL library wrappers --------------------------------------------------------
 
 // Parses the given spec according to the extracted scheme type. Normal users