Skip to content

Commit

Permalink
[Messages] Introduce MessageDispatcherBridge::MapToJavaDrawableId
Browse files Browse the repository at this point in the history
MessageDispatcherBridge::MapToJavaDrawableId allows mapping native
resource ID to Java Drawable resource ID for consumers that don't have
access to ResourceMapper (e.g. when implementation is in //components).

BUG=1233696
[email protected]

Change-Id: I4b8396fc6212cc4fe3d52c88a71275b9ae618892
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3087808
Reviewed-by: Lijin Shen <[email protected]>
Reviewed-by: Theresa  <[email protected]>
Reviewed-by: Charlie Harrison <[email protected]>
Commit-Queue: Pavel Yatsuk <[email protected]>
Cr-Commit-Position: refs/heads/master@{#912753}
  • Loading branch information
Pavel Yatsuk authored and Chromium LUCI CQ committed Aug 17, 2021
1 parent 4128659 commit 9947bf8
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 16 deletions.
1 change: 1 addition & 0 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ android_library("chrome_java") {
"//chrome/browser/android/browserservices/verification:java",
"//chrome/browser/android/crypto:java",
"//chrome/browser/android/lifecycle:java",
"//chrome/browser/android/messages:java",
"//chrome/browser/android/webapps/launchpad:java",
"//chrome/browser/attribution_reporting/android:factory_java",
"//chrome/browser/attribution_reporting/android:java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import org.chromium.chrome.browser.messages.ChromeMessageAutodismissDurationProvider;
import org.chromium.chrome.browser.messages.ChromeMessageQueueMediator;
import org.chromium.chrome.browser.messages.MessageContainerCoordinator;
import org.chromium.chrome.browser.messages.MessagesResourceMapperInitializer;
import org.chromium.chrome.browser.omnibox.OmniboxFocusReason;
import org.chromium.chrome.browser.omnibox.geo.GeolocationHeader;
import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler;
Expand Down Expand Up @@ -605,6 +606,7 @@ public void onFinishNativeInitialization() {
// TODO(crbug.com/1185887): Move feature flag and parameters into a separate class in
// the Messages component.
if (ChromeFeatureList.isEnabled(ChromeFeatureList.MESSAGES_FOR_ANDROID_INFRASTRUCTURE)) {
MessagesResourceMapperInitializer.init();
MessageContainer container = mActivity.findViewById(R.id.message_container);
mMessageContainerCoordinator =
new MessageContainerCoordinator(container, mBrowserControlsManager);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2782,6 +2782,7 @@ static_library("browser") {
"android/logo_bridge.cc",
"android/logo_bridge.h",
"android/media/media_capture_devices_dispatcher_android.cc",
"android/messages_resource_mapper_initializer.cc",
"android/metrics/android_session_durations_service.cc",
"android/metrics/android_session_durations_service.h",
"android/metrics/android_session_durations_service_factory.cc",
Expand Down Expand Up @@ -3254,6 +3255,7 @@ static_library("browser") {
":usage_stats_proto",
"//chrome/android:jni_headers",
"//chrome/android/modules/extra_icu/provider:native",
"//chrome/browser/android/messages:jni_headers",
"//chrome/browser/android/metrics:jni_headers",
"//chrome/browser/banners/android:jni_headers",
"//chrome/browser/commerce:feature_list",
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/android/messages/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2021 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.

import("//build/config/android/rules.gni")

android_library("java") {
sources = [ "java/src/org/chromium/chrome/browser/messages/MessagesResourceMapperInitializer.java" ]

annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]

deps = [ "//base:base_java" ]
}

generate_jni("jni_headers") {
sources = [ "java/src/org/chromium/chrome/browser/messages/MessagesResourceMapperInitializer.java" ]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2021 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.

package org.chromium.chrome.browser.messages;

import org.chromium.base.annotations.NativeMethods;

/** Helper class to initialize ResourceIdMapper for MessageDispatcherBridge. */
public class MessagesResourceMapperInitializer {
/** Calls native method to initialize ResourceIdMapper for MessageDispatcherBridge. */
public static void init() {
MessagesResourceMapperInitializerJni.get().init();
}

@NativeMethods
interface Natives {
void init();
}
}
15 changes: 15 additions & 0 deletions chrome/browser/android/messages_resource_mapper_initializer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2021 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 "chrome/browser/android/messages/jni_headers/MessagesResourceMapperInitializer_jni.h"
#include "chrome/browser/android/resource_mapper.h"
#include "components/messages/android/message_dispatcher_bridge.h"

// Sets up a callback for MessageDispatcherBridge ResourceIdMapper. This is done
// in chrome/browser/android because ResourceMapper is not available in
// components.
void JNI_MessagesResourceMapperInitializer_Init(JNIEnv* env) {
messages::MessageDispatcherBridge::Get()->SetResourceIdMapper(
base::BindRepeating(&ResourceMapper::MapToJavaDrawableId));
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "chrome/browser/ui/blocked_content/chrome_popup_navigation_delegate.h"

#include "build/build_config.h"
#include "chrome/browser/android/resource_mapper.h"
#include "chrome/browser/content_settings/chrome_content_settings_utils.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/ui/browser_navigator.h"
Expand Down Expand Up @@ -82,7 +81,6 @@ void ChromePopupNavigationDelegate::OnPopupBlocked(
total_popups_blocked_on_page,
HostContentSettingsMapFactory::GetForProfile(
web_contents->GetBrowserContext()),
base::BindRepeating(&ResourceMapper::MapToJavaDrawableId),
base::BindOnce(
&content_settings::RecordPopupsAction,
content_settings::POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ namespace blocked_content {
bool PopupBlockedMessageDelegate::ShowMessage(
int num_popups,
HostContentSettingsMap* settings_map,
base::RepeatingCallback<int(int)> resource_id_mapper,
base::OnceClosure on_show_popups_callback) {
if (message_ != nullptr) { // update title only
message_->SetTitle(l10n_util::GetPluralStringFUTF16(
Expand Down Expand Up @@ -48,13 +47,15 @@ bool PopupBlockedMessageDelegate::ShowMessage(
// is managed by policy.
int button_text_id = allow_settings_changes_ ? IDS_SHOW_CONTENT : IDS_OK;
message->SetPrimaryButtonText(l10n_util::GetStringUTF16(button_text_id));
message->SetIconResourceId(
resource_id_mapper.Run(IDR_ANDROID_INFOBAR_BLOCKED_POPUPS));
messages::MessageDispatcherBridge* message_dispatcher_bridge =
messages::MessageDispatcherBridge::Get();
message->SetIconResourceId(message_dispatcher_bridge->MapToJavaDrawableId(
IDR_ANDROID_INFOBAR_BLOCKED_POPUPS));

// On rare occasions, such as the moment when activity is being recreated
// or destroyed, popup blocked message will not be displayed and the
// method will return false.
if (!messages::MessageDispatcherBridge::Get()->EnqueueMessage(
if (!message_dispatcher_bridge->EnqueueMessage(
message.get(), web_contents_, messages::MessageScopeType::NAVIGATION,
messages::MessagePriority::kNormal)) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class PopupBlockedMessageDelegate
public:
bool ShowMessage(int num_popups,
HostContentSettingsMap* settings_map,
base::RepeatingCallback<int(int)> resource_id_mapper,
base::OnceClosure on_accept_callback);

~PopupBlockedMessageDelegate() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ class PopupBlockedMessageDelegateTest

HostContentSettingsMap* settings_map() { return settings_map_.get(); }

base::RepeatingCallback<int(int)> GetResourceIdMapper() {
return base::BindRepeating(ResourceMap);
}

bool EnqueueMessage(int num_pops,
base::OnceClosure on_accept_callback,
bool success);
Expand All @@ -54,8 +50,6 @@ class PopupBlockedMessageDelegateTest
return popup_blocked_message_delegate_;
}

static int ResourceMap(int id) { return -1; }

private:
PopupBlockerTabHelper* helper_ = nullptr;
base::test::ScopedFeatureList feature_list_;
Expand Down Expand Up @@ -107,7 +101,6 @@ bool PopupBlockedMessageDelegateTest::EnqueueMessage(
EXPECT_CALL(message_dispatcher_bridge_, EnqueueMessage)
.WillOnce(testing::Return(success));
return GetDelegate()->ShowMessage(num_pops, settings_map(),
GetResourceIdMapper(),
std::move(on_accept_callback));
}

Expand Down Expand Up @@ -135,7 +128,7 @@ TEST_F(PopupBlockedMessageDelegateTest, MessagePropertyValues) {

// Should update title; #EnqueueMessage ensure message is enqueued only once.
GetDelegate()->ShowMessage(num_popups + 1, settings_map(),
GetResourceIdMapper(), base::NullCallback());
base::NullCallback());
EXPECT_EQ(l10n_util::GetPluralStringFUTF16(IDS_POPUPS_BLOCKED_INFOBAR_TEXT,
num_popups + 1),
GetMessageWrapper()->GetTitle());
Expand Down
13 changes: 13 additions & 0 deletions components/messages/android/message_dispatcher_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ void MessageDispatcherBridge::SetInstanceForTesting(
g_message_dospatcher_bridge_for_testing = instance;
}

MessageDispatcherBridge::MessageDispatcherBridge() = default;
MessageDispatcherBridge::~MessageDispatcherBridge() = default;

bool MessageDispatcherBridge::EnqueueMessage(MessageWrapper* message,
content::WebContents* web_contents,
MessageScopeType scope_type,
Expand Down Expand Up @@ -81,4 +84,14 @@ void MessageDispatcherBridge::DismissMessage(MessageWrapper* message,
}
}

int MessageDispatcherBridge::MapToJavaDrawableId(int resource_id) {
DCHECK(resource_id_mapper_);
return resource_id_mapper_.Run(resource_id);
}

void MessageDispatcherBridge::SetResourceIdMapper(
ResourceIdMapper resource_id_mapper) {
resource_id_mapper_ = std::move(resource_id_mapper);
}

} // namespace messages
16 changes: 15 additions & 1 deletion components/messages/android/message_dispatcher_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ namespace messages {
// enqueue/dismiss messages with MessageDispatcher.java.
class MessageDispatcherBridge {
public:
using ResourceIdMapper = base::RepeatingCallback<int(int)>;

static MessageDispatcherBridge* Get();

static void SetInstanceForTesting(MessageDispatcherBridge* instance);

MessageDispatcherBridge();

virtual bool EnqueueMessage(MessageWrapper* message,
content::WebContents* web_contents,
MessageScopeType scope_type,
Expand All @@ -34,8 +38,18 @@ class MessageDispatcherBridge {
content::WebContents* web_contents,
DismissReason dismiss_reason);

// Helper method for mapping native resource id to Java Drawable resource id.
// This is useful for setting icon resource ids in MessageWrapper from the
// code that doesn't have access to ResourceMapper, e.g. code in //components.
virtual int MapToJavaDrawableId(int resource_id);

void SetResourceIdMapper(ResourceIdMapper resource_id_mapper);

protected:
virtual ~MessageDispatcherBridge() = default;
virtual ~MessageDispatcherBridge();

private:
ResourceIdMapper resource_id_mapper_;
};

} // namespace messages
Expand Down
4 changes: 4 additions & 0 deletions components/messages/android/mock_message_dispatcher_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@ namespace messages {
MockMessageDispatcherBridge::MockMessageDispatcherBridge() = default;
MockMessageDispatcherBridge::~MockMessageDispatcherBridge() = default;

int MockMessageDispatcherBridge::MapToJavaDrawableId(int resource_id) {
return -1;
}

} // namespace messages
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class MockMessageDispatcherBridge : public MessageDispatcherBridge {
content::WebContents* web_contents,
DismissReason dismiss_reason),
(override));
int MapToJavaDrawableId(int resource_id) override;
};

} // namespace messages
Expand Down

0 comments on commit 9947bf8

Please sign in to comment.