Skip to content

Commit

Permalink
Add basic java support for mapping objects to a Profile.
Browse files Browse the repository at this point in the history
BUG=1422789

Change-Id: I2d93a41b8eadd62397b643da39a5e19ab050660a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4326924
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Commit-Queue: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115989}
  • Loading branch information
Ted Choc authored and Chromium LUCI CQ committed Mar 11, 2023
1 parent 1796303 commit 5f33938
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 78 deletions.
1 change: 1 addition & 0 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,7 @@ if (current_toolchain == default_toolchain) {
"//chrome/browser/privacy_guide/android:java",
"//chrome/browser/privacy_guide/android:junit",
"//chrome/browser/profiles/android:java",
"//chrome/browser/profiles/android:junit",
"//chrome/browser/quick_delete:java",
"//chrome/browser/quick_delete:junit",
"//chrome/browser/safety_check/android:java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileKeyedMap;
import org.chromium.components.dom_distiller.core.DomDistillerService;

import java.util.HashMap;

/**
* DomDistillerServiceFactory maps Profiles to instances of
* {@link DomDistillerService} instances. Each {@link Profile} will at most
Expand All @@ -20,21 +19,16 @@
*/
@JNINamespace("dom_distiller::android")
public class DomDistillerServiceFactory {

private static final HashMap<Profile, DomDistillerService> sServiceMap =
new HashMap<Profile, DomDistillerService>();
private static final ProfileKeyedMap<DomDistillerService> sServiceMap =
new ProfileKeyedMap<>(ProfileKeyedMap.NO_REQUIRED_CLEANUP_ACTION);

/**
* Returns Java DomDistillerService for given Profile.
*/
public static DomDistillerService getForProfile(Profile profile) {
ThreadUtils.assertOnUiThread();
DomDistillerService service = sServiceMap.get(profile);
if (service == null) {
service = DomDistillerServiceFactoryJni.get().getForProfile(profile);
sServiceMap.put(profile, service);
}
return service;
return sServiceMap.getForProfile(
profile, () -> DomDistillerServiceFactoryJni.get().getForProfile(profile));
}

@NativeMethods
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,20 @@
import androidx.annotation.VisibleForTesting;

import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.profiles.ProfileManager;

import java.util.HashMap;
import java.util.Map;
import org.chromium.chrome.browser.profiles.ProfileKeyedMap;

/**
* {@link PageAnnotationService} cached by {@link Profile}.
*/
public class PageAnnotationsServiceFactory {
@VisibleForTesting
protected static final Map<Profile, PageAnnotationsService> sProfileToPageAnnotationsService =
new HashMap<>();
private static ProfileManager.Observer sProfileManagerObserver;
protected static ProfileKeyedMap<PageAnnotationsService> sProfileToPageAnnotationsService;

/** Creates new instance. */
public PageAnnotationsServiceFactory() {
if (sProfileManagerObserver == null) {
sProfileManagerObserver = new ProfileManager.Observer() {
@Override
public void onProfileAdded(Profile profile) {}

@Override
public void onProfileDestroyed(Profile destroyedProfile) {
PageAnnotationsService serviceToDestroy =
sProfileToPageAnnotationsService.get(destroyedProfile);
if (serviceToDestroy != null) {
sProfileToPageAnnotationsService.remove(destroyedProfile);
}

if (sProfileToPageAnnotationsService.isEmpty()) {
ProfileManager.removeObserver(sProfileManagerObserver);
sProfileManagerObserver = null;
}
}
};
ProfileManager.addObserver(sProfileManagerObserver);
if (sProfileToPageAnnotationsService == null) {
sProfileToPageAnnotationsService = new ProfileKeyedMap<PageAnnotationsService>(
ProfileKeyedMap.NO_REQUIRED_CLEANUP_ACTION);
}
}

Expand All @@ -56,11 +34,7 @@ public void onProfileDestroyed(Profile destroyedProfile) {
*/
public PageAnnotationsService getForLastUsedProfile() {
Profile profile = Profile.getLastUsedRegularProfile();
PageAnnotationsService service = sProfileToPageAnnotationsService.get(profile);
if (service == null) {
service = new PageAnnotationsService(new PageAnnotationsServiceProxy(profile));
sProfileToPageAnnotationsService.put(profile, service);
}
return service;
return sProfileToPageAnnotationsService.getForProfile(profile,
() -> new PageAnnotationsService(new PageAnnotationsServiceProxy(profile)));
}
}
16 changes: 16 additions & 0 deletions chrome/browser/profiles/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ android_library("java") {
"java/src/org/chromium/chrome/browser/profiles/OriginalProfileSupplier.java",
"java/src/org/chromium/chrome/browser/profiles/Profile.java",
"java/src/org/chromium/chrome/browser/profiles/ProfileKey.java",
"java/src/org/chromium/chrome/browser/profiles/ProfileKeyedMap.java",
"java/src/org/chromium/chrome/browser/profiles/ProfileManager.java",
"java/src/org/chromium/chrome/browser/profiles/ProfileManagerUtils.java",
"java/src/org/chromium/chrome/browser/profiles/ProfileResolver.java",
Expand All @@ -44,3 +45,18 @@ generate_jni("jni_headers") {
"java/src/org/chromium/chrome/browser/profiles/ProfileResolver.java",
]
}

robolectric_library("junit") {
sources = [
"java/src/org/chromium/chrome/browser/profiles/ProfileKeyedMapTest.java",
]

deps = [
":java",
"//base:base_java",
"//base:base_junit_test_support",
"//third_party/hamcrest:hamcrest_java",
"//third_party/junit",
"//third_party/mockito:mockito_java",
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2023 The Chromium Authors
// 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.profiles;

import androidx.annotation.NonNull;

import org.chromium.base.Callback;
import org.chromium.base.CollectionUtil;
import org.chromium.base.lifetime.Destroyable;
import org.chromium.base.supplier.Supplier;

import java.util.HashMap;
import java.util.Map;

/**
* A Profile-lifetime aware data structure that allows mapping objects to Profile and handling the
* necessary cleanup when a Profile is destroyed.
*
* This data structure owns the objects associated with the Profile and will delete them when
* appropriate.
*
* @param <T> The type of object being mapped to the Profile.
*/
public class ProfileKeyedMap<T> {
/** Indicates no cleanup action is required when destroying an object in the map. */
public static Callback NO_REQUIRED_CLEANUP_ACTION = (e) -> {};

private final Map<Profile, T> mData = new HashMap<>();
private final Callback<T> mDestroyAction;

private ProfileManager.Observer mProfileManagerObserver;

/**
* Creates a map of Profile -> Object that handles automatically cleaning up when the profiles
* are destroyed.
* @param destroyAction The action to be taken on the object during destruction of this map
* or when a Profile is destroyed.
*/
public ProfileKeyedMap(@NonNull Callback<T> destroyAction) {
mDestroyAction = destroyAction;
}

/**
* @return A data structure that maps Profile to Destroyable objects that will be destroyed
* when appropriate.
* @param <T> The object type being mapped against the Profile.
*/
public static <T extends Destroyable> ProfileKeyedMap<T> createMapOfDestroyables() {
return new ProfileKeyedMap<>((e) -> e.destroy());
}

/**
* Gets (and lazily constructs if needed) the mapped object for a given Profile.
* @param profile The Profile the object is associated with.
* @param factory The factory that will construct the object if it does not already exist.
* @return The object associated with the passed in Profile.
*/
public T getForProfile(Profile profile, Supplier<T> factory) {
T obj = mData.get(profile);
if (obj == null) {
obj = factory.get();
mData.put(profile, obj);
}
if (mProfileManagerObserver == null) {
mProfileManagerObserver = new ProfileManager.Observer() {
@Override
public void onProfileAdded(Profile profile) {}

@Override
public void onProfileDestroyed(Profile destroyedProfile) {
T obj = mData.remove(destroyedProfile);
if (obj == null) return;
mDestroyAction.onResult(obj);
}
};
ProfileManager.addObserver(mProfileManagerObserver);
}
return obj;
}

/**
* Destroys this object and all objects currently mapped to Profiles.
*/
public void destroy() {
if (mProfileManagerObserver != null) ProfileManager.removeObserver(mProfileManagerObserver);
mProfileManagerObserver = null;
CollectionUtil.forEach(mData, e -> mDestroyAction.onResult(e.getValue()));
mData.clear();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2023 The Chromium Authors
// 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.profiles;

import static org.chromium.chrome.browser.profiles.ProfileKeyedMap.NO_REQUIRED_CLEANUP_ACTION;

import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.robolectric.annotation.Config;

import org.chromium.base.lifetime.Destroyable;
import org.chromium.base.test.BaseRobolectricTestRunner;

import java.util.HashSet;
import java.util.Set;

/**
* Tests for ProfileKeyedMap.
*/
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class ProfileKeyedMapTest {
@Mock
private Profile mProfile1;
@Mock
private Profile mProfile2;
@Mock
private Profile mProfile3;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
}

@Test
public void testReusesObjects() {
ProfileKeyedMap<Object> map = new ProfileKeyedMap<Object>(NO_REQUIRED_CLEANUP_ACTION);

Object obj1 = new Object();
Assert.assertEquals(obj1, map.getForProfile(mProfile1, () -> obj1));
Assert.assertEquals(obj1, map.getForProfile(mProfile1, Object::new));
}

@Test
public void testCleanupOnProfileDestruction() {
Set<Object> destroyedObjects = new HashSet<>();
ProfileKeyedMap<Object> map =
new ProfileKeyedMap<Object>((obj) -> destroyedObjects.add(obj));

Object obj1 = new Object();
Assert.assertEquals(obj1, map.getForProfile(mProfile1, () -> obj1));

ProfileManager.onProfileDestroyed(mProfile1);
MatcherAssert.assertThat(destroyedObjects, Matchers.hasItem(obj1));
}

@Test
public void testDestroy() {
Set<Object> destroyedObjects = new HashSet<>();
ProfileKeyedMap<Object> map =
new ProfileKeyedMap<Object>((obj) -> destroyedObjects.add(obj));

Object obj1 = new Object();
Assert.assertEquals(obj1, map.getForProfile(mProfile1, () -> obj1));
Object obj2 = new Object();
Assert.assertEquals(obj2, map.getForProfile(mProfile2, () -> obj2));
Object obj3 = new Object();
Assert.assertEquals(obj3, map.getForProfile(mProfile3, () -> obj3));

map.destroy();
MatcherAssert.assertThat(destroyedObjects, Matchers.hasItems(obj1, obj2, obj3));
}

@Test
public void testMapsAreIndependent() {
Set<Object> destroyedObjects = new HashSet<>();
ProfileKeyedMap<Object> map1 =
new ProfileKeyedMap<Object>((obj) -> destroyedObjects.add(obj));

ProfileKeyedMap<Object> map2 =
new ProfileKeyedMap<Object>((obj) -> destroyedObjects.add(obj));

Object obj1 = new Object();
Assert.assertEquals(obj1, map1.getForProfile(mProfile1, () -> obj1));

Object obj2 = new Object();
Assert.assertEquals(obj2, map2.getForProfile(mProfile1, () -> obj2));

map1.destroy();
MatcherAssert.assertThat(destroyedObjects, Matchers.hasItem(obj1));

Assert.assertEquals(obj2, map2.getForProfile(mProfile1, null));
}

@Test
public void testDestroyableMap() {
ProfileKeyedMap<Destroyable> map = ProfileKeyedMap.createMapOfDestroyables();

Destroyable destroyable1 = Mockito.mock(Destroyable.class);
Destroyable destroyable2 = Mockito.mock(Destroyable.class);

map.getForProfile(mProfile1, () -> destroyable1);
map.getForProfile(mProfile2, () -> destroyable2);

Assert.assertEquals(destroyable1, map.getForProfile(mProfile1, null));
Assert.assertEquals(destroyable2, map.getForProfile(mProfile2, null));

map.destroy();
Mockito.verify(destroyable1).destroy();
Mockito.verify(destroyable2).destroy();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.base.lifetime.Destroyable;
import org.chromium.chrome.browser.omnibox.voice.VoiceRecognitionHandler.VoiceResult;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.Tab;
Expand Down Expand Up @@ -40,7 +41,7 @@
* AutocompleteController is no longer valid, and removes it from the AutocompleteControllerFactory
* cache.
*/
public class AutocompleteController {
public class AutocompleteController implements Destroyable {
// Maximum number of voice suggestions to show.
private static final int MAX_VOICE_SUGGESTION_COUNT = 3;

Expand Down Expand Up @@ -224,7 +225,8 @@ private void onSuggestionsReceived(@NonNull AutocompleteResult autocompleteResul
}
}

/* package */ void destroy() {
@Override
public void destroy() {
mListeners.clear();
if (mNativeController == 0) return;
AutocompleteControllerJni.get().destroy(mNativeController);
Expand Down

0 comments on commit 5f33938

Please sign in to comment.