SpeculationRule: Implement "target_hint" for prerendering This CL implements "target_hint" for Speculation Rules based on the following spec PR: https://github.com/WICG/nav-speculation/pull/173 - This CL propagates "target_hint" from blink/ to content/browser/, but the browser process doesn't use this information yet. Follow-up CLs will implement the part. This half-baked implementation is still spec compatible as this information is just a hint and browser implementation is allowed to ignore them. - The spec requires the browser to accept the "valid browsing context name or keyword"[1] as the hint. As the initial step, this CL only supports "_blank" and "_self". If unsupported names or keywords are specified in the prerender rules, the rules are treated as no hint. - When "target_hint" is specified in prefetch rules, the rules are ignored. [1]https://html.spec.whatwg.org/C/#valid-browsing-context-name-or-keyword Bug: 1354049 Change-Id: Ia4a876e11e814f2dcc86c7dc19bef31b813c33ae Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3833323 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Lingqi Chi <lingqi@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Jeremy Roman <jbroman@chromium.org> Cr-Commit-Position: refs/heads/main@{#1041915}
diff --git a/content/browser/preloading/speculation_rules/speculation_host_impl.cc b/content/browser/preloading/speculation_rules/speculation_host_impl.cc index e45f125..538e30f 100644 --- a/content/browser/preloading/speculation_rules/speculation_host_impl.cc +++ b/content/browser/preloading/speculation_rules/speculation_host_impl.cc
@@ -42,6 +42,15 @@ mojo::ReportBadMessage("SH_NON_HTTP"); return false; } + + // `target_browsing_context_name_hint` on non-prerender actions should be + // filtered out in Blink. + if (candidate->action != blink::mojom::SpeculationAction::kPrerender && + candidate->target_browsing_context_name_hint != + blink::mojom::SpeculationTargetHint::kNoHint) { + mojo::ReportBadMessage("SH_TARGET_HINT_ON_PREFETCH"); + return false; + } } return true; } @@ -330,6 +339,8 @@ } } + // TODO(crbug.com/1354049): Pass `target_browsing_context_name_hint` to + // start prerendering in a new tab. Referrer referrer(*(it->referrer)); int prerender_host_id = registry_->CreateAndStartHost( PrerenderAttributes(it->url, PrerenderTriggerType::kSpeculationRule, @@ -341,6 +352,10 @@ rfhi.GetPageUkmSourceId(), ui::PAGE_TRANSITION_LINK, /*url_match_predicate=*/absl::nullopt), *web_contents, /*preloading_attempt=*/preloading_attempt); + // TODO(crbug.com/1354049): Handle the case where multiple speculation rules + // have the same URL but its `target_browsing_context_name_hint` is + // different. In the current implementation, only the first rule is + // triggered. started_prerenders_.insert(end, {.url = it->url, .referrer = referrer, .prerender_host_id = prerender_host_id});
diff --git a/content/browser/preloading/speculation_rules/speculation_host_impl_unittest.cc b/content/browser/preloading/speculation_rules/speculation_host_impl_unittest.cc index 3c2b92b6..f611bf4 100644 --- a/content/browser/preloading/speculation_rules/speculation_host_impl_unittest.cc +++ b/content/browser/preloading/speculation_rules/speculation_host_impl_unittest.cc
@@ -141,7 +141,7 @@ registry->FindHostByUrlForTesting(kSecondPrerenderingUrlSameOrigin)); } -// Tests that SpeculationHostImpl crash the renderer process if it receives +// Tests that SpeculationHostImpl crashes the renderer process if it receives // non-http prerender candidates. TEST_F(SpeculationHostImplTest, ReportNonHttpMessage) { RenderFrameHostImpl* render_frame_host = GetRenderFrameHost(); @@ -169,6 +169,40 @@ EXPECT_FALSE(registry->FindHostByUrlForTesting(kPrerenderingUrl)); } +// Tests that SpeculationHostImpl crashes the renderer process if it receives +// prefetch candidates that have a valid `target_browsing_context_name_hint`. +TEST_F(SpeculationHostImplTest, + ReportTargetBrowsingContextNameHintOnPrefetchCandidate) { + RenderFrameHostImpl* render_frame_host = GetRenderFrameHost(); + mojo::Remote<blink::mojom::SpeculationHost> remote; + SpeculationHostImpl::Bind(render_frame_host, + remote.BindNewPipeAndPassReceiver()); + + // Set up the error handler for bad mojo messages. + std::string bad_message_error; + mojo::SetDefaultProcessErrorHandler( + base::BindLambdaForTesting([&](const std::string& error) { + EXPECT_FALSE(error.empty()); + EXPECT_TRUE(bad_message_error.empty()); + bad_message_error = error; + })); + + // Create a prefetch candidate that has a valid target hint. + auto candidate = blink::mojom::SpeculationCandidate::New(); + candidate->action = blink::mojom::SpeculationAction::kPrefetch; + candidate->url = GetSameOriginUrl("/empty.html"); + candidate->referrer = blink::mojom::Referrer::New(); + candidate->target_browsing_context_name_hint = + blink::mojom::SpeculationTargetHint::kBlank; + + std::vector<blink::mojom::SpeculationCandidatePtr> candidates; + candidates.push_back(std::move(candidate)); + + remote->UpdateSpeculationCandidates(std::move(candidates)); + remote.FlushForTesting(); + EXPECT_EQ(bad_message_error, "SH_TARGET_HINT_ON_PREFETCH"); +} + class TestSpeculationHostDelegate : public SpeculationHostDelegate { public: TestSpeculationHostDelegate() = default;
diff --git a/third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom b/third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom index b719e57..9fc848d 100644 --- a/third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom +++ b/third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom
@@ -31,8 +31,17 @@ kPrerender, }; +// The target hint that is proposed. +enum SpeculationTargetHint { + kNoHint, + kBlank, + kSelf, +}; + // A single candidate: a URL, an action, a referrer, and any associated // metadata that might be needed to make a decision. +// https://wicg.github.io/nav-speculation/speculation-rules.html#prefetch-candidate +// https://wicg.github.io/nav-speculation/speculation-rules.html#prerender-candidate struct SpeculationCandidate { // The URL which is eligible for some action. url.mojom.Url url; @@ -47,4 +56,8 @@ // made in a manner which anonymizes the client IP. If this is not possible, // this candidate must be discarded. bool requires_anonymous_client_ip_when_cross_origin = false; + + // The hint to be used to decide a target browsing context where preloaded + // resource will be used. This is kNoHint for actions other than `kPrerender`. + SpeculationTargetHint target_browsing_context_name_hint = kNoHint; };
diff --git a/third_party/blink/renderer/core/speculation_rules/document_speculation_rules.cc b/third_party/blink/renderer/core/speculation_rules/document_speculation_rules.cc index 29c96a9..e49f6e5 100644 --- a/third_party/blink/renderer/core/speculation_rules/document_speculation_rules.cc +++ b/third_party/blink/renderer/core/speculation_rules/document_speculation_rules.cc
@@ -104,7 +104,9 @@ KURL(referrer.referrer), referrer.referrer_policy); candidates.push_back(mojom::blink::SpeculationCandidate::New( url, action, std::move(referrer_ptr), - rule->requires_anonymous_client_ip_when_cross_origin())); + rule->requires_anonymous_client_ip_when_cross_origin(), + rule->target_browsing_context_name_hint().value_or( + mojom::blink::SpeculationTargetHint::kNoHint))); } } };
diff --git a/third_party/blink/renderer/core/speculation_rules/speculation_rule.cc b/third_party/blink/renderer/core/speculation_rules/speculation_rule.cc index 950c701..bc3e509 100644 --- a/third_party/blink/renderer/core/speculation_rules/speculation_rule.cc +++ b/third_party/blink/renderer/core/speculation_rules/speculation_rule.cc
@@ -8,9 +8,11 @@ SpeculationRule::SpeculationRule( Vector<KURL> urls, - RequiresAnonymousClientIPWhenCrossOrigin requires_anonymous_client_ip) - : urls_(urls), - requires_anonymous_client_ip_(requires_anonymous_client_ip) {} + RequiresAnonymousClientIPWhenCrossOrigin requires_anonymous_client_ip, + absl::optional<mojom::blink::SpeculationTargetHint> target_hint) + : urls_(std::move(urls)), + requires_anonymous_client_ip_(requires_anonymous_client_ip), + target_browsing_context_name_hint_(target_hint) {} SpeculationRule::~SpeculationRule() = default;
diff --git a/third_party/blink/renderer/core/speculation_rules/speculation_rule.h b/third_party/blink/renderer/core/speculation_rules/speculation_rule.h index fc32843..bff737d 100644 --- a/third_party/blink/renderer/core/speculation_rules/speculation_rule.h +++ b/third_party/blink/renderer/core/speculation_rules/speculation_rule.h
@@ -6,6 +6,7 @@ #define THIRD_PARTY_BLINK_RENDERER_CORE_SPECULATION_RULES_SPECULATION_RULE_H_ #include "base/types/strong_alias.h" +#include "third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom-blink.h" #include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/platform/heap/garbage_collected.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" @@ -23,19 +24,28 @@ base::StrongAlias<class RequiresAnonymousClientIPWhenCrossOriginTag, bool>; - SpeculationRule(Vector<KURL>, RequiresAnonymousClientIPWhenCrossOrigin); + SpeculationRule( + Vector<KURL>, + RequiresAnonymousClientIPWhenCrossOrigin, + absl::optional<mojom::blink::SpeculationTargetHint> target_hint); ~SpeculationRule(); const Vector<KURL>& urls() const { return urls_; } bool requires_anonymous_client_ip_when_cross_origin() const { return requires_anonymous_client_ip_.value(); } + absl::optional<mojom::blink::SpeculationTargetHint> + target_browsing_context_name_hint() const { + return target_browsing_context_name_hint_; + } void Trace(Visitor*) const; private: - Vector<KURL> urls_; - RequiresAnonymousClientIPWhenCrossOrigin requires_anonymous_client_ip_; + const Vector<KURL> urls_; + const RequiresAnonymousClientIPWhenCrossOrigin requires_anonymous_client_ip_; + const absl::optional<mojom::blink::SpeculationTargetHint> + target_browsing_context_name_hint_; }; } // namespace blink
diff --git a/third_party/blink/renderer/core/speculation_rules/speculation_rule_set.cc b/third_party/blink/renderer/core/speculation_rules/speculation_rule_set.cc index 26852698..a2f7cea 100644 --- a/third_party/blink/renderer/core/speculation_rules/speculation_rule_set.cc +++ b/third_party/blink/renderer/core/speculation_rules/speculation_rule_set.cc
@@ -5,20 +5,51 @@ #include "third_party/blink/renderer/core/speculation_rules/speculation_rule_set.h" #include "base/containers/contains.h" +#include "third_party/blink/public/mojom/speculation_rules/speculation_rules.mojom-blink.h" #include "third_party/blink/renderer/platform/json/json_parser.h" #include "third_party/blink/renderer/platform/json/json_values.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" +#include "third_party/blink/renderer/platform/wtf/text/string_view.h" namespace blink { namespace { +// https://html.spec.whatwg.org/C/#valid-browsing-context-name +bool IsValidContextName(const String& name_or_keyword) { + // "A valid browsing context name is any string with at least one character + // that does not start with a U+005F LOW LINE character. (Names starting with + // an underscore are reserved for special keywords.)" + if (name_or_keyword.IsEmpty()) + return false; + if (name_or_keyword.StartsWith("_")) + return false; + return true; +} + +// https://html.spec.whatwg.org/C/#valid-browsing-context-name-or-keyword +bool IsValidBrowsingContextNameOrKeyword(const String& name_or_keyword) { + // "A valid browsing context name or keyword is any string that is either a + // valid browsing context name or that is an ASCII case-insensitive match for + // one of: _blank, _self, _parent, or _top." + String canonicalized_name_or_keyword = name_or_keyword.LowerASCII(); + if (IsValidContextName(name_or_keyword) || + EqualIgnoringASCIICase(name_or_keyword, "_blank") || + EqualIgnoringASCIICase(name_or_keyword, "_self") || + EqualIgnoringASCIICase(name_or_keyword, "_parent") || + EqualIgnoringASCIICase(name_or_keyword, "_top")) { + return true; + } + return false; +} + SpeculationRule* ParseSpeculationRule(JSONObject* input, const KURL& base_url) { // https://wicg.github.io/nav-speculation/speculation-rules.html#parse-a-speculation-rule - // If input has any key other than "source", "urls" and "requires", then - // return null. - const char* const kKnownKeys[] = {"source", "urls", "requires"}; + // If input has any key other than "source", "urls", "requires", and + // "target_hint", then return null. + const char* const kKnownKeys[] = {"source", "urls", "requires", + "target_hint"}; for (wtf_size_t i = 0; i < input->size(); ++i) { if (!base::Contains(kKnownKeys, input->at(i).first)) return nullptr; @@ -78,8 +109,34 @@ } } - return MakeGarbageCollected<SpeculationRule>(std::move(urls), - requires_anonymous_client_ip); + // Let targetHint be null. + absl::optional<mojom::blink::SpeculationTargetHint> target_hint; + + // If input["target_hint"] exists: + JSONValue* target_hint_value = input->Get("target_hint"); + if (target_hint_value) { + // If input["target_hint"] is not a valid browsing context name or keyword, + // then return null. + // Set targetHint to input["target_hint"]. + String target_hint_str; + if (!target_hint_value->AsString(&target_hint_str)) + return nullptr; + if (!IsValidBrowsingContextNameOrKeyword(target_hint_str)) + return nullptr; + // Currently only "_blank" and "_self" are supported. + // TODO(https://crbug.com/1354049): Support more browsing context names and + // keywords. + if (EqualIgnoringASCIICase(target_hint_str, "_blank")) { + target_hint = mojom::blink::SpeculationTargetHint::kBlank; + } else if (EqualIgnoringASCIICase(target_hint_str, "_self")) { + target_hint = mojom::blink::SpeculationTargetHint::kSelf; + } else { + target_hint = mojom::blink::SpeculationTargetHint::kNoHint; + } + } + + return MakeGarbageCollected<SpeculationRule>( + std::move(urls), requires_anonymous_client_ip, target_hint); } } // namespace @@ -88,7 +145,7 @@ SpeculationRuleSet* SpeculationRuleSet::ParseInline(const String& source_text, const KURL& base_url, String* out_error) { - // https://wicg.github.io/nav-speculation/prerendering.html#parse-speculation-rules + // https://wicg.github.io/nav-speculation/speculation-rules.html#parse-speculation-rules // Let parsed be the result of parsing a JSON string to an Infra value given // input. @@ -109,31 +166,49 @@ SpeculationRuleSet* result = MakeGarbageCollected<SpeculationRuleSet>(); const auto parse_for_action = - [&](const char* key, HeapVector<Member<SpeculationRule>>& destination) { + [&](const char* key, HeapVector<Member<SpeculationRule>>& destination, + bool allow_target_hint) { JSONArray* array = parsed->GetArray(key); if (!array) return; for (wtf_size_t i = 0; i < array->size(); ++i) { + // If prefetch/prerenderRule is not a map, then continue. JSONObject* input_rule = JSONObject::Cast(array->at(i)); if (!input_rule) continue; - if (SpeculationRule* r = ParseSpeculationRule(input_rule, base_url)) - destination.push_back(r); + // Let rule be the result of parsing a speculation rule given + // prefetch/prerenderRule and baseURL. + SpeculationRule* rule = ParseSpeculationRule(input_rule, base_url); + + // If rule is null, then continue. + if (!rule) + continue; + + // If rule's target browsing context name hint is not null, then + // continue. + if (!allow_target_hint && + rule->target_browsing_context_name_hint().has_value()) { + continue; + } + + // Append rule to result's prefetch/prerender rules. + destination.push_back(rule); } }; // If parsed["prefetch"] exists and is a list, then for each... - parse_for_action("prefetch", result->prefetch_rules_); + parse_for_action("prefetch", result->prefetch_rules_, false); // If parsed["prefetch_with_subresources"] exists and is a list, then for // each... parse_for_action("prefetch_with_subresources", - result->prefetch_with_subresources_rules_); + result->prefetch_with_subresources_rules_, false); // If parsed["prerender"] exists and is a list, then for each... - parse_for_action("prerender", result->prerender_rules_); + parse_for_action("prerender", result->prerender_rules_, true); + return result; }
diff --git a/third_party/blink/renderer/core/speculation_rules/speculation_rule_set_test.cc b/third_party/blink/renderer/core/speculation_rules/speculation_rule_set_test.cc index b3ba4a8..69bed1d 100644 --- a/third_party/blink/renderer/core/speculation_rules/speculation_rule_set_test.cc +++ b/third_party/blink/renderer/core/speculation_rules/speculation_rule_set_test.cc
@@ -86,6 +86,30 @@ return arg->requires_anonymous_client_ip_when_cross_origin(); } +SpeculationRuleSet* CreateSpeculationRuleSetWithTargetHint( + const char* target_hint) { + return SpeculationRuleSet::ParseInline( + String::Format(R"({ + "prefetch": [{ + "source": "list", + "urls": ["https://example.com/hint.html"], + "target_hint": "%s" + }], + "prefetch_with_subresources": [{ + "source": "list", + "urls": ["https://example.com/hint.html"], + "target_hint": "%s" + }], + "prerender": [{ + "source": "list", + "urls": ["https://example.com/hint.html"], + "target_hint": "%s" + }] + })", + target_hint, target_hint, target_hint), + KURL("https://example.com/")); +} + class SpeculationRuleSetTest : public ::testing::Test { private: ScopedSpeculationRulesPrefetchProxyForTest enable_prefetch_{true}; @@ -256,6 +280,106 @@ ElementsAre(MatchesListOfURLs("https://example.com/valid.html"))); } +// Test that only prerender rule can process a "_blank" target hint. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_Blank) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("_blank"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), + ElementsAre(MatchesListOfURLs("https://example.com/hint.html"))); + EXPECT_EQ(rule_set->prerender_rules()[0]->target_browsing_context_name_hint(), + mojom::blink::SpeculationTargetHint::kBlank); +} + +// Test that only prerender rule can process a "_self" target hint. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_Self) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("_self"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), + ElementsAre(MatchesListOfURLs("https://example.com/hint.html"))); + EXPECT_EQ(rule_set->prerender_rules()[0]->target_browsing_context_name_hint(), + mojom::blink::SpeculationTargetHint::kSelf); +} + +// Test that only prerender rule can process a "_parent" target hint but treat +// it as no hint. +// TODO(https://crbug.com/1354049): Support the "_parent" keyword for +// prerendering. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_Parent) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("_parent"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), + ElementsAre(MatchesListOfURLs("https://example.com/hint.html"))); + EXPECT_EQ(rule_set->prerender_rules()[0]->target_browsing_context_name_hint(), + mojom::blink::SpeculationTargetHint::kNoHint); +} + +// Test that only prerender rule can process a "_top" target hint but treat it +// as no hint. +// Test that rules with a "_top" hint are ignored. +// TODO(https://crbug.com/1354049): Support the "_top" keyword for prerendering. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_Top) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("_top"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), + ElementsAre(MatchesListOfURLs("https://example.com/hint.html"))); + EXPECT_EQ(rule_set->prerender_rules()[0]->target_browsing_context_name_hint(), + mojom::blink::SpeculationTargetHint::kNoHint); +} + +// Test that rules with an empty target hint are ignored. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_EmptyString) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint(""); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), ElementsAre()); +} + +// Test that only prerender rule can process a browsing context name target hint +// but treat it as no hint. +// TODO(https://crbug.com/1354049): Support valid browsing context names. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_ValidBrowsingContextName) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("valid"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), + ElementsAre(MatchesListOfURLs("https://example.com/hint.html"))); + EXPECT_EQ(rule_set->prerender_rules()[0]->target_browsing_context_name_hint(), + mojom::blink::SpeculationTargetHint::kNoHint); +} + +// Test that rules with an invalid browsing context name target hint are +// ignored. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_InvalidBrowsingContextName) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("_invalid"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), ElementsAre()); +} + +// Test that the the validation of the browsing context keywords runs an ASCII +// case-insensitive match. +TEST_F(SpeculationRuleSetTest, RulesWithTargetHint_CaseInsensitive) { + auto* rule_set = CreateSpeculationRuleSetWithTargetHint("_BlAnK"); + ASSERT_TRUE(rule_set); + EXPECT_THAT(rule_set->prefetch_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prefetch_with_subresources_rules(), ElementsAre()); + EXPECT_THAT(rule_set->prerender_rules(), + ElementsAre(MatchesListOfURLs("https://example.com/hint.html"))); + EXPECT_EQ(rule_set->prerender_rules()[0]->target_browsing_context_name_hint(), + mojom::blink::SpeculationTargetHint::kBlank); +} + TEST_F(SpeculationRuleSetTest, PropagatesToDocument) { // A <script> with a case-insensitive type match should be propagated to the // document.