Restrict TextFragmentAnchor to user or browser initiated navigations.

Only activate the text fragment anchor if the navigation was initiated
by the user or browser. This adds a is_browser_initiated bit to
CommitNavigationParams to plumb the browser_initiated_ state from the
browser to the renderer.

Also added TextFragmentAnchorBrowserTests, testing that both user and
browser initiated navigations can activate the feature.

Bug: 924966
Change-Id: Id262eb6967e0109a06739a27b012466031bbd74b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638288Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680536}

content/browser/frame_host/navigation_request.cc

@@ -778,6 +778,8 @@ NavigationRequest::NavigationRequest(
       std::move(navigation_initiator)));
 
   navigation_entry_offset_ = EstimateHistoryOffset();
+
+  commit_params_.is_browser_initiated = browser_initiated_;
 }
 
 NavigationRequest::~NavigationRequest() {

content/browser/navigation_browsertest.cc

@@ -13,6 +13,7 @@
 #include "base/task/post_task.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/test_timeouts.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
 #include "content/browser/child_process_security_policy_impl.h"
@@ -2175,4 +2176,160 @@ IN_PROC_BROWSER_TEST_P(NavigationBrowserTest, AboutSrcDocUsesBeginNavigation) {
   interceptor.Wait(1);  // DidCommitNavigation is called.
 }
 
+class TextFragmentAnchorBrowserTest : public NavigationBrowserTest {
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    NavigationBrowserTest::SetUpCommandLine(command_line);
+
+    command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures,
+                                    "TextFragmentIdentifiers");
+  }
+
+  // Simulates a click on the middle of the DOM element with the given |id|.
+  void ClickElementWithId(WebContents* web_contents, const std::string& id) {
+    // Get the center coordinates of the DOM element.
+    const int x = EvalJs(web_contents,
+                         JsReplace("const bounds = "
+                                   "document.getElementById($1)."
+                                   "getBoundingClientRect();"
+                                   "Math.floor(bounds.left + bounds.width / 2)",
+                                   id))
+                      .ExtractInt();
+    const int y = EvalJs(web_contents,
+                         JsReplace("const bounds = "
+                                   "document.getElementById($1)."
+                                   "getBoundingClientRect();"
+                                   "Math.floor(bounds.top + bounds.height / 2)",
+                                   id))
+                      .ExtractInt();
+
+    SimulateMouseClickAt(web_contents, 0, blink::WebMouseEvent::Button::kLeft,
+                         gfx::Point(x, y));
+  }
+
+  void WaitForPageLoad(WebContents* contents) {
+    EXPECT_TRUE(WaitForLoadStop(contents));
+    EXPECT_TRUE(WaitForRenderFrameReady(contents->GetMainFrame()));
+  }
+};
+
+IN_PROC_BROWSER_TEST_P(TextFragmentAnchorBrowserTest, EnabledOnUserNavigation) {
+  GURL url(embedded_test_server()->GetURL("/target_text_link.html"));
+  GURL target_text_url(embedded_test_server()->GetURL(
+      "/scrollable_page_with_content.html#targetText=text"));
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  WebContents* main_contents = shell()->web_contents();
+  TestNavigationObserver observer(main_contents);
+  RenderFrameSubmissionObserver frame_observer(main_contents);
+  ClickElementWithId(main_contents, "link");
+  observer.Wait();
+  EXPECT_EQ(target_text_url, main_contents->GetLastCommittedURL());
+
+  WaitForPageLoad(main_contents);
+  frame_observer.WaitForScrollOffsetAtTop(false);
+  EXPECT_FALSE(main_contents->GetMainFrame()->GetView()->IsScrollOffsetAtTop());
+}
+
+IN_PROC_BROWSER_TEST_P(TextFragmentAnchorBrowserTest,
+                       EnabledOnBrowserNavigation) {
+  GURL url(embedded_test_server()->GetURL(
+      "/scrollable_page_with_content.html#targetText=text"));
+  WebContents* main_contents = shell()->web_contents();
+  RenderFrameSubmissionObserver frame_observer(main_contents);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  WaitForPageLoad(main_contents);
+  frame_observer.WaitForScrollOffsetAtTop(false);
+  EXPECT_FALSE(main_contents->GetMainFrame()->GetView()->IsScrollOffsetAtTop());
+}
+
+IN_PROC_BROWSER_TEST_P(TextFragmentAnchorBrowserTest,
+                       EnabledOnUserGestureScriptNavigation) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+  GURL target_text_url(embedded_test_server()->GetURL(
+      "/scrollable_page_with_content.html#targetText=text"));
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  WebContents* main_contents = shell()->web_contents();
+  TestNavigationObserver observer(main_contents);
+  RenderFrameSubmissionObserver frame_observer(main_contents);
+
+  // ExecuteScript executes with a user gesture
+  EXPECT_TRUE(ExecuteScript(main_contents,
+                            "location = '" + target_text_url.spec() + "';"));
+  observer.Wait();
+  EXPECT_EQ(target_text_url, main_contents->GetLastCommittedURL());
+
+  WaitForPageLoad(main_contents);
+  frame_observer.WaitForScrollOffsetAtTop(false);
+  EXPECT_FALSE(main_contents->GetMainFrame()->GetView()->IsScrollOffsetAtTop());
+}
+
+IN_PROC_BROWSER_TEST_P(TextFragmentAnchorBrowserTest,
+                       DisabledOnScriptNavigation) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+  GURL target_text_url(embedded_test_server()->GetURL(
+      "/scrollable_page_with_content.html#targetText=text"));
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  WebContents* main_contents = shell()->web_contents();
+  TestNavigationObserver observer(main_contents);
+  EXPECT_TRUE(ExecuteScriptWithoutUserGesture(
+      main_contents, "location = '" + target_text_url.spec() + "';"));
+  observer.Wait();
+  EXPECT_EQ(target_text_url, main_contents->GetLastCommittedURL());
+
+  WaitForPageLoad(main_contents);
+
+  // Wait a short amount of time to ensure the page does not scroll.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+  run_loop.Run();
+  EXPECT_TRUE(main_contents->GetMainFrame()->GetView()->IsScrollOffsetAtTop());
+}
+
+IN_PROC_BROWSER_TEST_P(TextFragmentAnchorBrowserTest,
+                       DisabledOnScriptHistoryNavigation) {
+  GURL target_text_url(embedded_test_server()->GetURL(
+      "/scrollable_page_with_content.html#targetText=text"));
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  EXPECT_TRUE(NavigateToURL(shell(), target_text_url));
+
+  WebContents* main_contents = shell()->web_contents();
+  RenderFrameSubmissionObserver frame_observer(main_contents);
+  frame_observer.WaitForScrollOffsetAtTop(false);
+
+  // Scroll the page back to top so scroll restoration does not scroll the
+  // target back into view.
+  EXPECT_TRUE(ExecuteScript(main_contents, "window.scrollTo(0, 0)"));
+  frame_observer.WaitForScrollOffsetAtTop(true);
+
+  EXPECT_TRUE(NavigateToURL(shell(), url));
+
+  TestNavigationObserver observer(main_contents);
+  EXPECT_TRUE(ExecuteScriptWithoutUserGesture(main_contents, "history.back()"));
+  observer.Wait();
+  EXPECT_EQ(target_text_url, main_contents->GetLastCommittedURL());
+
+  WaitForPageLoad(main_contents);
+
+  // Wait a short amount of time to ensure the page does not scroll.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+  run_loop.Run();
+  EXPECT_TRUE(main_contents->GetMainFrame()->GetView()->IsScrollOffsetAtTop());
+}
+
+INSTANTIATE_TEST_SUITE_P(/* no prefix */,
+                         TextFragmentAnchorBrowserTest,
+                         ::testing::Bool());
+
 }  // namespace content

content/common/frame_messages.h

@@ -537,6 +537,7 @@ IPC_STRUCT_TRAITS_BEGIN(content::CommitNavigationParams)
 #if defined(OS_ANDROID)
   IPC_STRUCT_TRAITS_MEMBER(data_url_as_string)
 #endif
+  IPC_STRUCT_TRAITS_MEMBER(is_browser_initiated)
 IPC_STRUCT_TRAITS_END()
 
 IPC_STRUCT_TRAITS_BEGIN(blink::ParsedFeaturePolicyDeclaration)

content/common/navigation_params.h

@@ -350,6 +350,9 @@ struct CONTENT_EXPORT CommitNavigationParams {
   // passed in the |CommonNavigationParams::url| field.
   std::string data_url_as_string;
 #endif
+
+  // Whether this navigation was browser initiated.
+  bool is_browser_initiated = false;
 };
 
 }  // namespace content

content/renderer/render_frame_impl.cc

@@ -510,9 +510,7 @@ void FillNavigationParamsRequest(
     }
   }
 
-#if defined(OS_ANDROID)
   navigation_params->had_transient_activation = common_params.has_user_gesture;
-#endif
 }
 
 CommonNavigationParams MakeCommonNavigationParams(
@@ -1039,6 +1037,8 @@ void FillMiscNavigationParams(const CommonNavigationParams& common_params,
   navigation_params->is_user_activated =
       commit_params.was_activated == WasActivatedOption::kYes;
 
+  navigation_params->is_browser_initiated = commit_params.is_browser_initiated;
+
   if (commit_params.origin_to_commit) {
     navigation_params->origin_to_commit =
         commit_params.origin_to_commit.value();

content/test/data/scrollable_page_with_content.html

+<html>
+  <head>
+    <meta name="viewport" content="width=device-width, minimum-scale=1.0">
+    <style>
+      p {
+        position: absolute;
+        top: 10000px;
+      }
+    </style>
+  </head>
+  <body>
+    <p>Some text</p>
+  </body>
+</html>>

content/test/data/target_text_link.html

+<html>
+  <head>
+    <meta name="viewport" content="width=device-width, minimum-scale=1.0">
+  </head>
+  <body>
+    <a id="link" href="/scrollable_page_with_content.html#targetText=text">link</a>
+  </body>
+</html>>

third_party/blink/public/web/web_navigation_params.h

@@ -295,6 +295,8 @@ struct BLINK_EXPORT WebNavigationParams {
   bool had_transient_activation = false;
   // Whether this navigation has a sticky user activation flag.
   bool is_user_activated = false;
+  // Whether this navigation was browser initiated.
+  bool is_browser_initiated = false;
   // The previews state which should be used for this navigation.
   WebURLRequest::PreviewsState previews_state =
       WebURLRequest::kPreviewsUnspecified;

third_party/blink/renderer/core/exported/web_document_loader_impl.cc

@@ -189,7 +189,7 @@ WebArchiveInfo WebDocumentLoaderImpl::GetArchiveInfo() const {
 }
 
 bool WebDocumentLoaderImpl::HadUserGesture() const {
-  return DocumentLoader::had_transient_activation();
+  return DocumentLoader::HadTransientActivation();
 }
 
 bool WebDocumentLoaderImpl::IsListingFtpDirectory() const {

third_party/blink/renderer/core/frame/frame_test_helpers.cc

@@ -118,6 +118,7 @@ void LoadFrameDontWait(WebLocalFrame* frame, const WebURL& url) {
     params->url = url;
     params->navigation_timings.navigation_start = base::TimeTicks::Now();
     params->navigation_timings.fetch_start = base::TimeTicks::Now();
+    params->is_browser_initiated = true;
     FillNavigationParamsResponse(params.get());
     impl->CommitNavigation(std::move(params), nullptr /* extra_data */);
   }

third_party/blink/renderer/core/loader/document_loader.cc

@@ -133,6 +133,7 @@ DocumentLoader::DocumentLoader(
       data_buffer_(SharedBuffer::Create()),
       devtools_navigation_token_(params_->devtools_navigation_token),
       had_sticky_activation_(params_->is_user_activated),
+      is_browser_initiated_(params_->is_browser_initiated),
       was_discarded_(params_->was_discarded),
       use_counter_(),
       clock_(params_->tick_clock ? params_->tick_clock

third_party/blink/renderer/core/loader/document_loader.h

@@ -289,9 +289,11 @@ class CORE_EXPORT DocumentLoader
     return previews_state_;
   }
 
- protected:
-  bool had_transient_activation() const { return had_transient_activation_; }
+  bool HadTransientActivation() const { return had_transient_activation_; }
+
+  bool IsBrowserInitiated() const { return is_browser_initiated_; }
 
+ protected:
   Vector<KURL> redirect_chain_;
 
   // Archive used to load document and/or subresources. If one of the ancestor
@@ -480,6 +482,9 @@ class CORE_EXPORT DocumentLoader
   // Whether this load request had a user activation when created.
   bool had_transient_activation_ = false;
 
+  // Whether this load request was initiated by the browser.
+  bool is_browser_initiated_ = false;
+
   // See WebNavigationParams for definition.
   bool was_discarded_ = false;
 

third_party/blink/renderer/core/page/scrolling/text_fragment_anchor.cc

@@ -14,6 +14,7 @@
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/layout/layout_object.h"
+#include "third_party/blink/renderer/core/loader/document_loader.h"
 #include "third_party/blink/renderer/core/page/scrolling/text_fragment_selector.h"
 #include "third_party/blink/renderer/core/scroll/scroll_alignment.h"
 #include "third_party/blink/renderer/core/scroll/scrollable_area.h"
@@ -66,6 +67,13 @@ TextFragmentAnchor* TextFragmentAnchor::TryCreate(
       same_document_navigation)
     return nullptr;
 
+  // For security reasons, we only allow text fragment anchors for user or
+  // browser initiated navigations, i.e. no script navigations.
+  if (!(frame.Loader().GetDocumentLoader()->HadTransientActivation() ||
+        frame.Loader().GetDocumentLoader()->IsBrowserInitiated())) {
+    return nullptr;
+  }
+
   Vector<TextFragmentSelector> selectors;
 
   if (!ParseTargetTextIdentifier(url.FragmentIdentifier(), &selectors))