Commit e2dce251 authored by Hidehiko Abe's avatar Hidehiko Abe Committed by Commit Bot

Remove LocalActivityResolver.

Now ArcIntentHelperBridge can be obtained via ArcServiceManager.
So, LocalActivityResolver can be merged into ArcServiceManager
to get rid of unnecessary reference from ArcServiceManager to it.

BUG=739097
TEST=Ran trybot.

Change-Id: Idaee93706e14b268feb28d98811f7b290de2165e
Reviewed-on: https://chromium-review.googlesource.com/566785Reviewed-by: default avatarYusuke Sato <yusukes@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485858}
parent 27d2945c
......@@ -112,8 +112,8 @@ void ArcServiceLauncher::Initialize() {
base::MakeUnique<ArcFileSystemMounter>(arc_bridge_service));
arc_service_manager_->AddService(
base::MakeUnique<ArcImeService>(arc_bridge_service));
arc_service_manager_->AddService(base::MakeUnique<ArcIntentHelperBridge>(
arc_bridge_service, arc_service_manager_->activity_resolver()));
arc_service_manager_->AddService(
base::MakeUnique<ArcIntentHelperBridge>(arc_bridge_service));
arc_service_manager_->AddService(
base::MakeUnique<ArcMetricsService>(arc_bridge_service));
arc_service_manager_->AddService(
......
......@@ -13,7 +13,6 @@
#include "components/arc/arc_bridge_service.h"
#include "components/arc/arc_service_manager.h"
#include "components/arc/intent_helper/arc_intent_helper_bridge.h"
#include "components/arc/intent_helper/local_activity_resolver.h"
#include "components/arc/intent_helper/page_transition_util.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_controller.h"
......@@ -179,9 +178,12 @@ ArcNavigationThrottle::HandleRequest() {
if (!arc_service_manager)
return content::NavigationThrottle::PROCEED;
scoped_refptr<LocalActivityResolver> local_resolver =
arc_service_manager->activity_resolver();
if (local_resolver->ShouldChromeHandleUrl(url)) {
auto* intent_helper_bridge =
arc_service_manager->GetService<ArcIntentHelperBridge>();
if (!intent_helper_bridge)
return content::NavigationThrottle::PROCEED;
if (intent_helper_bridge->ShouldChromeHandleUrl(url)) {
// Allow navigation to proceed if there isn't an android app that handles
// the given URL.
return content::NavigationThrottle::PROCEED;
......
......@@ -43,8 +43,6 @@ static_library("arc") {
"intent_helper/intent_filter.h",
"intent_helper/link_handler_model_impl.cc",
"intent_helper/link_handler_model_impl.h",
"intent_helper/local_activity_resolver.cc",
"intent_helper/local_activity_resolver.h",
"intent_helper/page_transition_util.cc",
"intent_helper/page_transition_util.h",
"kiosk/arc_kiosk_bridge.cc",
......@@ -225,7 +223,6 @@ source_set("unit_tests") {
"intent_helper/font_size_util_unittest.cc",
"intent_helper/intent_filter_unittest.cc",
"intent_helper/link_handler_model_impl_unittest.cc",
"intent_helper/local_activity_resolver_unittest.cc",
"intent_helper/page_transition_util_unittest.cc",
"kiosk/arc_kiosk_bridge_unittest.cc",
]
......
......@@ -20,8 +20,7 @@ ArcServiceManager* g_arc_service_manager = nullptr;
} // namespace
ArcServiceManager::ArcServiceManager()
: arc_bridge_service_(base::MakeUnique<ArcBridgeService>()),
activity_resolver_(new LocalActivityResolver()) {
: arc_bridge_service_(base::MakeUnique<ArcBridgeService>()) {
DCHECK(!g_arc_service_manager);
g_arc_service_manager = this;
}
......@@ -75,7 +74,6 @@ ArcService* ArcServiceManager::GetNamedServiceInternal(
void ArcServiceManager::Shutdown() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
activity_resolver_ = nullptr;
services_.clear();
}
......
......@@ -16,7 +16,6 @@
#include "base/memory/ref_counted.h"
#include "base/threading/thread_checker.h"
#include "components/arc/arc_service.h"
#include "components/arc/intent_helper/local_activity_resolver.h"
namespace content {
class BrowserContext;
......@@ -116,14 +115,7 @@ class ArcServiceManager {
// Called to shut down all ARC services.
void Shutdown();
// Returns the activity resolver owned by ArcServiceManager.
scoped_refptr<LocalActivityResolver> activity_resolver() {
return activity_resolver_;
}
private:
class IntentHelperObserverImpl; // implemented in arc_service_manager.cc.
// Helper methods for AddService and GetService.
bool AddServiceInternal(const std::string& name,
std::unique_ptr<ArcService> service);
......@@ -133,7 +125,6 @@ class ArcServiceManager {
std::unique_ptr<ArcBridgeService> arc_bridge_service_;
std::unordered_multimap<std::string, std::unique_ptr<ArcService>> services_;
scoped_refptr<LocalActivityResolver> activity_resolver_;
// This holds the pointer to the BrowserContext (practically Profile)
// which is allowed to use ARC.
......
......@@ -15,7 +15,6 @@
#include "components/arc/arc_service_manager.h"
#include "components/arc/audio/arc_audio_bridge.h"
#include "components/arc/intent_helper/link_handler_model_impl.h"
#include "components/arc/intent_helper/local_activity_resolver.h"
#include "ui/base/layout.h"
#include "url/gurl.h"
......@@ -29,12 +28,8 @@ const char ArcIntentHelperBridge::kArcServiceName[] =
const char ArcIntentHelperBridge::kArcIntentHelperPackageName[] =
"org.chromium.arc.intent_helper";
ArcIntentHelperBridge::ArcIntentHelperBridge(
ArcBridgeService* bridge_service,
const scoped_refptr<LocalActivityResolver>& activity_resolver)
: ArcService(bridge_service),
binding_(this),
activity_resolver_(activity_resolver) {
ArcIntentHelperBridge::ArcIntentHelperBridge(ArcBridgeService* bridge_service)
: ArcService(bridge_service), binding_(this) {
arc_bridge_service()->intent_helper()->AddObserver(this);
}
......@@ -100,6 +95,21 @@ ArcIntentHelperBridge::GetResult ArcIntentHelperBridge::GetActivityIcons(
return icon_loader_.GetActivityIcons(activities, callback);
}
bool ArcIntentHelperBridge::ShouldChromeHandleUrl(const GURL& url) {
if (!url.SchemeIsHTTPOrHTTPS()) {
// Chrome will handle everything that is not http and https.
return true;
}
for (const IntentFilter& filter : intent_filters_) {
if (filter.Match(url))
return false;
}
// Didn't find any matches for Android so let Chrome handle it.
return true;
}
void ArcIntentHelperBridge::AddObserver(ArcIntentHelperObserver* observer) {
observer_list_.AddObserver(observer);
}
......@@ -139,7 +149,7 @@ ArcIntentHelperBridge::FilterOutIntentHelper(
void ArcIntentHelperBridge::OnIntentFiltersUpdated(
std::vector<IntentFilter> filters) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
activity_resolver_->UpdateIntentFilters(std::move(filters));
intent_filters_ = std::move(filters);
for (auto& observer : observer_list_)
observer.OnIntentFiltersUpdated();
......
......@@ -11,7 +11,6 @@
#include "ash/link_handler_model_factory.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/observer_list.h"
#include "base/threading/thread_checker.h"
#include "components/arc/arc_service.h"
......@@ -31,7 +30,6 @@ namespace arc {
class ArcBridgeService;
class IntentFilter;
class LocalActivityResolver;
// Receives intents from ARC.
class ArcIntentHelperBridge
......@@ -40,9 +38,7 @@ class ArcIntentHelperBridge
public mojom::IntentHelperHost,
public ash::LinkHandlerModelFactory {
public:
ArcIntentHelperBridge(
ArcBridgeService* bridge_service,
const scoped_refptr<LocalActivityResolver>& activity_resolver);
explicit ArcIntentHelperBridge(ArcBridgeService* bridge_service);
~ArcIntentHelperBridge() override;
void AddObserver(ArcIntentHelperObserver* observer);
......@@ -73,6 +69,14 @@ class ArcIntentHelperBridge
GetResult GetActivityIcons(const std::vector<ActivityName>& activities,
const OnIconsReadyCallback& callback);
// Returns true when |url| can only be handled by Chrome. Otherwise, which is
// when there might be one or more ARC apps that can handle |url|, returns
// false. This function synchronously checks the |url| without making any IPC
// to ARC side. Note that this function only supports http and https. If url's
// scheme is neither http nor https, the function immediately returns true
// without checking the filters.
bool ShouldChromeHandleUrl(const GURL& url);
// ash::LinkHandlerModelFactory
std::unique_ptr<ash::LinkHandlerModel> CreateModel(const GURL& url) override;
......@@ -90,11 +94,14 @@ class ArcIntentHelperBridge
static const char kArcIntentHelperPackageName[];
private:
THREAD_CHECKER(thread_checker_);
mojo::Binding<mojom::IntentHelperHost> binding_;
internal::ActivityIconLoader icon_loader_;
scoped_refptr<LocalActivityResolver> activity_resolver_;
THREAD_CHECKER(thread_checker_);
// List of intent filters from Android. Used to determine if Chrome should
// handle a URL without handing off to Android.
std::vector<IntentFilter> intent_filters_;
base::ObserverList<ArcIntentHelperObserver> observer_list_;
......
......@@ -9,33 +9,38 @@
#include "base/memory/ptr_util.h"
#include "components/arc/arc_bridge_service.h"
#include "components/arc/common/intent_helper.mojom.h"
#include "components/arc/intent_helper/local_activity_resolver.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace arc {
namespace {
IntentFilter GetIntentFilter(const std::string& host) {
std::vector<IntentFilter::AuthorityEntry> authorities;
authorities.emplace_back(host, -1);
// TODO
return IntentFilter(std::move(authorities),
std::vector<IntentFilter::PatternMatcher>());
}
class ArcIntentHelperTest : public testing::Test {
public:
ArcIntentHelperTest() = default;
protected:
std::unique_ptr<ArcBridgeService> arc_bridge_service_;
scoped_refptr<LocalActivityResolver> activity_resolver_;
std::unique_ptr<ArcIntentHelperBridge> instance_;
private:
void SetUp() override {
arc_bridge_service_ = base::MakeUnique<ArcBridgeService>();
activity_resolver_ = new LocalActivityResolver();
instance_ = base::MakeUnique<ArcIntentHelperBridge>(
arc_bridge_service_.get(), activity_resolver_);
instance_ =
base::MakeUnique<ArcIntentHelperBridge>(arc_bridge_service_.get());
}
void TearDown() override {
instance_.reset();
activity_resolver_ = nullptr;
arc_bridge_service_.reset();
}
......@@ -161,4 +166,92 @@ TEST_F(ArcIntentHelperTest, TestObserver) {
EXPECT_FALSE(observer->IsUpdated());
}
// Tests that ShouldChromeHandleUrl returns true by default.
TEST_F(ArcIntentHelperTest, TestDefault) {
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("file:///etc/password")));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("chrome://help")));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("about://chrome")));
}
// Tests that ShouldChromeHandleUrl returns false when there's a match.
TEST_F(ArcIntentHelperTest, TestSingleFilter) {
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
instance_->OnIntentFiltersUpdated(std::move(array));
EXPECT_FALSE(instance_->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_TRUE(
instance_->ShouldChromeHandleUrl(GURL("https://www.google.co.uk")));
}
// Tests the same with multiple filters.
TEST_F(ArcIntentHelperTest, TestMultipleFilters) {
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
array.emplace_back(GetIntentFilter("www.google.co.uk"));
array.emplace_back(GetIntentFilter("dev.chromium.org"));
instance_->OnIntentFiltersUpdated(std::move(array));
EXPECT_FALSE(instance_->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("http://www.google.co.uk")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://www.google.co.uk")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("http://dev.chromium.org")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://dev.chromium.org")));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("http://www.android.com")));
}
// Tests that ShouldChromeHandleUrl returns true for non http(s) URLs.
TEST_F(ArcIntentHelperTest, TestNonHttp) {
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
instance_->OnIntentFiltersUpdated(std::move(array));
EXPECT_TRUE(
instance_->ShouldChromeHandleUrl(GURL("chrome://www.google.com")));
EXPECT_TRUE(
instance_->ShouldChromeHandleUrl(GURL("custom://www.google.com")));
}
// Tests that ShouldChromeHandleUrl discards the previous filters when
// UpdateIntentFilters is called with new ones.
TEST_F(ArcIntentHelperTest, TestMultipleUpdate) {
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
array.emplace_back(GetIntentFilter("dev.chromium.org"));
instance_->OnIntentFiltersUpdated(std::move(array));
std::vector<IntentFilter> array2;
array2.emplace_back(GetIntentFilter("www.google.co.uk"));
array2.emplace_back(GetIntentFilter("dev.chromium.org"));
array2.emplace_back(GetIntentFilter("www.android.com"));
instance_->OnIntentFiltersUpdated(std::move(array2));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_TRUE(instance_->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("http://www.google.co.uk")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://www.google.co.uk")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("http://dev.chromium.org")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://dev.chromium.org")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("http://www.android.com")));
EXPECT_FALSE(
instance_->ShouldChromeHandleUrl(GURL("https://www.android.com")));
}
} // namespace arc
// Copyright 2016 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.
#include "components/arc/intent_helper/local_activity_resolver.h"
#include <utility>
#include "url/gurl.h"
namespace arc {
LocalActivityResolver::LocalActivityResolver() = default;
LocalActivityResolver::~LocalActivityResolver() = default;
bool LocalActivityResolver::ShouldChromeHandleUrl(const GURL& url) {
if (!url.SchemeIsHTTPOrHTTPS()) {
// Chrome will handle everything that is not http and https.
return true;
}
for (const IntentFilter& filter : intent_filters_) {
if (filter.Match(url))
return false;
}
// Didn't find any matches for Android so let Chrome handle it.
return true;
}
void LocalActivityResolver::UpdateIntentFilters(
std::vector<IntentFilter> intent_filters) {
intent_filters_ = std::move(intent_filters);
}
} // namespace arc
// Copyright 2016 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.
#ifndef COMPONENTS_ARC_INTENT_HELPER_LOCAL_ACTIVITY_RESOLVER_H_
#define COMPONENTS_ARC_INTENT_HELPER_LOCAL_ACTIVITY_RESOLVER_H_
#include <vector>
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "components/arc/common/intent_helper.mojom.h"
#include "components/arc/intent_helper/intent_filter.h"
class GURL;
namespace arc {
class LocalActivityResolver : public base::RefCounted<LocalActivityResolver> {
public:
LocalActivityResolver();
// Returns true when |url| can only be handled by Chrome. Otherwise, which is
// when there might be one or more ARC apps that can handle |url|, returns
// false. This function synchronously checks the |url| without making any IPC
// to ARC side. Note that this function only supports http and https. If url's
// scheme is neither http nor https, the function immediately returns true
// without checking the filters.
bool ShouldChromeHandleUrl(const GURL& url);
// Called when the list of intent filters on ARC side is updated.
void UpdateIntentFilters(std::vector<IntentFilter> intent_filters);
private:
friend class base::RefCounted<LocalActivityResolver>;
~LocalActivityResolver();
// List of intent filters from Android. Used to determine if Chrome should
// handle a URL without handing off to Android.
std::vector<IntentFilter> intent_filters_;
DISALLOW_COPY_AND_ASSIGN(LocalActivityResolver);
};
} // namespace arc
#endif // COMPONENTS_ARC_INTENT_HELPER_LOCAL_ACTIVITY_RESOLVER_H_
// Copyright 2016 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.
#include "components/arc/intent_helper/local_activity_resolver.h"
#include <string>
#include <utility>
#include "components/arc/intent_helper/intent_filter.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace arc {
namespace {
IntentFilter GetIntentFilter(const std::string& host) {
std::vector<IntentFilter::AuthorityEntry> authorities;
authorities.emplace_back(host, -1);
return IntentFilter(std::move(authorities),
std::vector<IntentFilter::PatternMatcher>());
}
} // namespace
// Tests that ShouldChromeHandleUrl returns true by default.
TEST(LocalActivityResolverTest, TestDefault) {
scoped_refptr<LocalActivityResolver> resolver(new LocalActivityResolver());
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("file:///etc/password")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("chrome://help")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("about://chrome")));
}
// Tests that ShouldChromeHandleUrl returns false when there's a match.
TEST(LocalActivityResolverTest, TestSingleFilter) {
scoped_refptr<LocalActivityResolver> resolver(new LocalActivityResolver());
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
resolver->UpdateIntentFilters(std::move(array));
EXPECT_FALSE(resolver->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_FALSE(resolver->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_TRUE(
resolver->ShouldChromeHandleUrl(GURL("https://www.google.co.uk")));
}
// Tests the same with multiple filters.
TEST(LocalActivityResolverTest, TestMultipleFilters) {
scoped_refptr<LocalActivityResolver> resolver(new LocalActivityResolver());
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
array.emplace_back(GetIntentFilter("www.google.co.uk"));
array.emplace_back(GetIntentFilter("dev.chromium.org"));
resolver->UpdateIntentFilters(std::move(array));
EXPECT_FALSE(resolver->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_FALSE(resolver->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("http://www.google.co.uk")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("https://www.google.co.uk")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("http://dev.chromium.org")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("https://dev.chromium.org")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("http://www.android.com")));
}
// Tests that ShouldChromeHandleUrl returns true for non http(s) URLs.
TEST(LocalActivityResolverTest, TestNonHttp) {
scoped_refptr<LocalActivityResolver> resolver(new LocalActivityResolver());
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
resolver->UpdateIntentFilters(std::move(array));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("chrome://www.google.com")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("custom://www.google.com")));
}
// Tests that ShouldChromeHandleUrl discards the previous filters when
// UpdateIntentFilters is called with new ones.
TEST(LocalActivityResolverTest, TestMultipleUpdate) {
scoped_refptr<LocalActivityResolver> resolver(new LocalActivityResolver());
std::vector<IntentFilter> array;
array.emplace_back(GetIntentFilter("www.google.com"));
array.emplace_back(GetIntentFilter("dev.chromium.org"));
resolver->UpdateIntentFilters(std::move(array));
std::vector<IntentFilter> array2;
array2.emplace_back(GetIntentFilter("www.google.co.uk"));
array2.emplace_back(GetIntentFilter("dev.chromium.org"));
array2.emplace_back(GetIntentFilter("www.android.com"));
resolver->UpdateIntentFilters(std::move(array2));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("http://www.google.com")));
EXPECT_TRUE(resolver->ShouldChromeHandleUrl(GURL("https://www.google.com")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("http://www.google.co.uk")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("https://www.google.co.uk")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("http://dev.chromium.org")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("https://dev.chromium.org")));
EXPECT_FALSE(resolver->ShouldChromeHandleUrl(GURL("http://www.android.com")));
EXPECT_FALSE(
resolver->ShouldChromeHandleUrl(GURL("https://www.android.com")));
}
} // namespace arc
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