Commit a542ec99 authored by David Maunder's avatar David Maunder Committed by Chromium LUCI CQ

Fix inconsistency and saving bug in price drop experiment

Two bugs were discovered
1) If a Tab navigates to a new URL, the ShoppingPersistedTabData should
be invalidated. Otherwise we will return ShoppingPersistedTabData for
the previous URL. To achieve this an observer on onUrlUpdated is added
to delete the saved ShoppingPersistedTabData and disassociate the object
with the Tab via UserDataHost. We should also do this on a tab close.
2) An observable supplier was not registered for
ShoppingPersistedTabData. This pattern is used to control turning saving
of the PersistedTabData object (in this case ShoppingPersistedTabData)
on and off.

Bug: 1159657
Change-Id: I5ac0d914cb1f03e0bdc9efc7914458dc16877d4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2596435
Commit-Queue: David Maunder <davidjm@chromium.org>
Reviewed-by: default avatarYusuf Ozuysal <yusufo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838424}
parent 44d393c4
...@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.tab.TabCreationState; ...@@ -23,6 +23,7 @@ import org.chromium.chrome.browser.tab.TabCreationState;
import org.chromium.chrome.browser.tab.TabLaunchType; import org.chromium.chrome.browser.tab.TabLaunchType;
import org.chromium.chrome.browser.tab.TabSelectionType; import org.chromium.chrome.browser.tab.TabSelectionType;
import org.chromium.chrome.browser.tab.state.CriticalPersistedTabData; import org.chromium.chrome.browser.tab.state.CriticalPersistedTabData;
import org.chromium.chrome.browser.tab.state.PersistedTabData;
import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier; import org.chromium.chrome.browser.tabmodel.NextTabPolicy.NextTabPolicySupplier;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities; import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.components.external_intents.InterceptNavigationDelegateImpl; import org.chromium.components.external_intents.InterceptNavigationDelegateImpl;
...@@ -662,7 +663,7 @@ public class TabModelImpl extends TabModelJniBridge { ...@@ -662,7 +663,7 @@ public class TabModelImpl extends TabModelJniBridge {
private void finalizeTabClosure(Tab tab, boolean notifyTabClosureCommitted) { private void finalizeTabClosure(Tab tab, boolean notifyTabClosureCommitted) {
if (mTabContentManager != null) mTabContentManager.removeTabThumbnail(tab.getId()); if (mTabContentManager != null) mTabContentManager.removeTabThumbnail(tab.getId());
mTabSaver.removeTabFromQueues(tab); mTabSaver.removeTabFromQueues(tab);
tab.setIsTabSaveEnabled(false); PersistedTabData.onTabClose(tab);
if (!isIncognito()) HistoricalTabSaver.createHistoricalTab(tab); if (!isIncognito()) HistoricalTabSaver.createHistoricalTab(tab);
......
...@@ -58,9 +58,6 @@ public class ShoppingPersistedTabDataTest { ...@@ -58,9 +58,6 @@ public class ShoppingPersistedTabDataTest {
@Mock @Mock
EndpointFetcher.Natives mEndpointFetcherJniMock; EndpointFetcher.Natives mEndpointFetcherJniMock;
// Tracks if the endpoint fetcher has been called once or not
private boolean mCalledOnce;
private static final long PRICE_MICROS = 123456789012345L; private static final long PRICE_MICROS = 123456789012345L;
private static final long UPDATED_PRICE_MICROS = 287000000L; private static final long UPDATED_PRICE_MICROS = 287000000L;
private static final long HIGH_PRICE_MICROS = 141000000L; private static final long HIGH_PRICE_MICROS = 141000000L;
...@@ -86,6 +83,8 @@ public class ShoppingPersistedTabDataTest { ...@@ -86,6 +83,8 @@ public class ShoppingPersistedTabDataTest {
+ "\"currentPrice\":{\"currencyCode\":\"USD\",\"amountMicros\":\"287000000\"}," + "\"currentPrice\":{\"currencyCode\":\"USD\",\"amountMicros\":\"287000000\"},"
+ "\"referenceType\":\"MAIN_PRODUCT\"}}]}"; + "\"referenceType\":\"MAIN_PRODUCT\"}}]}";
private static final String EMPTY_RESPONSE = "{}";
@Before @Before
public void setUp() { public void setUp() {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
...@@ -150,7 +149,7 @@ public class ShoppingPersistedTabDataTest { ...@@ -150,7 +149,7 @@ public class ShoppingPersistedTabDataTest {
private long shoppingPriceChange(Tab tab) { private long shoppingPriceChange(Tab tab) {
final Semaphore initialSemaphore = new Semaphore(0); final Semaphore initialSemaphore = new Semaphore(0);
final Semaphore updateSemaphore = new Semaphore(0); final Semaphore updateSemaphore = new Semaphore(0);
mockEndpointResponse(); mockEndpointResponse(ENDPOINT_RESPONSE_INITIAL);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> { ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> {
verifyEndpointFetcherCalled(1); verifyEndpointFetcherCalled(1);
...@@ -169,6 +168,7 @@ public class ShoppingPersistedTabDataTest { ...@@ -169,6 +168,7 @@ public class ShoppingPersistedTabDataTest {
}); });
acquireSemaphore(initialSemaphore); acquireSemaphore(initialSemaphore);
long firstUpdateTime = getTimeLastUpdatedOnUiThread(tab); long firstUpdateTime = getTimeLastUpdatedOnUiThread(tab);
mockEndpointResponse(ENDPOINT_RESPONSE_UPDATE);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
ShoppingPersistedTabData.from(tab, (updatedShoppingPersistedTabData) -> { ShoppingPersistedTabData.from(tab, (updatedShoppingPersistedTabData) -> {
verifyEndpointFetcherCalled(2); verifyEndpointFetcherCalled(2);
...@@ -191,7 +191,7 @@ public class ShoppingPersistedTabDataTest { ...@@ -191,7 +191,7 @@ public class ShoppingPersistedTabDataTest {
final Semaphore initialSemaphore = new Semaphore(0); final Semaphore initialSemaphore = new Semaphore(0);
final Semaphore updateSemaphore = new Semaphore(0); final Semaphore updateSemaphore = new Semaphore(0);
Tab tab = createTabOnUiThread(TAB_ID, IS_INCOGNITO); Tab tab = createTabOnUiThread(TAB_ID, IS_INCOGNITO);
mockEndpointResponse(); mockEndpointResponse(ENDPOINT_RESPONSE_INITIAL);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> { ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> {
Assert.assertEquals(PRICE_MICROS, shoppingPersistedTabData.getPriceMicros()); Assert.assertEquals(PRICE_MICROS, shoppingPersistedTabData.getPriceMicros());
...@@ -206,6 +206,7 @@ public class ShoppingPersistedTabDataTest { ...@@ -206,6 +206,7 @@ public class ShoppingPersistedTabDataTest {
}); });
acquireSemaphore(initialSemaphore); acquireSemaphore(initialSemaphore);
verifyEndpointFetcherCalled(1); verifyEndpointFetcherCalled(1);
mockEndpointResponse(ENDPOINT_RESPONSE_UPDATE);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> { ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> {
Assert.assertEquals(PRICE_MICROS, shoppingPersistedTabData.getPriceMicros()); Assert.assertEquals(PRICE_MICROS, shoppingPersistedTabData.getPriceMicros());
...@@ -515,6 +516,50 @@ public class ShoppingPersistedTabDataTest { ...@@ -515,6 +516,50 @@ public class ShoppingPersistedTabDataTest {
Assert.assertNull(shoppingPersistedTabData.getPriceDrop()); Assert.assertNull(shoppingPersistedTabData.getPriceDrop());
} }
@UiThreadTest
@SmallTest
@Test
public void testNewUrl() {
Tab tab = createTabOnUiThread(TAB_ID, IS_INCOGNITO);
ShoppingPersistedTabData shoppingPersistedTabData = new ShoppingPersistedTabData(tab);
Assert.assertFalse(shoppingPersistedTabData.mIsTabSaveEnabledSupplier.get());
shoppingPersistedTabData.mIsTabSaveEnabledSupplier.set(true);
shoppingPersistedTabData.mUrlUpdatedObserver.onUrlUpdated(tab);
Assert.assertFalse(shoppingPersistedTabData.mIsTabSaveEnabledSupplier.get());
}
@UiThreadTest
@SmallTest
@Test
public void testSPTDSavingEnabledUponSuccessfulResponse() {
final Semaphore semaphore = new Semaphore(0);
Tab tab = createTabOnUiThread(TAB_ID, IS_INCOGNITO);
mockEndpointResponse(ENDPOINT_RESPONSE_INITIAL);
TestThreadUtils.runOnUiThreadBlocking(() -> {
ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> {
Assert.assertTrue(shoppingPersistedTabData.mIsTabSaveEnabledSupplier.get());
semaphore.release();
});
});
acquireSemaphore(semaphore);
}
@UiThreadTest
@SmallTest
@Test
public void testSPTDNullUponUnsuccessfulResponse() {
final Semaphore semaphore = new Semaphore(0);
Tab tab = createTabOnUiThread(TAB_ID, IS_INCOGNITO);
mockEndpointResponse(EMPTY_RESPONSE);
TestThreadUtils.runOnUiThreadBlocking(() -> {
ShoppingPersistedTabData.from(tab, (shoppingPersistedTabData) -> {
Assert.assertNull(shoppingPersistedTabData);
semaphore.release();
});
});
acquireSemaphore(semaphore);
}
private void verifyEndpointFetcherCalled(int numTimes) { private void verifyEndpointFetcherCalled(int numTimes) {
verify(mEndpointFetcherJniMock, times(numTimes)) verify(mEndpointFetcherJniMock, times(numTimes))
.nativeFetchChromeAPIKey(any(Profile.class), anyString(), anyString(), anyString(), .nativeFetchChromeAPIKey(any(Profile.class), anyString(), anyString(), anyString(),
...@@ -537,14 +582,12 @@ public class ShoppingPersistedTabDataTest { ...@@ -537,14 +582,12 @@ public class ShoppingPersistedTabDataTest {
return res.get(); return res.get();
} }
private void mockEndpointResponse() { private void mockEndpointResponse(String response) {
doAnswer(new Answer<Void>() { doAnswer(new Answer<Void>() {
@Override @Override
public Void answer(InvocationOnMock invocation) { public Void answer(InvocationOnMock invocation) {
Callback callback = (Callback) invocation.getArguments()[7]; Callback callback = (Callback) invocation.getArguments()[7];
String res = mCalledOnce ? ENDPOINT_RESPONSE_UPDATE : ENDPOINT_RESPONSE_INITIAL; callback.onResult(new EndpointResponse(response));
mCalledOnce = true;
callback.onResult(new EndpointResponse(res));
return null; return null;
} }
}) })
......
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
package org.chromium.chrome.browser.tab.state; package org.chromium.chrome.browser.tab.state;
import android.os.SystemClock;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
...@@ -39,13 +37,15 @@ public abstract class PersistedTabData implements UserData { ...@@ -39,13 +37,15 @@ public abstract class PersistedTabData implements UserData {
private static final String TAG = "PTD"; private static final String TAG = "PTD";
private static final Map<String, List<Callback>> sCachedCallbacks = new HashMap<>(); private static final Map<String, List<Callback>> sCachedCallbacks = new HashMap<>();
private static final long NEEDS_UPDATE_DISABLED = Long.MAX_VALUE; private static final long NEEDS_UPDATE_DISABLED = Long.MAX_VALUE;
private static final long LAST_UPDATE_UNKNOWN = 0;
protected final Tab mTab; protected final Tab mTab;
private final PersistedTabDataStorage mPersistedTabDataStorage; private final PersistedTabDataStorage mPersistedTabDataStorage;
private final String mPersistedTabDataId; private final String mPersistedTabDataId;
private long mLastUpdatedMs; private long mLastUpdatedMs = LAST_UPDATE_UNKNOWN;
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
public ObservableSupplierImpl<Boolean> mIsTabSaveEnabledSupplier; public ObservableSupplierImpl<Boolean> mIsTabSaveEnabledSupplier;
private Callback<Boolean> mTabSaveEnabledToggleCallback; private Callback<Boolean> mTabSaveEnabledToggleCallback;
private boolean mFirstSaveDone;
/** /**
* @param tab {@link Tab} {@link PersistedTabData} is being stored for * @param tab {@link Tab} {@link PersistedTabData} is being stored for
...@@ -84,7 +84,9 @@ public abstract class PersistedTabData implements UserData { ...@@ -84,7 +84,9 @@ public abstract class PersistedTabData implements UserData {
PersistedTabDataConfiguration config = PersistedTabDataConfiguration config =
PersistedTabDataConfiguration.get(clazz, tab.isIncognito()); PersistedTabDataConfiguration.get(clazz, tab.isIncognito());
T persistedTabData = factory.create(data, config.getStorage(), config.getId()); T persistedTabData = factory.create(data, config.getStorage(), config.getId());
setUserData(tab, clazz, persistedTabData); if (persistedTabData != null) {
setUserData(tab, clazz, persistedTabData);
}
return persistedTabData; return persistedTabData;
} }
...@@ -153,7 +155,7 @@ public abstract class PersistedTabData implements UserData { ...@@ -153,7 +155,7 @@ public abstract class PersistedTabData implements UserData {
private static void updateLastUpdatedMs(PersistedTabData persistedTabData) { private static void updateLastUpdatedMs(PersistedTabData persistedTabData) {
if (persistedTabData != null) { if (persistedTabData != null) {
persistedTabData.setLastUpdatedMs(SystemClock.uptimeMillis()); persistedTabData.setLastUpdatedMs(System.currentTimeMillis());
} }
} }
...@@ -164,7 +166,10 @@ public abstract class PersistedTabData implements UserData { ...@@ -164,7 +166,10 @@ public abstract class PersistedTabData implements UserData {
if (getTimeToLiveMs() == NEEDS_UPDATE_DISABLED) { if (getTimeToLiveMs() == NEEDS_UPDATE_DISABLED) {
return false; return false;
} }
return mLastUpdatedMs + getTimeToLiveMs() < SystemClock.uptimeMillis(); if (mLastUpdatedMs == LAST_UPDATE_UNKNOWN) {
return true;
}
return mLastUpdatedMs + getTimeToLiveMs() < System.currentTimeMillis();
} }
private static <T extends PersistedTabData> void onPersistedTabDataResult( private static <T extends PersistedTabData> void onPersistedTabDataResult(
...@@ -338,10 +343,21 @@ public abstract class PersistedTabData implements UserData { ...@@ -338,10 +343,21 @@ public abstract class PersistedTabData implements UserData {
mTabSaveEnabledToggleCallback = (isTabSaveEnabled) -> { mTabSaveEnabledToggleCallback = (isTabSaveEnabled) -> {
if (isTabSaveEnabled) { if (isTabSaveEnabled) {
save(); save();
} else { mFirstSaveDone = true;
} else if (mFirstSaveDone) {
delete(); delete();
} }
}; };
mIsTabSaveEnabledSupplier.addObserver(mTabSaveEnabledToggleCallback); mIsTabSaveEnabledSupplier.addObserver(mTabSaveEnabledToggleCallback);
} }
/**
* Delete all {@link PersistedTabData} when a {@link Tab} is closed.
*/
public static void onTabClose(Tab tab) {
tab.setIsTabSaveEnabled(false);
if (ShoppingPersistedTabData.from(tab) != null) {
ShoppingPersistedTabData.from(tab).disableSaving();
}
}
} }
...@@ -18,8 +18,10 @@ import org.chromium.base.Callback; ...@@ -18,8 +18,10 @@ import org.chromium.base.Callback;
import org.chromium.base.LocaleUtils; import org.chromium.base.LocaleUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.chrome.browser.endpoint_fetcher.EndpointFetcher; import org.chromium.chrome.browser.endpoint_fetcher.EndpointFetcher;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.proto.ShoppingPersistedTabData.ShoppingPersistedTabDataProto; import org.chromium.chrome.browser.tab.proto.ShoppingPersistedTabData.ShoppingPersistedTabDataProto;
import org.chromium.components.payments.CurrencyFormatter; import org.chromium.components.payments.CurrencyFormatter;
...@@ -78,6 +80,13 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -78,6 +80,13 @@ public class ShoppingPersistedTabData extends PersistedTabData {
private String mCurrencyCode; private String mCurrencyCode;
@VisibleForTesting
protected ObservableSupplierImpl<Boolean> mIsTabSaveEnabledSupplier =
new ObservableSupplierImpl<>();
@VisibleForTesting
protected EmptyTabObserver mUrlUpdatedObserver;
/** /**
* A price drop for the offer {@link ShoppingPersistedTabData} * A price drop for the offer {@link ShoppingPersistedTabData}
* refers to * refers to
...@@ -119,11 +128,32 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -119,11 +128,32 @@ public class ShoppingPersistedTabData extends PersistedTabData {
.getStorage(), .getStorage(),
PersistedTabDataConfiguration.get(ShoppingPersistedTabData.class, tab.isIncognito()) PersistedTabDataConfiguration.get(ShoppingPersistedTabData.class, tab.isIncognito())
.getId()); .getId());
setupPersistence(tab);
} }
private ShoppingPersistedTabData( private ShoppingPersistedTabData(
Tab tab, byte[] data, PersistedTabDataStorage storage, String persistedTabDataId) { Tab tab, byte[] data, PersistedTabDataStorage storage, String persistedTabDataId) {
super(tab, data, storage, persistedTabDataId); super(tab, data, storage, persistedTabDataId);
setupPersistence(tab);
}
private void setupPersistence(Tab tab) {
// ShoppingPersistedTabData is not saved by default - only when its fields are populated
// (after a successful endpoint repsonse)
disableSaving();
registerIsTabSaveEnabledSupplier(mIsTabSaveEnabledSupplier);
mUrlUpdatedObserver = new EmptyTabObserver() {
// When the URL is updated, the ShoppingPersistedTabData is redundant and should be
// cleaned up.
@Override
public void onUrlUpdated(Tab tab) {
disableSaving();
if (tab.getUserDataHost().getUserData(ShoppingPersistedTabData.class) != null) {
tab.getUserDataHost().removeUserData(ShoppingPersistedTabData.class);
}
}
};
tab.addObserver(mUrlUpdatedObserver);
} }
/** /**
...@@ -155,6 +185,13 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -155,6 +185,13 @@ public class ShoppingPersistedTabData extends PersistedTabData {
ShoppingPersistedTabData.class, callback); ShoppingPersistedTabData.class, callback);
} }
/**
* @return {@link ShoppingPersistedTabData} from {@link UserDataHost}
*/
public static ShoppingPersistedTabData from(Tab tab) {
return from(tab, USER_DATA_KEY);
}
/** /**
* Whether a BuyableProductAnnotation was found or not * Whether a BuyableProductAnnotation was found or not
*/ */
...@@ -166,6 +203,21 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -166,6 +203,21 @@ public class ShoppingPersistedTabData extends PersistedTabData {
int NUM_ENTRIES = 2; int NUM_ENTRIES = 2;
} }
/**
* Enable saving of {@link ShoppingPersistedTabData}
*/
protected void enableSaving() {
mIsTabSaveEnabledSupplier.set(true);
}
/**
* Disable saving of {@link ShoppingPersistedTabData}. Deletes previously saved {@link
* ShoppingPersistedTabData} as well.
*/
public void disableSaving() {
mIsTabSaveEnabledSupplier.set(false);
}
private static ShoppingPersistedTabData build(Tab tab, String responseString, private static ShoppingPersistedTabData build(Tab tab, String responseString,
ShoppingPersistedTabData previousShoppingPersistedTabData) { ShoppingPersistedTabData previousShoppingPersistedTabData) {
ShoppingPersistedTabData res = new ShoppingPersistedTabData(tab); ShoppingPersistedTabData res = new ShoppingPersistedTabData(tab);
...@@ -182,6 +234,7 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -182,6 +234,7 @@ public class ShoppingPersistedTabData extends PersistedTabData {
res.setPriceMicros(Long.parseLong(priceMetadata.getString(AMOUNT_MICROS_KEY)), res.setPriceMicros(Long.parseLong(priceMetadata.getString(AMOUNT_MICROS_KEY)),
previousShoppingPersistedTabData); previousShoppingPersistedTabData);
res.setCurrencyCode(priceMetadata.getString(CURRENCY_CODE_KEY)); res.setCurrencyCode(priceMetadata.getString(CURRENCY_CODE_KEY));
res.setLastUpdatedMs(System.currentTimeMillis());
foundBuyableProductAnnotation = FoundBuyableProductAnnotation.FOUND; foundBuyableProductAnnotation = FoundBuyableProductAnnotation.FOUND;
break; break;
} }
...@@ -197,7 +250,13 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -197,7 +250,13 @@ public class ShoppingPersistedTabData extends PersistedTabData {
RecordHistogram.recordEnumeratedHistogram( RecordHistogram.recordEnumeratedHistogram(
"Tabs.ShoppingPersistedTabData.FoundBuyableProductAnnotation", "Tabs.ShoppingPersistedTabData.FoundBuyableProductAnnotation",
foundBuyableProductAnnotation, FoundBuyableProductAnnotation.NUM_ENTRIES); foundBuyableProductAnnotation, FoundBuyableProductAnnotation.NUM_ENTRIES);
return res; // Only persist this ShoppingPersistedTabData if it was correctly populated from the
// response
if (foundBuyableProductAnnotation == FoundBuyableProductAnnotation.FOUND) {
res.enableSaving();
return res;
}
return null;
} }
/** /**
...@@ -224,6 +283,7 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -224,6 +283,7 @@ public class ShoppingPersistedTabData extends PersistedTabData {
protected void setCurrencyCode(String currencyCode) { protected void setCurrencyCode(String currencyCode) {
mCurrencyCode = currencyCode; mCurrencyCode = currencyCode;
save();
} }
@VisibleForTesting @VisibleForTesting
...@@ -234,6 +294,7 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -234,6 +294,7 @@ public class ShoppingPersistedTabData extends PersistedTabData {
@VisibleForTesting @VisibleForTesting
protected void setPreviousPriceMicros(long previousPriceMicros) { protected void setPreviousPriceMicros(long previousPriceMicros) {
mPreviousPriceMicros = previousPriceMicros; mPreviousPriceMicros = previousPriceMicros;
save();
} }
@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
...@@ -333,6 +394,10 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -333,6 +394,10 @@ public class ShoppingPersistedTabData extends PersistedTabData {
@Override @Override
public boolean deserialize(@Nullable byte[] bytes) { public boolean deserialize(@Nullable byte[] bytes) {
// TODO(crbug.com/1135573) add in metrics for serialize and deserialize // TODO(crbug.com/1135573) add in metrics for serialize and deserialize
// Do not attempt to deserialize if the bytes are null
if (bytes == null) {
return false;
}
try { try {
ShoppingPersistedTabDataProto shoppingPersistedTabDataProto = ShoppingPersistedTabDataProto shoppingPersistedTabDataProto =
ShoppingPersistedTabDataProto.parseFrom(bytes); ShoppingPersistedTabDataProto.parseFrom(bytes);
...@@ -375,4 +440,10 @@ public class ShoppingPersistedTabData extends PersistedTabData { ...@@ -375,4 +440,10 @@ public class ShoppingPersistedTabData extends PersistedTabData {
public void setLastPriceChangeTimeMsForTesting(long lastPriceChangeTimeMs) { public void setLastPriceChangeTimeMsForTesting(long lastPriceChangeTimeMs) {
mLastPriceChangeTimeMs = lastPriceChangeTimeMs; mLastPriceChangeTimeMs = lastPriceChangeTimeMs;
} }
@Override
public void destroy() {
mTab.removeObserver(mUrlUpdatedObserver);
super.destroy();
}
} }
\ No newline at end of file
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