Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(791)

Unified Diff: chrome/browser/chromeos/arc/arc_navigation_throttle.cc

Issue 2357053002: Always use arc::InstanceHolder<T>::GetInstanceForMethod (Closed)
Patch Set: rebased to catch up tot Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/chromeos/arc/arc_navigation_throttle.cc
diff --git a/chrome/browser/chromeos/arc/arc_navigation_throttle.cc b/chrome/browser/chromeos/arc/arc_navigation_throttle.cc
index 4db4b12a3b23a32ff0badce1e5f90f7602386134..ae7f574fe6c1e60912ccdffd467c264ff4ee390d 100644
--- a/chrome/browser/chromeos/arc/arc_navigation_throttle.cc
+++ b/chrome/browser/chromeos/arc/arc_navigation_throttle.cc
@@ -27,7 +27,9 @@ namespace arc {
namespace {
-constexpr int kMinInstanceVersion = 7;
+constexpr uint32_t kMinVersionForHandleUrl = 2;
+constexpr uint32_t kMinVersionForRequestUrlHandlerList = 2;
+constexpr uint32_t kMinVersionForAddPreferredPackage = 7;
scoped_refptr<ActivityIconLoader> GetIconLoader() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -111,9 +113,9 @@ ArcNavigationThrottle::HandleRequest() {
if (!ShouldOverrideUrlLoading(previous_url, current_url))
return content::NavigationThrottle::PROCEED;
- arc::ArcServiceManager* arc_service_manager = arc::ArcServiceManager::Get();
+ ArcServiceManager* arc_service_manager = ArcServiceManager::Get();
DCHECK(arc_service_manager);
- scoped_refptr<arc::LocalActivityResolver> local_resolver =
+ scoped_refptr<LocalActivityResolver> local_resolver =
arc_service_manager->activity_resolver();
if (local_resolver->ShouldChromeHandleUrl(url)) {
// Allow navigation to proceed if there isn't an android app that handles
@@ -121,11 +123,11 @@ ArcNavigationThrottle::HandleRequest() {
return content::NavigationThrottle::PROCEED;
}
- mojom::IntentHelperInstance* bridge_instance =
- arc::ArcIntentHelperBridge::GetIntentHelperInstance(kMinInstanceVersion);
- if (!bridge_instance)
+ auto* instance = ArcIntentHelperBridge::GetIntentHelperInstance(
+ "RequestUrlHandlerList", kMinVersionForRequestUrlHandlerList);
+ if (!instance)
return content::NavigationThrottle::PROCEED;
- bridge_instance->RequestUrlHandlerList(
+ instance->RequestUrlHandlerList(
url.spec(), base::Bind(&ArcNavigationThrottle::OnAppCandidatesReceived,
weak_ptr_factory_.GetWeakPtr()));
return content::NavigationThrottle::DEFER;
@@ -227,11 +229,11 @@ void ArcNavigationThrottle::OnIntentPickerClosed(
previous_user_action_ = close_reason;
- mojom::IntentHelperInstance* bridge =
- arc::ArcIntentHelperBridge::GetIntentHelperInstance(kMinInstanceVersion);
- if (!bridge || selected_app_index >= handlers.size()) {
+ // Make sure that the instance at least supports HandleUrl.
+ auto* instance = ArcIntentHelperBridge::GetIntentHelperInstance(
+ "HandleUrl", kMinVersionForHandleUrl);
Luis Héctor Chávez 2016/09/23 22:51:03 nit: Given that version 7 was added more than thre
Yusuke Sato 2016/09/24 00:15:22 Yes that's true, but in terms of safety, probably
Luis Héctor Chávez 2016/09/24 02:00:00 Fair enough.
+ if (!instance || selected_app_index >= handlers.size())
close_reason = CloseReason::ERROR;
- }
switch (close_reason) {
case CloseReason::ERROR:
@@ -244,7 +246,12 @@ void ArcNavigationThrottle::OnIntentPickerClosed(
break;
}
case CloseReason::ALWAYS_PRESSED: {
- bridge->AddPreferredPackage(handlers[selected_app_index]->package_name);
+ // Call AddPreferredPackage if it is supported.
+ if (ArcIntentHelperBridge::GetIntentHelperInstance(
+ "AddPreferredPackage", kMinVersionForAddPreferredPackage)) {
+ instance->AddPreferredPackage(
+ handlers[selected_app_index]->package_name);
+ }
// fall through.
}
case CloseReason::JUST_ONCE_PRESSED:
@@ -253,8 +260,8 @@ void ArcNavigationThrottle::OnIntentPickerClosed(
handlers[selected_app_index]->package_name)) {
handle->Resume();
} else {
- bridge->HandleUrl(url.spec(),
- handlers[selected_app_index]->package_name);
+ instance->HandleUrl(url.spec(),
+ handlers[selected_app_index]->package_name);
handle->CancelDeferredNavigation(
content::NavigationThrottle::CANCEL_AND_IGNORE);
if (handle->GetWebContents()->GetController().IsInitialNavigation())

Powered by Google App Engine
This is Rietveld 408576698