Commit 6b3c4bdb authored by Owen Min's avatar Owen Min Committed by Commit Bot

Add onPolicyUpdate notification in Java.

Support PolicyService::onPolicyUpdate event in Java. Same as other
functions, only Chrome policies are supported.

Other changes:
1) Implement PolicyMap.equals()
2) Use ObserverList instead of Set to manage PolicyService's observers.

Bug: 1127469
Change-Id: I40c6958e87c746aa40afec306ee632bba52183f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446639
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: default avatarSky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814310}
parent c1074cfd
...@@ -236,10 +236,13 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupport ...@@ -236,10 +236,13 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupport
if (policyService.isInitializationComplete()) { if (policyService.isInitializationComplete()) {
updateCctTosPolicy(); updateCctTosPolicy();
} else { } else {
mPolicyServiceObserver = () -> { mPolicyServiceObserver = new PolicyService.Observer() {
policyService.removeObserver(mPolicyServiceObserver); @Override
mPolicyServiceObserver = null; public void onPolicyServiceInitialized() {
updateCctTosPolicy(); policyService.removeObserver(mPolicyServiceObserver);
mPolicyServiceObserver = null;
updateCctTosPolicy();
}
}; };
policyService.addObserver(mPolicyServiceObserver); policyService.addObserver(mPolicyServiceObserver);
} }
......
...@@ -66,6 +66,11 @@ public class PolicyMap { ...@@ -66,6 +66,11 @@ public class PolicyMap {
return PolicyMapJni.get().getDictValue(mNativePolicyMap, PolicyMap.this, policy); return PolicyMapJni.get().getDictValue(mNativePolicyMap, PolicyMap.this, policy);
} }
public boolean isEqual(PolicyMap other) {
if (this == other) return true;
return PolicyMapJni.get().equals(mNativePolicyMap, PolicyMap.this, other.mNativePolicyMap);
}
@CalledByNative @CalledByNative
private PolicyMap(long nativePolicyMap) { private PolicyMap(long nativePolicyMap) {
mNativePolicyMap = nativePolicyMap; mNativePolicyMap = nativePolicyMap;
...@@ -85,5 +90,7 @@ public class PolicyMap { ...@@ -85,5 +90,7 @@ public class PolicyMap {
String getListValue(long nativePolicyMap, PolicyMap caller, String policy); String getListValue(long nativePolicyMap, PolicyMap caller, String policy);
@NativeClassQualifiedName("PolicyMapAndroid") @NativeClassQualifiedName("PolicyMapAndroid")
String getDictValue(long nativePolicyMap, PolicyMap caller, String policy); String getDictValue(long nativePolicyMap, PolicyMap caller, String policy);
@NativeClassQualifiedName("PolicyMapAndroid")
boolean equals(long nativePolicyMap, PolicyMap caller, long nativeOtherPolicyMap);
} }
} }
...@@ -4,15 +4,12 @@ ...@@ -4,15 +4,12 @@
package org.chromium.components.policy; package org.chromium.components.policy;
import org.chromium.base.CollectionUtil; import org.chromium.base.ObserverList;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeClassQualifiedName; import org.chromium.base.annotations.NativeClassQualifiedName;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import java.util.HashSet;
import java.util.Set;
/** /**
* Wrapper of the native PolicyService class in the Java layer. * Wrapper of the native PolicyService class in the Java layer.
* *
...@@ -28,20 +25,32 @@ import java.util.Set; ...@@ -28,20 +25,32 @@ import java.util.Set;
@JNINamespace("policy::android") @JNINamespace("policy::android")
public class PolicyService { public class PolicyService {
private long mNativePolicyService; private long mNativePolicyService;
private final Set<Observer> mObservers = new HashSet<Observer>(); private final ObserverList<Observer> mObservers = new ObserverList<Observer>();
/** /**
* Observer interface for observing PolicyService change for Chrome policy * Observer interface for observing PolicyService change for Chrome policy
* domain. * domain.
*
* The default method below may increase method count with Desugar. If there
* are more than 10+ observer implementations, please consider use
* EmptyObserver instead for default behavior.
*/ */
public interface Observer { public interface Observer {
/**
* Invoked when Chrome policy is modified. The native class of both
* |previous| and |current| become invalid once the method
* returns. Do not use their references outside the method.
* @param previous PolicyMap contains values before the update.
* @param current PolicyMap contains values after the update.
*/
default void onPolicyUpdated(PolicyMap previous, PolicyMap current) {}
/** /**
* Invoked when Chome policy domain is initialized. Observer must be * Invoked when Chome policy domain is initialized. Observer must be
* added before the naitve PolicyService initialization being finished. * added before the naitve PolicyService initialization being finished.
* Use {@link #isInitializationComplete} to check the initialization * Use {@link #isInitializationComplete} to check the initialization
* state before listening to this event. * state before listening to this event.
*/ */
void onPolicyServiceInitialized(); default void onPolicyServiceInitialized() {}
} }
/** /**
...@@ -52,7 +61,7 @@ public class PolicyService { ...@@ -52,7 +61,7 @@ public class PolicyService {
if (mObservers.isEmpty()) { if (mObservers.isEmpty()) {
PolicyServiceJni.get().addObserver(mNativePolicyService, PolicyService.this); PolicyServiceJni.get().addObserver(mNativePolicyService, PolicyService.this);
} }
mObservers.add(observer); mObservers.addObserver(observer);
} }
/** /**
...@@ -60,7 +69,7 @@ public class PolicyService { ...@@ -60,7 +69,7 @@ public class PolicyService {
* policy update. * policy update.
*/ */
public void removeObserver(Observer observer) { public void removeObserver(Observer observer) {
mObservers.remove(observer); mObservers.removeObserver(observer);
if (mObservers.isEmpty()) { if (mObservers.isEmpty()) {
PolicyServiceJni.get().removeObserver(mNativePolicyService, PolicyService.this); PolicyServiceJni.get().removeObserver(mNativePolicyService, PolicyService.this);
} }
...@@ -86,7 +95,19 @@ public class PolicyService { ...@@ -86,7 +95,19 @@ public class PolicyService {
*/ */
@CalledByNative @CalledByNative
private void onPolicyServiceInitialized() { private void onPolicyServiceInitialized() {
CollectionUtil.forEach(mObservers, observer -> observer.onPolicyServiceInitialized()); for (Observer observer : mObservers) {
observer.onPolicyServiceInitialized();
}
}
/**
* Pass the onPolicyServiceInitialized event to the |mObservers|.
*/
@CalledByNative
private void onPolicyUpdated(PolicyMap previous, PolicyMap current) {
for (Observer observer : mObservers) {
observer.onPolicyUpdated(previous, current);
}
} }
@CalledByNative @CalledByNative
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
package org.chromium.components.policy.test; package org.chromium.components.policy.test;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
import org.junit.Assert; import org.junit.Assert;
...@@ -11,6 +13,7 @@ import org.mockito.Mockito; ...@@ -11,6 +13,7 @@ import org.mockito.Mockito;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.components.policy.PolicyMap;
import org.chromium.components.policy.PolicyService; import org.chromium.components.policy.PolicyService;
import java.util.ArrayList; import java.util.ArrayList;
...@@ -25,6 +28,7 @@ import java.util.List; ...@@ -25,6 +28,7 @@ import java.util.List;
@JNINamespace("policy::android") @JNINamespace("policy::android")
public class PolicyServiceTestSupporter { public class PolicyServiceTestSupporter {
private List<PolicyService.Observer> mObservers = new ArrayList<>(); private List<PolicyService.Observer> mObservers = new ArrayList<>();
private List<Boolean> mPolicyUpdated = new ArrayList<>();
PolicyService mPolicyService; PolicyService mPolicyService;
...@@ -42,6 +46,7 @@ public class PolicyServiceTestSupporter { ...@@ -42,6 +46,7 @@ public class PolicyServiceTestSupporter {
private int addObserver() { private int addObserver() {
mObservers.add(Mockito.mock(PolicyService.Observer.class)); mObservers.add(Mockito.mock(PolicyService.Observer.class));
mPolicyService.addObserver(mObservers.get(mObservers.size() - 1)); mPolicyService.addObserver(mObservers.get(mObservers.size() - 1));
mPolicyUpdated.add(false);
return mObservers.size() - 1; return mObservers.size() - 1;
} }
...@@ -51,10 +56,35 @@ public class PolicyServiceTestSupporter { ...@@ -51,10 +56,35 @@ public class PolicyServiceTestSupporter {
} }
@CalledByNative @CalledByNative
private void verifyObserverCalled(int index, int cnt) { private void verifyInitializationEvent(int index, int cnt) {
Mockito.verify(mObservers.get(index), times(cnt)).onPolicyServiceInitialized(); Mockito.verify(mObservers.get(index), times(cnt)).onPolicyServiceInitialized();
} }
@CalledByNative
private void verifyPolicyUpdatedEvent(int index, int cnt) {
Mockito.verify(mObservers.get(index), times(cnt))
.onPolicyUpdated(any(PolicyMap.class), any(PolicyMap.class));
}
@CalledByNative
private void setupPolicyUpdatedEventWithValues(
int index, PolicyMap expectPrevious, PolicyMap expectCurrent) {
// The native PolicyMapAndroid instance is only available inside the
// PolicyService.Observer.onPolicUpdated() function. Hence we have to setup the argument
// expectation before the event being triggered.
mPolicyUpdated.set(index, false);
Mockito.doAnswer(invocation -> mPolicyUpdated.set(index, true))
.when(mObservers.get(index))
.onPolicyUpdated(argThat(actualPrevious -> expectPrevious.isEqual(actualPrevious)),
argThat(actualCurrent -> expectCurrent.isEqual(actualCurrent)));
}
@CalledByNative
private void verifyPolicyUpdatedEventWithValues(int index, int cnt) {
Assert.assertTrue(mPolicyUpdated.get(index));
verifyPolicyUpdatedEvent(index, cnt);
}
@CalledByNative @CalledByNative
private void verifyNoMoreInteractions() { private void verifyNoMoreInteractions() {
for (PolicyService.Observer observer : mObservers) { for (PolicyService.Observer observer : mObservers) {
......
...@@ -70,6 +70,14 @@ base::android::ScopedJavaLocalRef<jstring> PolicyMapAndroid::GetDictValue( ...@@ -70,6 +70,14 @@ base::android::ScopedJavaLocalRef<jstring> PolicyMapAndroid::GetDictValue(
return GetListOrDictValue(env, policy, /* is_dict */ true); return GetListOrDictValue(env, policy, /* is_dict */ true);
} }
jboolean PolicyMapAndroid::Equals(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller,
jlong other) const {
return policy_map_.Equals(
reinterpret_cast<PolicyMapAndroid*>(other)->policy_map_);
}
base::android::ScopedJavaLocalRef<jobject> PolicyMapAndroid::GetJavaObject() { base::android::ScopedJavaLocalRef<jobject> PolicyMapAndroid::GetJavaObject() {
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
if (!java_ref_) { if (!java_ref_) {
......
...@@ -52,6 +52,10 @@ class POLICY_EXPORT PolicyMapAndroid { ...@@ -52,6 +52,10 @@ class POLICY_EXPORT PolicyMapAndroid {
const base::android::JavaParamRef<jobject>& caller, const base::android::JavaParamRef<jobject>& caller,
const base::android::JavaRef<jstring>& policy) const; const base::android::JavaRef<jstring>& policy) const;
jboolean Equals(JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller,
jlong other) const;
base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); base::android::ScopedJavaLocalRef<jobject> GetJavaObject();
private: private:
......
...@@ -36,6 +36,19 @@ void PolicyServiceAndroid::OnPolicyServiceInitialized(PolicyDomain domain) { ...@@ -36,6 +36,19 @@ void PolicyServiceAndroid::OnPolicyServiceInitialized(PolicyDomain domain) {
base::android::ScopedJavaLocalRef<jobject>(java_ref_)); base::android::ScopedJavaLocalRef<jobject>(java_ref_));
} }
void PolicyServiceAndroid::OnPolicyUpdated(const PolicyNamespace& ns,
const PolicyMap& previous,
const PolicyMap& current) {
DCHECK_EQ(POLICY_DOMAIN_CHROME, ns.domain);
DCHECK(java_ref_);
PolicyMapAndroid previous_android(previous);
PolicyMapAndroid current_android(current);
Java_PolicyService_onPolicyUpdated(
base::android::AttachCurrentThread(),
base::android::ScopedJavaLocalRef<jobject>(java_ref_),
previous_android.GetJavaObject(), current_android.GetJavaObject());
}
bool PolicyServiceAndroid::IsInitializationComplete( bool PolicyServiceAndroid::IsInitializationComplete(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& caller) const { const base::android::JavaParamRef<jobject>& caller) const {
......
...@@ -47,6 +47,9 @@ class POLICY_EXPORT PolicyServiceAndroid : public PolicyService::Observer { ...@@ -47,6 +47,9 @@ class POLICY_EXPORT PolicyServiceAndroid : public PolicyService::Observer {
// PolicyService::Observer implementation. // PolicyService::Observer implementation.
// Pass the event to the Java observers. // Pass the event to the Java observers.
void OnPolicyServiceInitialized(PolicyDomain domain) override; void OnPolicyServiceInitialized(PolicyDomain domain) override;
void OnPolicyUpdated(const PolicyNamespace& ns,
const PolicyMap& previous,
const PolicyMap& current) override;
base::android::ScopedJavaLocalRef<jobject> GetJavaObject(); base::android::ScopedJavaLocalRef<jobject> GetJavaObject();
......
...@@ -6,12 +6,12 @@ ...@@ -6,12 +6,12 @@
#include <jni.h> #include <jni.h>
#include "base/android/java_exception_reporter.h"
#include "base/android/jni_android.h" #include "base/android/jni_android.h"
#include "base/android/scoped_java_ref.h" #include "base/android/scoped_java_ref.h"
#include "components/policy/android/test_jni_headers/PolicyServiceTestSupporter_jni.h" #include "components/policy/android/test_jni_headers/PolicyServiceTestSupporter_jni.h"
#include "components/policy/core/common/mock_policy_service.h" #include "components/policy/core/common/mock_policy_service.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
using ::testing::Return; using ::testing::Return;
using ::testing::ReturnRef; using ::testing::ReturnRef;
...@@ -69,7 +69,13 @@ TEST_F(PolicyServiceAndroidTest, OneObserver) { ...@@ -69,7 +69,13 @@ TEST_F(PolicyServiceAndroidTest, OneObserver) {
Java_PolicyServiceTestSupporter_addObserver(env_, j_support_); Java_PolicyServiceTestSupporter_addObserver(env_, j_support_);
policy_service_android_->OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME); policy_service_android_->OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME);
Java_PolicyServiceTestSupporter_verifyObserverCalled( Java_PolicyServiceTestSupporter_verifyInitializationEvent(
env_, j_support_, /*index*/ 0, /*times*/ 1);
policy_service_android_->OnPolicyUpdated(
PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()), PolicyMap(),
PolicyMap());
Java_PolicyServiceTestSupporter_verifyPolicyUpdatedEvent(
env_, j_support_, /*index*/ 0, /*times*/ 1); env_, j_support_, /*index*/ 0, /*times*/ 1);
EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME, EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME,
...@@ -96,10 +102,10 @@ TEST_F(PolicyServiceAndroidTest, MultipleObservers) { ...@@ -96,10 +102,10 @@ TEST_F(PolicyServiceAndroidTest, MultipleObservers) {
// Trigger the event and only the activated Java observer get notified. // Trigger the event and only the activated Java observer get notified.
policy_service_android_->OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME); policy_service_android_->OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME);
Java_PolicyServiceTestSupporter_verifyObserverCalled(env_, j_support_, Java_PolicyServiceTestSupporter_verifyInitializationEvent(
observer1, /*times*/ 1); env_, j_support_, observer1, /*times*/ 1);
Java_PolicyServiceTestSupporter_verifyObserverCalled(env_, j_support_, Java_PolicyServiceTestSupporter_verifyInitializationEvent(
observer2, /*times*/ 0); env_, j_support_, observer2, /*times*/ 0);
// Remove the last Java observers and triggers the C++ observer cleanup too. // Remove the last Java observers and triggers the C++ observer cleanup too.
EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME, EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME,
...@@ -109,5 +115,34 @@ TEST_F(PolicyServiceAndroidTest, MultipleObservers) { ...@@ -109,5 +115,34 @@ TEST_F(PolicyServiceAndroidTest, MultipleObservers) {
::testing::Mock::VerifyAndClearExpectations(&policy_service_); ::testing::Mock::VerifyAndClearExpectations(&policy_service_);
} }
TEST_F(PolicyServiceAndroidTest, PolicyUpdateEvent) {
EXPECT_CALL(policy_service_,
AddObserver(POLICY_DOMAIN_CHROME, policy_service_android_.get()))
.Times(1);
int observer_id =
Java_PolicyServiceTestSupporter_addObserver(env_, j_support_);
PolicyMap previous;
PolicyMap current;
previous.Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
POLICY_SOURCE_PLATFORM, base::Value(1), nullptr);
current.Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
POLICY_SOURCE_PLATFORM, base::Value(2), nullptr);
Java_PolicyServiceTestSupporter_setupPolicyUpdatedEventWithValues(
env_, j_support_, /*index*/ 0, PolicyMapAndroid(previous).GetJavaObject(),
PolicyMapAndroid(current).GetJavaObject());
policy_service_android_->OnPolicyUpdated(
PolicyNamespace(POLICY_DOMAIN_CHROME, std::string()), previous, current);
Java_PolicyServiceTestSupporter_verifyPolicyUpdatedEventWithValues(
env_, j_support_, /*index*/ 0, /*times*/ 1);
EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME,
policy_service_android_.get()))
.Times(1);
Java_PolicyServiceTestSupporter_removeObserver(env_, j_support_, observer_id);
::testing::Mock::VerifyAndClearExpectations(&policy_service_);
}
} // namespace android } // namespace android
} // namespace policy } // namespace policy
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