Revert "Add onPolicyUpdate notification in Java."

This reverts commit 6b3c4bdb.

Reason for revert: Broke PolicyServiceAndroidTest.PolicyUpdateEvent on Android ASAN
https://ci.chromium.org/p/chromium/builders/ci/android-asan/8133

Original change's description:
> 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: Sky Malice <skym@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#814310}

TBR=zmin@chromium.org,skym@chromium.org

Change-Id: Ibde93e22e01f4d3eb8d9eb2a8d4d9c42166ed191
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1127469
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454653Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814628}

chrome/android/java/src/org/chromium/chrome/browser/firstrun/TosAndUmaFirstRunFragmentWithEnterpriseSupport.java

@@ -236,13 +236,10 @@ public class TosAndUmaFirstRunFragmentWithEnterpriseSupport
         if (policyService.isInitializationComplete()) {
             updateCctTosPolicy();
         } else {
-            mPolicyServiceObserver = new PolicyService.Observer() {
-                @Override
-                public void onPolicyServiceInitialized() {
-                    policyService.removeObserver(mPolicyServiceObserver);
-                    mPolicyServiceObserver = null;
-                    updateCctTosPolicy();
-                }
+            mPolicyServiceObserver = () -> {
+                policyService.removeObserver(mPolicyServiceObserver);
+                mPolicyServiceObserver = null;
+                updateCctTosPolicy();
             };
             policyService.addObserver(mPolicyServiceObserver);
         }

components/policy/android/java/src/org/chromium/components/policy/PolicyMap.java

@@ -66,11 +66,6 @@ public class PolicyMap {
         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
     private PolicyMap(long nativePolicyMap) {
         mNativePolicyMap = nativePolicyMap;
@@ -90,7 +85,5 @@ public class PolicyMap {
         String getListValue(long nativePolicyMap, PolicyMap caller, String policy);
         @NativeClassQualifiedName("PolicyMapAndroid")
         String getDictValue(long nativePolicyMap, PolicyMap caller, String policy);
-        @NativeClassQualifiedName("PolicyMapAndroid")
-        boolean equals(long nativePolicyMap, PolicyMap caller, long nativeOtherPolicyMap);
     }
 }

components/policy/android/java/src/org/chromium/components/policy/PolicyService.java

@@ -4,12 +4,15 @@
 
 package org.chromium.components.policy;
 
-import org.chromium.base.ObserverList;
+import org.chromium.base.CollectionUtil;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeClassQualifiedName;
 import org.chromium.base.annotations.NativeMethods;
 
+import java.util.HashSet;
+import java.util.Set;
+
 /**
  * Wrapper of the native PolicyService class in the Java layer.
  *
@@ -25,32 +28,20 @@ import org.chromium.base.annotations.NativeMethods;
 @JNINamespace("policy::android")
 public class PolicyService {
     private long mNativePolicyService;
-    private final ObserverList<Observer> mObservers = new ObserverList<Observer>();
+    private final Set<Observer> mObservers = new HashSet<Observer>();
 
     /**
      * Observer interface for observing PolicyService change for Chrome policy
      * 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 {
-        /**
-         * 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
          * added before the naitve PolicyService initialization being finished.
          * Use {@link #isInitializationComplete} to check the initialization
          * state before listening to this event.
          */
-        default void onPolicyServiceInitialized() {}
+        void onPolicyServiceInitialized();
     }
 
     /**
@@ -61,7 +52,7 @@ public class PolicyService {
         if (mObservers.isEmpty()) {
             PolicyServiceJni.get().addObserver(mNativePolicyService, PolicyService.this);
         }
-        mObservers.addObserver(observer);
+        mObservers.add(observer);
     }
 
     /**
@@ -69,7 +60,7 @@ public class PolicyService {
      * policy update.
      */
     public void removeObserver(Observer observer) {
-        mObservers.removeObserver(observer);
+        mObservers.remove(observer);
         if (mObservers.isEmpty()) {
             PolicyServiceJni.get().removeObserver(mNativePolicyService, PolicyService.this);
         }
@@ -95,19 +86,7 @@ public class PolicyService {
      */
     @CalledByNative
     private void 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);
-        }
+        CollectionUtil.forEach(mObservers, observer -> observer.onPolicyServiceInitialized());
     }
 
     @CalledByNative

components/policy/android/javatests/src/org/chromium/components/policy/test/PolicyServiceTestSupporter.java

@@ -4,8 +4,6 @@
 
 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 org.junit.Assert;
