Simplified domain display: accommodate browser-initiated navigations

In the hide-on-interaction field trial, we were previously handling
navigations only in DidFinishNavigation(). That is, when a navigation
began, we didn’t change the elision state until it commited in
DidFinishNavigation(). This worked well for renderer-initiated
navigations, where the visible URL doesn’t update until the navigation
commits. But it didn’t work for browser-initiated navigations, where
the URL would update at the very beginning of the navigation and
inherit the elision state of the previous navigation until
DidFinishNavigation (see screencast in crbug.com/1111665).

The fix for this is rather tricky and not perfect. For a
browser-initiated navigation, the order of events is as follows:
- URL is updated (triggering EmphasizeURLComponents)
- DidStartNavigation
- DidFinishNavigation
Because the URL is updated at the first step, we need to set the
elision state in EmphasizeURLComponents. Even setting in
DidStartNavigation would be too late, as we could still get a flash of
the new URL with the previous navigation’s elision applied to it. The
problem, though, is that we don’t know if we should elide the URL
until the navigation finishes, because the elision logic depends on
whether the navigation was same-document and if it changed the visible
URL’s path. (In particular, for same-document navigations that only
change the fragment, we ignore those and don’t have them change the
elision state at all.) Therefore, in EmphasizeURLComponents(), we
don’t know for sure whether we should leave the URL elided if it
already is elided, or if we should unelide and reset to the
on-page-load state.

I wasn’t able to think up a perfect solution for this, but the best I
can come up with is to make our best guess how to elide in
EmphasizeURLComponents(), and then fix it up as we get more
information in DidStartNavigation() and
DidFinishNavigation(). Specifically, EmphasizeURLComponents() elides
to the simplified domain if the URL had already been elided, and shows
the full URL otherwise. DidStartNavigation() then fully resets to the
on-page-load state for cross-document navigations, and
DidFinishNavigation() fully resets to the on-page-load state for
same-document navigations that changed the path (which we don't know
until the navigation commits). There remains a weirdness where a
browser-initiated navigation will initially briefly show the
simplified domain of the destination before updating to the full URL
in DidStartNavigation(), but I think this is far better than briefly
showing the destination URL truncated where the previous page’s URL
was truncated.

Bug: 1111665, 1109778
Change-Id: Ia1528254b8203dee93bd0964666b76d4b15bfd46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360803
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799914}

chrome/browser/ui/views/omnibox/omnibox_view_views.h

@@ -160,6 +160,7 @@ class OmniboxViewViews : public OmniboxView,
   bool IsCommandIdEnabled(int command_id) const override;
 
   // content::WebContentsObserver:
+  void DidStartNavigation(content::NavigationHandle* navigation) override;
   void DidFinishNavigation(content::NavigationHandle* navigation) override;
   void DidGetUserInteraction(const blink::WebInputEvent& event) override;
   void OnFocusChangedInPage(content::FocusedNodeDetails* details) override;
@@ -282,6 +283,9 @@ class OmniboxViewViews : public OmniboxView,
   FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsRevealOnHoverTest, HoverAndExit);
   FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsRevealOnHoverTest, HoverAndExitIDN);
   FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsRevealOnHoverTest, PrivateRegistry);
+  FRIEND_TEST_ALL_PREFIXES(
+      OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
+      BrowserInitiatedNavigation);
   FRIEND_TEST_ALL_PREFIXES(
       OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
       UserInteractionAndHover);

content/public/test/mock_navigation_handle.h

@@ -39,7 +39,8 @@ class MockNavigationHandle : public NavigationHandle {
     return render_frame_host_ ? !render_frame_host_->GetParent() : true;
   }
   MOCK_METHOD0(IsParentMainFrame, bool());
-  bool IsRendererInitiated() override { return true; }
+  // By default, MockNavigationHandles are renderer-initiated navigations.
+  bool IsRendererInitiated() override { return is_renderer_initiated_; }
   MOCK_METHOD0(GetFrameTreeNodeId, int());
   MOCK_METHOD0(GetPreviousRenderFrameHostId, GlobalFrameRoutingId());
   bool IsServedFromBackForwardCache() override { return false; }
@@ -152,6 +153,9 @@ class MockNavigationHandle : public NavigationHandle {
   void set_is_same_document(bool is_same_document) {
     is_same_document_ = is_same_document;
   }
+  void set_is_renderer_initiated(bool is_renderer_initiated) {
+    is_renderer_initiated_ = is_renderer_initiated;
+  }
   void set_redirect_chain(const std::vector<GURL>& redirect_chain) {
     redirect_chain_ = redirect_chain;
   }
@@ -201,6 +205,7 @@ class MockNavigationHandle : public NavigationHandle {
   net::Error net_error_code_ = net::OK;
   RenderFrameHost* render_frame_host_ = nullptr;
   bool is_same_document_ = false;
+  bool is_renderer_initiated_ = true;
   std::vector<GURL> redirect_chain_;
   bool has_committed_ = false;
   bool is_error_page_ = false;