Commit d9d2344d authored by Brandon Wylie's avatar Brandon Wylie Committed by Chromium LUCI CQ

Introduce UnownedUserDataSupplier to manage an UnownedUserData supplier

Combines all the setup/lifecycle management of UUD (UnownedUserData) and
Supplier, which removes a bunch of boilerplate code for setting up and
tearing down separate UUD/ObservableSuppliers.

It works by providing an abstract base class, which is then subclassed
by the specific type being supplied through UUD. This is similar to how
we currently encapsulate UUD keys to keep them private.

The base class, UnownedUserDataSupplier, stores an ObservableSupplier<E>
in UUD during construction. After construction, users can immediately
take a reference to the ObservableSupplier through the type subclass
mentioned above. This UnownedUserDataSupplier will act like a normal
ObservableSupplier until it's destroyed.

Change-Id: I72d466e6b34fb8e634d23a6f99ab49ee1ace1dc8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2442011Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarSky Malice <skym@chromium.org>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846402}
parent e010c2c3
......@@ -3782,6 +3782,7 @@ if (is_android) {
"android/java/src/org/chromium/base/supplier/OneshotSupplier.java",
"android/java/src/org/chromium/base/supplier/OneshotSupplierImpl.java",
"android/java/src/org/chromium/base/supplier/Supplier.java",
"android/java/src/org/chromium/base/supplier/UnownedUserDataSupplier.java",
"android/java/src/org/chromium/base/task/AsyncTask.java",
"android/java/src/org/chromium/base/task/BackgroundOnlyAsyncTask.java",
"android/java/src/org/chromium/base/task/ChoreographerTaskRunner.java",
......@@ -4040,6 +4041,7 @@ if (is_android) {
"android/junit/src/org/chromium/base/supplier/ObservableSupplierImplTest.java",
"android/junit/src/org/chromium/base/supplier/OneShotCallbackTest.java",
"android/junit/src/org/chromium/base/supplier/OneshotSupplierImplTest.java",
"android/junit/src/org/chromium/base/supplier/UnownedUserDataSupplierTest.java",
"android/junit/src/org/chromium/base/task/AsyncTaskThreadTest.java",
"android/junit/src/org/chromium/base/task/SequencedTaskRunnerTaskMigrationTest.java",
"android/junit/src/org/chromium/base/task/TaskTraitsTest.java",
......
......@@ -136,6 +136,8 @@ public final class UnownedUserDataKey<T extends UnownedUserData> {
/**
* Detaches the {@link UnownedUserData} from all hosts that it is currently attached to with
* this key. It is OK to call this for already detached objects.
*
* @param object The object to detach from all hosts.
*/
public final void detachFromAllHosts(@NonNull T object) {
assertNoDestroyedAttachments();
......
// Copyright 2020 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.base.supplier;
import androidx.annotation.NonNull;
import org.chromium.base.UnownedUserData;
import org.chromium.base.UnownedUserDataHost;
import org.chromium.base.UnownedUserDataKey;
/**
* Handles the combined lifecycle management for {@link UnownedUserData} and
* {@link DestroyableObservableSupplier}.
* <p>
* Classes that hold a reference to to the concrete implementation of this class are also in charge
* of its lifecycle. {@link #destroy} should be called when the applciation is shutting down. This
* will detach the {@link UnownedUserDataSupplier}, but it won't destroy the supplied object.
* <p>
* A functional example with best practices is defined in {@link UnownedUserDataSupplierTest}.
*
* @param <E> The type of the data to be Supplied and stored in UnownedUserData.
* @see UnownedUserDataHost for more details on ownership and typical usage.
* @see UnownedUserDataKey for information about the type of key that is required.
* @see UnownedUserData for the marker interface used for this type of data.
*/
public abstract class UnownedUserDataSupplier<E> extends ObservableSupplierImpl<E>
implements DestroyableObservableSupplier<E>, UnownedUserData {
private final UnownedUserDataKey<UnownedUserDataSupplier<E>> mUudKey;
private final UnownedUserDataHost mHost;
private boolean mIsDestroyed;
/**
* Constructs an UnownedUserDataSupplier.
*
* @param uudKey The {@link UnownedUserDataKey}, which is defined in subclasses.
* @param host The {@link UnownedUserDataHost} which hosts the UnownedUserDataSupplier.
*/
protected UnownedUserDataSupplier(
@NonNull UnownedUserDataKey<? extends UnownedUserDataSupplier<E>> uudKey,
@NonNull UnownedUserDataHost host) {
mUudKey = (UnownedUserDataKey<UnownedUserDataSupplier<E>>) uudKey;
mHost = host;
mIsDestroyed = false;
mUudKey.attachToHost(mHost, this);
}
@Override
public void destroy() {
assert !mIsDestroyed;
mUudKey.detachFromHost(mHost);
mIsDestroyed = true;
}
}
\ No newline at end of file
// Copyright 2020 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.base.supplier;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.robolectric.annotation.Config;
import org.chromium.base.Callback;
import org.chromium.base.UnownedUserDataHost;
import org.chromium.base.UnownedUserDataKey;
import org.chromium.base.test.BaseRobolectricTestRunner;
/**
* Unit tests for {@link ObservableSupplierImpl}.
*/
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class UnownedUserDataSupplierTest {
/** Serves as an example concrete class for {@link UnownedUserDataSupplier}. */
static class TestUnownedUserDataSupplier extends UnownedUserDataSupplier<String> {
private static final UnownedUserDataKey<TestUnownedUserDataSupplier> KEY =
new UnownedUserDataKey<TestUnownedUserDataSupplier>(
TestUnownedUserDataSupplier.class);
/** Use this pattern to mock the {@link UnownedUserDataSupplier} for testing. */
private static TestUnownedUserDataSupplier sInstanceForTesting;
/**
* Retrieves an {@link ObservableSupplier} from the given host. Real implementations should
* use {@link WindowAndroid}.
*/
public static ObservableSupplier<String> from(UnownedUserDataHost host) {
if (sInstanceForTesting != null) return sInstanceForTesting;
return KEY.retrieveDataFromHost(host);
}
/**
* Constructs a {@link TestUnownedUserDataSupplier} with the given host. Real
* implementations should use {@link WindowAndroid} instead.
*/
public TestUnownedUserDataSupplier(UnownedUserDataHost host) {
super(KEY, host);
}
static UnownedUserDataKey<TestUnownedUserDataSupplier> getUnownedUserDataKeyForTesting() {
return KEY;
}
static void setInstanceForTesting(TestUnownedUserDataSupplier testUnownedUserDataSupplier) {
sInstanceForTesting = testUnownedUserDataSupplier;
}
}
static final String TEST_STRING_1 = "testString1";
static final String TEST_STRING_2 = "testString2";
private final UnownedUserDataHost mHost = new UnownedUserDataHost();
private final TestUnownedUserDataSupplier mSupplier = new TestUnownedUserDataSupplier(mHost);
private boolean mIsDestroyed;
@Before
public void setUp() {
mIsDestroyed = false;
}
@After
public void tearDown() {
if (!mIsDestroyed) {
mSupplier.destroy();
mIsDestroyed = true;
}
Assert.assertNull(TestUnownedUserDataSupplier.from(mHost));
}
@Test
public void testSet() {
mSupplier.set(TEST_STRING_1);
// Simulate client
ObservableSupplierImpl<String> supplier =
(ObservableSupplierImpl) TestUnownedUserDataSupplier.from(mHost);
Assert.assertEquals(TEST_STRING_1, supplier.get());
Callback<String> callback = Mockito.mock(Callback.class);
supplier.addObserver(callback);
supplier.set(TEST_STRING_2);
Mockito.verify(callback).onResult(TEST_STRING_2);
}
@Test
public void testAttachMultipleSuppliersToSameHost() {
TestUnownedUserDataSupplier secondarySupplier = new TestUnownedUserDataSupplier(mHost);
Assert.assertFalse(
TestUnownedUserDataSupplier.getUnownedUserDataKeyForTesting().isAttachedToAnyHost(
mSupplier));
Assert.assertTrue(
TestUnownedUserDataSupplier.getUnownedUserDataKeyForTesting().isAttachedToAnyHost(
secondarySupplier));
}
@Test
public void testDestroy() {
mSupplier.destroy();
Assert.assertNull(TestUnownedUserDataSupplier.from(mHost));
Assert.assertFalse(
TestUnownedUserDataSupplier.getUnownedUserDataKeyForTesting().isAttachedToAnyHost(
mSupplier));
mIsDestroyed = true;
}
@Test
public void testDestroy_DoubleDestroy() {
mSupplier.destroy();
try {
mSupplier.destroy();
throw new Error("Expected an assert to be triggered.");
} catch (AssertionError e) {
}
mIsDestroyed = true;
}
}
\ No newline at end of file
......@@ -1228,6 +1228,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/share/ShareButtonController.java",
"java/src/org/chromium/chrome/browser/share/ShareDelegate.java",
"java/src/org/chromium/chrome/browser/share/ShareDelegateImpl.java",
"java/src/org/chromium/chrome/browser/share/ShareDelegateSupplier.java",
"java/src/org/chromium/chrome/browser/share/ShareHelper.java",
"java/src/org/chromium/chrome/browser/share/ShareUtils.java",
"java/src/org/chromium/chrome/browser/sharing/SharingAdapter.java",
......
......@@ -343,7 +343,4 @@ specific_include_rules = {
"Fido2CredentialRequest\.java": [
"+chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java",
],
"ShareServiceImplementationFactory\.java": [
"+chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java",
],
}
......@@ -53,6 +53,7 @@ import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.base.supplier.OneshotSupplier;
import org.chromium.base.supplier.OneshotSupplierImpl;
import org.chromium.base.supplier.UnownedUserDataSupplier;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.AppHooks;
......@@ -136,6 +137,7 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.settings.SettingsLauncherImpl;
import org.chromium.chrome.browser.share.ShareDelegate;
import org.chromium.chrome.browser.share.ShareDelegateImpl;
import org.chromium.chrome.browser.share.ShareDelegateSupplier;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.tab.AccessibilityVisibilityHandler;
import org.chromium.chrome.browser.tab.Tab;
......@@ -278,8 +280,7 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
private CompositorViewHolder mCompositorViewHolder;
private ObservableSupplierImpl<LayoutManagerImpl> mLayoutManagerSupplier =
new ObservableSupplierImpl<>();
private ObservableSupplierImpl<ShareDelegate> mShareDelegateSupplier =
new ObservableSupplierImpl<>();
private UnownedUserDataSupplier<ShareDelegate> mShareDelegateSupplier;
private InsetObserverView mInsetObserverView;
private ContextualSearchManager mContextualSearchManager;
private SnackbarManager mSnackbarManager;
......@@ -351,6 +352,9 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
@Override
public void performPreInflationStartup() {
// Setup UnownedUserData suppliers before they're used.
mShareDelegateSupplier = new ShareDelegateSupplier(getWindowAndroid());
// Make sure the root coordinator is created prior to calling super to ensure all the
// activity lifecycle events are called.
mRootUiCoordinator = createRootUiCoordinator();
......@@ -1330,6 +1334,11 @@ public abstract class ChromeActivity<C extends ChromeActivityComponent>
mBookmarkBridgeSupplier = null;
}
if (mShareDelegateSupplier != null) {
mShareDelegateSupplier.destroy();
mShareDelegateSupplier = null;
}
mActivityTabProvider.destroy();
mComponent = null;
......
// Copyright 2020 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.share;
import org.chromium.base.UnownedUserDataKey;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.UnownedUserDataSupplier;
import org.chromium.ui.base.WindowAndroid;
/**
* A {@link UnownedUserDataSupplier} which manages the supplier and UnownedUserData for a
* {@link ShareDelegate}.
*/
public class ShareDelegateSupplier extends UnownedUserDataSupplier<ShareDelegate> {
private static final UnownedUserDataKey<ShareDelegateSupplier> KEY =
new UnownedUserDataKey<ShareDelegateSupplier>(ShareDelegateSupplier.class);
private static ShareDelegateSupplier sInstanceForTesting;
/** Return {@link ShareDelegate} supplier associated with the given {@link WindowAndroid}. */
public static ObservableSupplier<ShareDelegate> from(WindowAndroid windowAndroid) {
if (sInstanceForTesting != null) return sInstanceForTesting;
return KEY.retrieveDataFromHost(windowAndroid.getUnownedUserDataHost());
}
/** Constructs a ShareDelegateSupplier and attaches it to the {@link WindowAndroid} */
public ShareDelegateSupplier(WindowAndroid windowAndroid) {
super(KEY, windowAndroid.getUnownedUserDataHost());
}
static void setInstanceForTesting(ShareDelegateSupplier instanceForTesting) {
sInstanceForTesting = instanceForTesting;
}
}
......@@ -4,9 +4,11 @@
package org.chromium.chrome.browser.webshare;
import org.chromium.chrome.browser.app.ChromeActivity;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.share.ChromeShareExtras;
import org.chromium.chrome.browser.share.ShareDelegate;
import org.chromium.chrome.browser.share.ShareDelegateImpl.ShareOrigin;
import org.chromium.chrome.browser.share.ShareDelegateSupplier;
import org.chromium.components.browser_ui.share.ShareParams;
import org.chromium.components.browser_ui.webshare.ShareServiceImpl;
import org.chromium.content_public.browser.WebContents;
......@@ -18,9 +20,12 @@ import org.chromium.webshare.mojom.ShareService;
*/
public class ShareServiceImplementationFactory implements InterfaceFactory<ShareService> {
private final WebContents mWebContents;
private Supplier<ShareDelegate> mShareDelegateSupplier;
public ShareServiceImplementationFactory(WebContents webContents) {
mWebContents = webContents;
mShareDelegateSupplier = ShareDelegateSupplier.from(webContents.getTopLevelNativeWindow());
assert mShareDelegateSupplier != null;
}
@Override
......@@ -28,14 +33,13 @@ public class ShareServiceImplementationFactory implements InterfaceFactory<Share
ShareServiceImpl.WebShareDelegate delegate = new ShareServiceImpl.WebShareDelegate() {
@Override
public boolean canShare() {
return mWebContents.getTopLevelNativeWindow().getActivity() != null;
return mShareDelegateSupplier.get() != null;
}
@Override
public void share(ShareParams params) {
ChromeActivity<?> activity =
(ChromeActivity<?>) params.getWindow().getActivity().get();
activity.getShareDelegateSupplier().get().share(
assert mShareDelegateSupplier.get() != null;
mShareDelegateSupplier.get().share(
params, new ChromeShareExtras.Builder().build(), ShareOrigin.WEBSHARE_API);
}
};
......
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