Commit b8f72bea authored by Tanmoy Mollik's avatar Tanmoy Mollik Committed by Commit Bot

Call syncStateChanged for two set methods in FakeProfileSyncService

Methods from ProfileSyncService should be called from the UI thread.
However its subclass FakeProfileSyncService sets and gets some sync
states without checking if its on the UI thread or not.

This cl calls syncStateChanged inside setEngineInitialized() and
setPassphraseRequiredForPreferredDataTypes(). Also assert these set and
their corresponding isSet methods are run on UI thread.

Bug: 1056677
Change-Id: I2b41c88c054a1abe9591021abf3476d3b754b972
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080232
Commit-Queue: Tanmoy Mollik <triploblastic@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752447}
parent 9b71609c
...@@ -345,8 +345,7 @@ public class ProfileSyncService { ...@@ -345,8 +345,7 @@ public class ProfileSyncService {
* UI elements can update themselves. * UI elements can update themselves.
*/ */
@CalledByNative @CalledByNative
@VisibleForTesting protected void syncStateChanged() {
public void syncStateChanged() {
for (SyncStateChangedListener listener : mListeners) { for (SyncStateChangedListener listener : mListeners) {
listener.syncStateChanged(); listener.syncStateChanged();
} }
......
...@@ -68,8 +68,7 @@ import java.io.IOException; ...@@ -68,8 +68,7 @@ import java.io.IOException;
// Resolving the error should not show the infobar again. // Resolving the error should not show the infobar again.
deleteSyncErrorInfoBarShowTimePref(); deleteSyncErrorInfoBarShowTimePref();
TestThreadUtils.runOnUiThreadBlocking( mFakeProfileSyncService.setAuthError(GoogleServiceAuthError.State.NONE);
() -> { mFakeProfileSyncService.setAuthError(GoogleServiceAuthError.State.NONE); });
InfoBarUtil.waitUntilNoInfoBarsExist(mSyncTestRule.getInfoBars()); InfoBarUtil.waitUntilNoInfoBarsExist(mSyncTestRule.getInfoBars());
} }
...@@ -85,7 +84,6 @@ import java.io.IOException; ...@@ -85,7 +84,6 @@ import java.io.IOException;
deleteSyncErrorInfoBarShowTimePref(); deleteSyncErrorInfoBarShowTimePref();
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
mFakeProfileSyncService.setFirstSetupComplete(SyncFirstSetupCompleteSource.BASIC_FLOW); mFakeProfileSyncService.setFirstSetupComplete(SyncFirstSetupCompleteSource.BASIC_FLOW);
mFakeProfileSyncService.syncStateChanged();
}); });
InfoBarUtil.waitUntilNoInfoBarsExist(mSyncTestRule.getInfoBars()); InfoBarUtil.waitUntilNoInfoBarsExist(mSyncTestRule.getInfoBars());
} }
...@@ -100,10 +98,7 @@ import java.io.IOException; ...@@ -100,10 +98,7 @@ import java.io.IOException;
// Resolving the error should not show the infobar again. // Resolving the error should not show the infobar again.
deleteSyncErrorInfoBarShowTimePref(); deleteSyncErrorInfoBarShowTimePref();
TestThreadUtils.runOnUiThreadBlocking(() -> { mFakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(false);
mFakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(false);
mFakeProfileSyncService.syncStateChanged();
});
InfoBarUtil.waitUntilNoInfoBarsExist(mSyncTestRule.getInfoBars()); InfoBarUtil.waitUntilNoInfoBarsExist(mSyncTestRule.getInfoBars());
} }
...@@ -114,14 +109,13 @@ import java.io.IOException; ...@@ -114,14 +109,13 @@ import java.io.IOException;
mSyncTestRule.getInfoBars().size()); mSyncTestRule.getInfoBars().size());
mSyncTestRule.setUpTestAccountAndSignIn(); mSyncTestRule.setUpTestAccountAndSignIn();
SyncTestUtil.waitForSyncActive(); SyncTestUtil.waitForSyncActive();
mFakeProfileSyncService.setEngineInitialized(true);
mFakeProfileSyncService.setAuthError(GoogleServiceAuthError.State.NONE);
mFakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(false);
@SyncError @SyncError
int syncError = TestThreadUtils.runOnUiThreadBlockingNoException(() -> { int syncError = TestThreadUtils.runOnUiThreadBlockingNoException(() -> {
mFakeProfileSyncService.setEngineInitialized(true);
mFakeProfileSyncService.setAuthError(GoogleServiceAuthError.State.NONE);
mFakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(false);
mFakeProfileSyncService.setFirstSetupComplete(SyncFirstSetupCompleteSource.BASIC_FLOW); mFakeProfileSyncService.setFirstSetupComplete(SyncFirstSetupCompleteSource.BASIC_FLOW);
mFakeProfileSyncService.syncStateChanged();
return SyncSettingsUtils.getSyncError(); return SyncSettingsUtils.getSyncError();
}); });
// syncError should not equal to any of these errors that trigger the infobar. // syncError should not equal to any of these errors that trigger the infobar.
...@@ -188,22 +182,14 @@ import java.io.IOException; ...@@ -188,22 +182,14 @@ import java.io.IOException;
private void showSyncErrorInfoBarForAuthError() { private void showSyncErrorInfoBarForAuthError() {
mSyncTestRule.setUpTestAccountAndSignIn(); mSyncTestRule.setUpTestAccountAndSignIn();
TestThreadUtils.runOnUiThreadBlocking(() -> { mFakeProfileSyncService.setAuthError(GoogleServiceAuthError.State.INVALID_GAIA_CREDENTIALS);
mFakeProfileSyncService.setAuthError(
GoogleServiceAuthError.State.INVALID_GAIA_CREDENTIALS);
});
mSyncTestRule.loadUrlInNewTab(UrlConstants.CHROME_BLANK_URL); mSyncTestRule.loadUrlInNewTab(UrlConstants.CHROME_BLANK_URL);
} }
private void showSyncErrorInfoBarForPassphraseRequired() { private void showSyncErrorInfoBarForPassphraseRequired() {
mSyncTestRule.setUpTestAccountAndSignIn(); mSyncTestRule.setUpTestAccountAndSignIn();
TestThreadUtils.runOnUiThreadBlocking(() -> { mFakeProfileSyncService.setEngineInitialized(true);
// TODO(https://crbug.com/1056677): call syncStateChanged inside mFakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(true);
// setPassphraseRequiredForPreferredDataTypes
mFakeProfileSyncService.setEngineInitialized(true);
mFakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(true);
mFakeProfileSyncService.syncStateChanged();
});
mSyncTestRule.loadUrlInNewTab(UrlConstants.CHROME_BLANK_URL); mSyncTestRule.loadUrlInNewTab(UrlConstants.CHROME_BLANK_URL);
} }
......
...@@ -4,7 +4,10 @@ ...@@ -4,7 +4,10 @@
package org.chromium.chrome.browser.sync; package org.chromium.chrome.browser.sync;
import androidx.annotation.AnyThread;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
...@@ -32,11 +35,16 @@ public class FakeProfileSyncService extends ProfileSyncService { ...@@ -32,11 +35,16 @@ public class FakeProfileSyncService extends ProfileSyncService {
@Override @Override
public boolean isEngineInitialized() { public boolean isEngineInitialized() {
ThreadUtils.assertOnUiThread();
return mEngineInitialized; return mEngineInitialized;
} }
@AnyThread
public void setEngineInitialized(boolean engineInitialized) { public void setEngineInitialized(boolean engineInitialized) {
mEngineInitialized = engineInitialized; TestThreadUtils.runOnUiThreadBlocking(() -> {
mEngineInitialized = engineInitialized;
syncStateChanged();
});
} }
@Override @Override
...@@ -45,10 +53,12 @@ public class FakeProfileSyncService extends ProfileSyncService { ...@@ -45,10 +53,12 @@ public class FakeProfileSyncService extends ProfileSyncService {
return mAuthError; return mAuthError;
} }
@AnyThread
public void setAuthError(@GoogleServiceAuthError.State int authError) { public void setAuthError(@GoogleServiceAuthError.State int authError) {
ThreadUtils.assertOnUiThread(); TestThreadUtils.runOnUiThreadBlocking(() -> {
mAuthError = authError; mAuthError = authError;
syncStateChanged(); syncStateChanged();
});
} }
@Override @Override
...@@ -77,12 +87,17 @@ public class FakeProfileSyncService extends ProfileSyncService { ...@@ -77,12 +87,17 @@ public class FakeProfileSyncService extends ProfileSyncService {
@Override @Override
public boolean isPassphraseRequiredForPreferredDataTypes() { public boolean isPassphraseRequiredForPreferredDataTypes() {
ThreadUtils.assertOnUiThread();
return mPassphraseRequiredForPreferredDataTypes; return mPassphraseRequiredForPreferredDataTypes;
} }
@AnyThread
public void setPassphraseRequiredForPreferredDataTypes( public void setPassphraseRequiredForPreferredDataTypes(
boolean passphraseRequiredForPreferredDataTypes) { boolean passphraseRequiredForPreferredDataTypes) {
mPassphraseRequiredForPreferredDataTypes = passphraseRequiredForPreferredDataTypes; TestThreadUtils.runOnUiThreadBlocking(() -> {
mPassphraseRequiredForPreferredDataTypes = passphraseRequiredForPreferredDataTypes;
syncStateChanged();
});
} }
@Override @Override
......
...@@ -149,7 +149,6 @@ public class ManageSyncSettingsWithFakeProfileSyncServiceTest { ...@@ -149,7 +149,6 @@ public class ManageSyncSettingsWithFakeProfileSyncServiceTest {
// PassphraseDialogFragment should be dismissed. // PassphraseDialogFragment should be dismissed.
fakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(false); fakeProfileSyncService.setPassphraseRequiredForPreferredDataTypes(false);
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
fakeProfileSyncService.syncStateChanged();
fragment.getFragmentManager().executePendingTransactions(); fragment.getFragmentManager().executePendingTransactions();
Assert.assertNull("PassphraseDialogFragment should be dismissed.", Assert.assertNull("PassphraseDialogFragment should be dismissed.",
mSettingsActivity.getFragmentManager().findFragmentByTag( mSettingsActivity.getFragmentManager().findFragmentByTag(
......
...@@ -77,7 +77,6 @@ public class PassphraseActivityTest { ...@@ -77,7 +77,6 @@ public class PassphraseActivityTest {
// Fake sync's backend finishing its initialization. // Fake sync's backend finishing its initialization.
FakeProfileSyncService pss = (FakeProfileSyncService) ProfileSyncService.get(); FakeProfileSyncService pss = (FakeProfileSyncService) ProfileSyncService.get();
pss.setEngineInitialized(true); pss.setEngineInitialized(true);
pss.syncStateChanged();
}); });
// Nothing crashed; success! // Nothing crashed; success!
......
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