@@ -13,7 +11,6 @@ import org.mockito.Mockito;
 
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
-import org.chromium.components.policy.PolicyMap;
 import org.chromium.components.policy.PolicyService;
 
 import java.util.ArrayList;
@@ -28,7 +25,6 @@ import java.util.List;
 @JNINamespace("policy::android")
 public class PolicyServiceTestSupporter {
     private List<PolicyService.Observer> mObservers = new ArrayList<>();
-    private List<Boolean> mPolicyUpdated = new ArrayList<>();
 
     PolicyService mPolicyService;
 
@@ -46,7 +42,6 @@ public class PolicyServiceTestSupporter {
     private int addObserver() {
         mObservers.add(Mockito.mock(PolicyService.Observer.class));
         mPolicyService.addObserver(mObservers.get(mObservers.size() - 1));
-        mPolicyUpdated.add(false);
         return mObservers.size() - 1;
     }
 
@@ -56,35 +51,10 @@ public class PolicyServiceTestSupporter {
     }
 
     @CalledByNative
-    private void verifyInitializationEvent(int index, int cnt) {
+    private void verifyObserverCalled(int index, int cnt) {
         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
     private void verifyNoMoreInteractions() {
         for (PolicyService.Observer observer : mObservers) {

components/policy/core/common/android/policy_map_android.cc

@@ -70,14 +70,6 @@ base::android::ScopedJavaLocalRef<jstring> PolicyMapAndroid::GetDictValue(
   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() {
   JNIEnv* env = base::android::AttachCurrentThread();
   if (!java_ref_) {

components/policy/core/common/android/policy_map_android.h

@@ -52,10 +52,6 @@ class POLICY_EXPORT PolicyMapAndroid {
       const base::android::JavaParamRef<jobject>& caller,
       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();
 
  private:

components/policy/core/common/android/policy_service_android.cc

@@ -36,19 +36,6 @@ void PolicyServiceAndroid::OnPolicyServiceInitialized(PolicyDomain domain) {
       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(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& caller) const {

components/policy/core/common/android/policy_service_android.h

@@ -47,9 +47,6 @@ class POLICY_EXPORT PolicyServiceAndroid : public PolicyService::Observer {
   // PolicyService::Observer implementation.
   // Pass the event to the Java observers.
   void OnPolicyServiceInitialized(PolicyDomain domain) override;
-  void OnPolicyUpdated(const PolicyNamespace& ns,
-                       const PolicyMap& previous,
-                       const PolicyMap& current) override;
 
   base::android::ScopedJavaLocalRef<jobject> GetJavaObject();
 

components/policy/core/common/android/policy_service_android_unittest.cc

@@ -6,12 +6,12 @@
 
 #include <jni.h>
 
-#include "base/android/java_exception_reporter.h"
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
 #include "components/policy/android/test_jni_headers/PolicyServiceTestSupporter_jni.h"
 #include "components/policy/core/common/mock_policy_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
+
 using ::testing::Return;
 using ::testing::ReturnRef;
 
@@ -69,13 +69,7 @@ TEST_F(PolicyServiceAndroidTest, OneObserver) {
       Java_PolicyServiceTestSupporter_addObserver(env_, j_support_);
 
   policy_service_android_->OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME);
-  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(
+  Java_PolicyServiceTestSupporter_verifyObserverCalled(
       env_, j_support_, /*index*/ 0, /*times*/ 1);
 
   EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME,
@@ -102,10 +96,10 @@ TEST_F(PolicyServiceAndroidTest, MultipleObservers) {
 
   // Trigger the event and only the activated Java observer get notified.
   policy_service_android_->OnPolicyServiceInitialized(POLICY_DOMAIN_CHROME);
-  Java_PolicyServiceTestSupporter_verifyInitializationEvent(
-      env_, j_support_, observer1, /*times*/ 1);
-  Java_PolicyServiceTestSupporter_verifyInitializationEvent(
-      env_, j_support_, observer2, /*times*/ 0);
+  Java_PolicyServiceTestSupporter_verifyObserverCalled(env_, j_support_,
+                                                       observer1, /*times*/ 1);
+  Java_PolicyServiceTestSupporter_verifyObserverCalled(env_, j_support_,
+                                                       observer2, /*times*/ 0);
 
   // Remove the last Java observers and triggers the C++ observer cleanup too.
   EXPECT_CALL(policy_service_, RemoveObserver(POLICY_DOMAIN_CHROME,
@@ -115,34 +109,5 @@ TEST_F(PolicyServiceAndroidTest, MultipleObservers) {
   ::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 policy