Commit 8bd593ba authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

[ContentIndex] Check if the launch URL is offline capable

This is guarded behind a feature flag. Ideally, we should be able to
check whether a launch URL works offline before registering the content
index entry.

Bug: 1122646
Change-Id: I204828691590670f3625eb02b53c1bb6dbbc574a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379836
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: default avatarRichard Knoll <knollr@chromium.org>
Reviewed-by: default avatarTom Sepez <tsepez@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814287}
parent a48217a0
......@@ -7,6 +7,7 @@
#include "base/run_loop.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/content_index_context.h"
#include "content/public/browser/storage_partition.h"
......@@ -54,12 +55,16 @@ class ContentIndexTest : public ContentBrowserTest {
switches::kEnableExperimentalWebPlatformFeatures);
}
// Runs |script| and expects it to complete successfully.
void RunScript(const std::string& script) {
std::string RunScriptWithResult(const std::string& script) {
std::string result;
ASSERT_TRUE(
EXPECT_TRUE(
ExecuteScriptAndExtractString(shell_->web_contents(), script, &result));
ASSERT_EQ(result, "ok");
return result;
}
// Runs |script| and expects it to complete successfully.
void RunScript(const std::string& script) {
ASSERT_EQ(RunScriptWithResult(script), "ok");
}
std::vector<SkBitmap> GetIcons(int64_t service_worker_registration_id,
......@@ -163,5 +168,27 @@ IN_PROC_BROWSER_TEST_F(ContentIndexTest, BestIconIsChosen) {
]))");
}
class ContentIndexOfflineCapabilityTest : public ContentIndexTest {
void SetUp() override {
feature_list_.InitFromCommandLine("ContentIndexCheckOffline", "");
ContentIndexTest::SetUp();
}
private:
base::test::ScopedFeatureList feature_list_;
};
IN_PROC_BROWSER_TEST_F(ContentIndexOfflineCapabilityTest,
CheckOfflineCapability) {
// Registering content should still work if the url is offline-capable.
RunScript("addContent('id1', [{src: '/single_face.jpg'}], 'forcesuccess')");
// Registering content should fail if the url is not offline-capable.
std::string result = RunScriptWithResult(
"addContent('id2', [{src: '/single_face.jpg'}], 'forcefail')");
EXPECT_EQ(result,
"TypeError - The provided launch URL is not offline-capable.");
}
} // namespace
} // namespace content
......@@ -18,6 +18,25 @@
namespace content {
namespace {
void DidCheckOfflineCapability(
ContentIndexServiceImpl::CheckOfflineCapabilityCallback callback,
int64_t expected_registration_id,
OfflineCapability capability,
int64_t registration_id) {
switch (capability) {
case OfflineCapability::kUnsupported:
std::move(callback).Run(false);
return;
case OfflineCapability::kSupported:
std::move(callback).Run(expected_registration_id == registration_id);
return;
}
}
} // namespace
// static
void ContentIndexServiceImpl::CreateForFrame(
RenderFrameHost* render_frame_host,
......@@ -31,7 +50,8 @@ void ContentIndexServiceImpl::CreateForFrame(
mojo::MakeSelfOwnedReceiver(std::make_unique<ContentIndexServiceImpl>(
render_frame_host->GetLastCommittedOrigin(),
storage_partition->GetContentIndexContext()),
storage_partition->GetContentIndexContext(),
storage_partition->GetServiceWorkerContext()),
std::move(receiver));
}
......@@ -52,15 +72,18 @@ void ContentIndexServiceImpl::CreateForWorker(
mojo::MakeSelfOwnedReceiver(
std::make_unique<ContentIndexServiceImpl>(
info.origin, storage_partition->GetContentIndexContext()),
info.origin, storage_partition->GetContentIndexContext(),
storage_partition->GetServiceWorkerContext()),
std::move(receiver));
}
ContentIndexServiceImpl::ContentIndexServiceImpl(
const url::Origin& origin,
scoped_refptr<ContentIndexContextImpl> content_index_context)
scoped_refptr<ContentIndexContextImpl> content_index_context,
scoped_refptr<ServiceWorkerContextWrapper> service_worker_context)
: origin_(origin),
content_index_context_(std::move(content_index_context)) {
content_index_context_(std::move(content_index_context)),
service_worker_context_(std::move(service_worker_context)) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
}
......@@ -74,6 +97,18 @@ void ContentIndexServiceImpl::GetIconSizes(
content_index_context_->GetIconSizes(category, std::move(callback));
}
void ContentIndexServiceImpl::CheckOfflineCapability(
int64_t service_worker_registration_id,
const GURL& launch_url,
CheckOfflineCapabilityCallback callback) {
// TODO(rayankans): Figure out if we can check the service worker specified
// by |service_worker_registration_id| rather than any service worker.
service_worker_context_->CheckOfflineCapability(
launch_url,
base::BindOnce(&DidCheckOfflineCapability, std::move(callback),
service_worker_registration_id));
}
void ContentIndexServiceImpl::Add(
int64_t service_worker_registration_id,
blink::mojom::ContentDescriptionPtr description,
......
......@@ -7,6 +7,7 @@
#include "base/memory/scoped_refptr.h"
#include "content/browser/content_index/content_index_context_impl.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/common/content_export.h"
#include "third_party/blink/public/mojom/content_index/content_index.mojom.h"
#include "url/origin.h"
......@@ -33,12 +34,16 @@ class CONTENT_EXPORT ContentIndexServiceImpl
ContentIndexServiceImpl(
const url::Origin& origin,
scoped_refptr<ContentIndexContextImpl> content_index_context);
scoped_refptr<ContentIndexContextImpl> content_index_context,
scoped_refptr<ServiceWorkerContextWrapper> service_worker_context);
~ContentIndexServiceImpl() override;
// blink::mojom::ContentIndexService implementation.
void GetIconSizes(blink::mojom::ContentCategory category,
GetIconSizesCallback callback) override;
void CheckOfflineCapability(int64_t service_worker_registration_id,
const GURL& launch_url,
CheckOfflineCapabilityCallback callback) override;
void Add(int64_t service_worker_registration_id,
blink::mojom::ContentDescriptionPtr description,
const std::vector<SkBitmap>& icons,
......@@ -53,6 +58,7 @@ class CONTENT_EXPORT ContentIndexServiceImpl
private:
url::Origin origin_;
scoped_refptr<ContentIndexContextImpl> content_index_context_;
scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
DISALLOW_COPY_AND_ASSIGN(ContentIndexServiceImpl);
};
......
......@@ -31,7 +31,8 @@ class ContentIndexServiceImplTest : public ::testing::Test {
ContentIndexServiceImplTest()
: service_(std::make_unique<ContentIndexServiceImpl>(
url::Origin::Create(GURL(kOrigin)),
/* content_index_context= */ nullptr)) {}
/* content_index_context= */ nullptr,
/* service_worker_context= */ nullptr)) {}
~ContentIndexServiceImplTest() override = default;
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
self.addEventListener('fetch', event => {
if (event.request.url.includes('/forcefail'))
event.respondWith(Promise.reject());
else if (event.request.url.includes('/forcesuccess'))
event.respondWith(new Response(null, {status: 200}));
else
event.respondWith(fetch(event.request));
});
\ No newline at end of file
......@@ -9,7 +9,7 @@
<script>
navigator.serviceWorker.register('sw.js');
async function addContent(id, icons = []) {
async function addContent(id, icons = [], url = '/content_index/test.html') {
try {
const registration = await navigator.serviceWorker.ready;
await registration.index.add({
......@@ -18,7 +18,7 @@
description: 'Description!',
category: 'article',
icons,
url: '/content_index/test.html',
url,
});
sendResultToTest('ok');
} catch (e) {
......
......@@ -73,6 +73,14 @@ interface ContentIndexService {
// Returns how many icons are needed and their sizes (in pixels).
GetIconSizes(ContentCategory category) => (array<gfx.mojom.Size> icon_sizes);
// Checks whether the provided url is offline-capable, i.e the ServiceWorker
// (identified by |service_worker_registration_id|) returns a 200 status when
// trying to navigate to |launch_url| while offline. Returns false if
// |launch_url| is handled by any other ServiceWorker.
CheckOfflineCapability(int64 service_worker_registration_id,
url.mojom.Url launch_url)
=> (bool is_offline_capable);
Add(int64 service_worker_registration_id,
ContentDescription description,
array<skia.mojom.Bitmap> icon,
......
......@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/modules/content_index/content_index.h"
#include "base/feature_list.h"
#include "base/optional.h"
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
#include "third_party/blink/public/platform/web_size.h"
......@@ -19,6 +20,15 @@
#include "third_party/blink/renderer/platform/heap/persistent.h"
#include "third_party/blink/renderer/platform/weborigin/security_origin.h"
namespace features {
// If enabled, registering content index entries will perform a check
// to see if the provided launch url is offline-capable.
const base::Feature kContentIndexCheckOffline{
"ContentIndexCheckOffline", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace features
namespace blink {
namespace {
......@@ -145,6 +155,36 @@ void ContentIndex::DidGetIcons(ScriptPromiseResolver* resolver,
KURL launch_url = registration_->GetExecutionContext()->CompleteURL(
description->launch_url);
if (base::FeatureList::IsEnabled(features::kContentIndexCheckOffline)) {
GetService()->CheckOfflineCapability(
registration_->RegistrationId(), launch_url,
WTF::Bind(&ContentIndex::DidCheckOfflineCapability,
WrapPersistent(this), WrapPersistent(resolver), launch_url,
std::move(description), std::move(icons)));
return;
}
DidCheckOfflineCapability(resolver, std::move(launch_url),
std::move(description), std::move(icons),
/* is_offline_capable= */ true);
}
void ContentIndex::DidCheckOfflineCapability(
ScriptPromiseResolver* resolver,
KURL launch_url,
mojom::blink::ContentDescriptionPtr description,
Vector<SkBitmap> icons,
bool is_offline_capable) {
ScriptState* script_state = resolver->GetScriptState();
ScriptState::Scope scope(script_state);
if (!is_offline_capable) {
resolver->Reject(V8ThrowException::CreateTypeError(
script_state->GetIsolate(),
"The provided launch URL is not offline-capable."));
return;
}
GetService()->Add(registration_->RegistrationId(), std::move(description),
icons, launch_url,
WTF::Bind(&ContentIndex::DidAdd, WrapPersistent(this),
......
......@@ -52,6 +52,12 @@ class ContentIndex final : public ScriptWrappable {
void DidGetIcons(ScriptPromiseResolver* resolver,
mojom::blink::ContentDescriptionPtr description,
Vector<SkBitmap> icons);
void DidCheckOfflineCapability(
ScriptPromiseResolver* resolver,
KURL launch_url,
mojom::blink::ContentDescriptionPtr description,
Vector<SkBitmap> icons,
bool is_offline_capable);
void DidAdd(ScriptPromiseResolver* resolver,
mojom::blink::ContentIndexError error);
void DidDeleteDescription(ScriptPromiseResolver* resolver,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment