Commit 8a2b9883 authored by Liquan (Max) Gu's avatar Liquan (Max) Gu Committed by Commit Bot

[PlayBilling] Refactor IsReadyToPayServiceHelper constructor

Context:
Devguides suggest against doing any concrete things in constructor
since it's harder to test and prone to reading confusion.

Before:
IsReadyToPayServiceHelper did the service binding in the constructor.

After:
IsReadyToPayServiceHelper did the service binding in query().

Change:
* Move the service binding part from constructor to query().


Change-Id: Ibbc9adae00e64e01cbf1fe15298736b3d83d7950
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198651
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769025}
parent d6cd57f6
...@@ -21,7 +21,6 @@ import org.chromium.chrome.browser.ChromeActivity; ...@@ -21,7 +21,6 @@ import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.components.payments.ErrorStrings; import org.chromium.components.payments.ErrorStrings;
import org.chromium.components.payments.PayerData; import org.chromium.components.payments.PayerData;
import org.chromium.components.payments.PaymentApp; import org.chromium.components.payments.PaymentApp;
import org.chromium.components.payments.PaymentApp.InstrumentDetailsCallback;
import org.chromium.components.payments.intent.IsReadyToPayServiceHelper; import org.chromium.components.payments.intent.IsReadyToPayServiceHelper;
import org.chromium.components.payments.intent.WebPaymentIntentHelper; import org.chromium.components.payments.intent.WebPaymentIntentHelper;
import org.chromium.components.payments.intent.WebPaymentIntentHelperType; import org.chromium.components.payments.intent.WebPaymentIntentHelperType;
...@@ -264,6 +263,7 @@ public class AndroidPaymentApp ...@@ -264,6 +263,7 @@ public class AndroidPaymentApp
} }
mIsReadyToPayServiceHelper = new IsReadyToPayServiceHelper( mIsReadyToPayServiceHelper = new IsReadyToPayServiceHelper(
ContextUtils.getApplicationContext(), isReadyToPayIntent, /*resultHandler=*/this); ContextUtils.getApplicationContext(), isReadyToPayIntent, /*resultHandler=*/this);
mIsReadyToPayServiceHelper.query();
} }
@VisibleForTesting @VisibleForTesting
......
...@@ -187,6 +187,7 @@ public class IsReadyToPayServiceHelperTest { ...@@ -187,6 +187,7 @@ public class IsReadyToPayServiceHelperTest {
mErrorReceived = true; mErrorReceived = true;
} }
}); });
helper.query();
CriteriaHelper.pollInstrumentationThread(new Criteria() { CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override @Override
public boolean isSatisfied() { public boolean isSatisfied() {
...@@ -215,6 +216,7 @@ public class IsReadyToPayServiceHelperTest { ...@@ -215,6 +216,7 @@ public class IsReadyToPayServiceHelperTest {
Assert.fail(); Assert.fail();
} }
}); });
helper.query();
}); });
CriteriaHelper.pollInstrumentationThread(new Criteria() { CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override @Override
...@@ -244,6 +246,7 @@ public class IsReadyToPayServiceHelperTest { ...@@ -244,6 +246,7 @@ public class IsReadyToPayServiceHelperTest {
mErrorReceived = true; mErrorReceived = true;
} }
}); });
helper.query();
}); });
CriteriaHelper.pollInstrumentationThread(new Criteria() { CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override @Override
...@@ -273,6 +276,7 @@ public class IsReadyToPayServiceHelperTest { ...@@ -273,6 +276,7 @@ public class IsReadyToPayServiceHelperTest {
mErrorReceived = true; mErrorReceived = true;
} }
}); });
helper.query();
}); });
CriteriaHelper.pollInstrumentationThread(new Criteria() { CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override @Override
...@@ -302,6 +306,7 @@ public class IsReadyToPayServiceHelperTest { ...@@ -302,6 +306,7 @@ public class IsReadyToPayServiceHelperTest {
mErrorReceived = true; mErrorReceived = true;
} }
}); });
helper.query();
}); });
// Assuming CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL > // Assuming CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL >
// IsReadyToPayServiceHelper.SERVICE_CONNECTION_TIMEOUT_MS. // IsReadyToPayServiceHelper.SERVICE_CONNECTION_TIMEOUT_MS.
......
...@@ -31,6 +31,7 @@ public class IsReadyToPayServiceHelper ...@@ -31,6 +31,7 @@ public class IsReadyToPayServiceHelper
private boolean mIsServiceBindingInitiated; private boolean mIsServiceBindingInitiated;
private boolean mIsReadyToPayQueried; private boolean mIsReadyToPayQueried;
private Handler mHandler; private Handler mHandler;
private Intent mIsReadyToPayIntent;
/** The callback that returns the result (success or error) to the helper's caller. */ /** The callback that returns the result (success or error) to the helper's caller. */
public interface ResultHandler { public interface ResultHandler {
...@@ -45,8 +46,7 @@ public class IsReadyToPayServiceHelper ...@@ -45,8 +46,7 @@ public class IsReadyToPayServiceHelper
} }
/** /**
* The constructor starts the IsReadyToPay service. The result would be returned asynchronously * Initiate the helper.
* with one callback.
* @param context The application context. Should not be null. * @param context The application context. Should not be null.
* @param isReadyToPayIntent The IsReaddyToPay intent created by {@link * @param isReadyToPayIntent The IsReaddyToPay intent created by {@link
* WebPaymentIntentHelper#createIsReadyToPayIntent}. Should not be null. * WebPaymentIntentHelper#createIsReadyToPayIntent}. Should not be null.
...@@ -60,6 +60,14 @@ public class IsReadyToPayServiceHelper ...@@ -60,6 +60,14 @@ public class IsReadyToPayServiceHelper
mContext = context; mContext = context;
mResultHandler = resultHandler; mResultHandler = resultHandler;
mHandler = new Handler(); mHandler = new Handler();
mIsReadyToPayIntent = isReadyToPayIntent;
}
/**
* Query the IsReadyToPay service. The result would be returned in the resultHandler callback
* asynchronously. Note that resultHandler would be invoked only once.
*/
public void query() {
try { try {
// This method returns "true if the system is in the process of bringing up a // This method returns "true if the system is in the process of bringing up a
// service that your client has permission to bind to; false if the system couldn't // service that your client has permission to bind to; false if the system couldn't
...@@ -68,7 +76,7 @@ public class IsReadyToPayServiceHelper ...@@ -68,7 +76,7 @@ public class IsReadyToPayServiceHelper
// the connection." // the connection."
// https://developer.android.com/reference/android/content/Context.html#bindService(android.content.Intent,%20android.content.ServiceConnection,%20int) // https://developer.android.com/reference/android/content/Context.html#bindService(android.content.Intent,%20android.content.ServiceConnection,%20int)
mIsServiceBindingInitiated = mContext.bindService( mIsServiceBindingInitiated = mContext.bindService(
isReadyToPayIntent, /*serviceConnection=*/this, Context.BIND_AUTO_CREATE); mIsReadyToPayIntent, /*serviceConnection=*/this, Context.BIND_AUTO_CREATE);
} catch (SecurityException e) { } catch (SecurityException e) {
// Intentionally blank, so mIsServiceBindingInitiated is false. // Intentionally blank, so mIsServiceBindingInitiated is false.
} }
......
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