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

[WebLayer] Moves broken down methods into PRService

Changes:
* Moved the method broken down in CL[1] into PRService unchanged.
* Moved initAndValidate() from CPRService into
PRService#initAndValidate()

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2514699

Bug: 1144527

Change-Id: Ieab4064ab02ead6c9ff3b6a8c8ac7f867437dd36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2511880
Commit-Queue: Liquan (Max) Gu <maxlg@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823754}
parent 2eeb1d81
...@@ -11,7 +11,6 @@ import androidx.annotation.Nullable; ...@@ -11,7 +11,6 @@ import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import androidx.collection.ArrayMap; import androidx.collection.ArrayMap;
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.chrome.R; import org.chromium.chrome.R;
...@@ -257,61 +256,17 @@ public class ChromePaymentRequestService ...@@ -257,61 +256,17 @@ public class ChromePaymentRequestService
/*observer=*/this); /*observer=*/this);
} }
// Implement BrowserPaymentRequest: // Implements BrowserPaymentRequest:
@Override @Override
public boolean initAndValidate(PaymentMethodData[] rawMethodData, PaymentDetails details, public void onWhetherGooglePayBridgeEligible(boolean googlePayBridgeEligible,
boolean googlePayBridgeEligible) {
assert mPaymentRequestService != null;
assert rawMethodData != null;
assert details != null;
onWhetherGooglePayBridgeEligible(googlePayBridgeEligible, mWebContents, rawMethodData);
@Nullable
Map<String, PaymentMethodData> methodData = getValidatedMethodData(rawMethodData);
modifyMethodData(methodData);
if (methodData == null) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage(
ErrorStrings.INVALID_PAYMENT_METHODS_OR_DATA, PaymentErrorReason.USER_CANCEL);
return false;
}
methodData = Collections.unmodifiableMap(methodData);
mQueryForQuota = new HashMap<>(methodData);
onQueryForQuotaCreated(mQueryForQuota);
if (!PaymentValidator.validatePaymentDetails(details)) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage(
ErrorStrings.INVALID_PAYMENT_DETAILS, PaymentErrorReason.USER_CANCEL);
return false;
}
if (disconnectIfExtraValidationFails(mWebContents, methodData, details, mPaymentOptions)) {
return false;
}
PaymentRequestSpec spec = new PaymentRequestSpec(mPaymentOptions, details,
methodData.values(), LocaleUtils.getDefaultLocaleString());
if (spec.getRawTotal() == null) {
mJourneyLogger.setAborted(AbortReason.INVALID_DATA_FROM_RENDERER);
disconnectFromClientWithDebugMessage(ErrorStrings.TOTAL_REQUIRED);
return false;
}
onSpecValidated(spec);
PaymentAppService service = PaymentAppService.getInstance();
addPaymentAppFactories(service);
service.create(/*delegate=*/this);
return true;
}
private void onWhetherGooglePayBridgeEligible(boolean googlePayBridgeEligible,
WebContents webContents, PaymentMethodData[] rawMethodData) { WebContents webContents, PaymentMethodData[] rawMethodData) {
mIsGooglePayBridgeActivated = googlePayBridgeEligible mIsGooglePayBridgeActivated = googlePayBridgeEligible
&& SkipToGPayHelperUtil.canActivateExperiment(mWebContents, rawMethodData); && SkipToGPayHelperUtil.canActivateExperiment(mWebContents, rawMethodData);
} }
private void onSpecValidated(PaymentRequestSpec spec) { // Implements BrowserPaymentRequest:
@Override
public void onSpecValidated(PaymentRequestSpec spec) {
mSpec = spec; mSpec = spec;
mPaymentUiService.initialize( mPaymentUiService.initialize(
mSpec.getPaymentDetails(), mSpec.getRawTotal(), mSpec.getRawLineItems()); mSpec.getPaymentDetails(), mSpec.getRawTotal(), mSpec.getRawLineItems());
...@@ -349,7 +304,9 @@ public class ChromePaymentRequestService ...@@ -349,7 +304,9 @@ public class ChromePaymentRequestService
requestedMethodGoogle, requestedMethodOther); requestedMethodGoogle, requestedMethodOther);
} }
private boolean disconnectIfExtraValidationFails(WebContents webContents, // Implements BrowserPaymentRequest:
@Override
public boolean disconnectIfExtraValidationFails(WebContents webContents,
Map<String, PaymentMethodData> methodData, PaymentDetails details, Map<String, PaymentMethodData> methodData, PaymentDetails details,
PaymentOptions options) { PaymentOptions options) {
assert mPaymentRequestService != null; assert mPaymentRequestService != null;
...@@ -369,7 +326,9 @@ public class ChromePaymentRequestService ...@@ -369,7 +326,9 @@ public class ChromePaymentRequestService
return false; return false;
} }
private void onQueryForQuotaCreated(Map<String, PaymentMethodData> queryForQuota) { // Implements BrowserPaymentRequest:
@Override
public void onQueryForQuotaCreated(Map<String, PaymentMethodData> queryForQuota) {
if (queryForQuota.containsKey(MethodStrings.BASIC_CARD) if (queryForQuota.containsKey(MethodStrings.BASIC_CARD)
&& PaymentFeatureList.isEnabledOrExperimentalFeaturesEnabled( && PaymentFeatureList.isEnabledOrExperimentalFeaturesEnabled(
PaymentFeatureList.STRICT_HAS_ENROLLED_AUTOFILL_INSTRUMENT)) { PaymentFeatureList.STRICT_HAS_ENROLLED_AUTOFILL_INSTRUMENT)) {
...@@ -378,9 +337,12 @@ public class ChromePaymentRequestService ...@@ -378,9 +337,12 @@ public class ChromePaymentRequestService
PaymentOptionsUtils.stringifyRequestedInformation(mPaymentOptions); PaymentOptionsUtils.stringifyRequestedInformation(mPaymentOptions);
queryForQuota.put("basic-card-payment-options", paymentMethodData); queryForQuota.put("basic-card-payment-options", paymentMethodData);
} }
mQueryForQuota = queryForQuota;
} }
private void addPaymentAppFactories(PaymentAppService service) { // Implements BrowserPaymentRequest:
@Override
public void addPaymentAppFactories(PaymentAppService service) {
String androidFactoryId = AndroidPaymentAppFactory.class.getName(); String androidFactoryId = AndroidPaymentAppFactory.class.getName();
if (!service.containsFactory(androidFactoryId)) { if (!service.containsFactory(androidFactoryId)) {
service.addUniqueFactory(new AndroidPaymentAppFactory(), androidFactoryId); service.addUniqueFactory(new AndroidPaymentAppFactory(), androidFactoryId);
...@@ -400,6 +362,12 @@ public class ChromePaymentRequestService ...@@ -400,6 +362,12 @@ public class ChromePaymentRequestService
} }
} }
// Implements BrowserPaymentRequest:
@Override
public PaymentAppFactoryDelegate getPaymentAppFactoryDelegate() {
return this;
}
/** @return Whether the UI was built. */ /** @return Whether the UI was built. */
private boolean buildUI(ChromeActivity activity) { private boolean buildUI(ChromeActivity activity) {
String error = mPaymentUiService.buildPaymentRequestUI(activity, String error = mPaymentUiService.buildPaymentRequestUI(activity,
...@@ -595,22 +563,9 @@ public class ChromePaymentRequestService ...@@ -595,22 +563,9 @@ public class ChromePaymentRequestService
} }
} }
@Nullable // Implements BrowserPaymentRequest:
private static Map<String, PaymentMethodData> getValidatedMethodData( @Override
PaymentMethodData[] methodDataList) { public void modifyMethodData(@Nullable Map<String, PaymentMethodData> methodDataMap) {
// Payment methodData are required.
assert methodDataList != null;
if (methodDataList.length == 0) return null;
Map<String, PaymentMethodData> result = new ArrayMap<>();
for (PaymentMethodData methodData : methodDataList) {
String methodName = methodData.supportedMethod;
if (TextUtils.isEmpty(methodName)) return null;
result.put(methodName, methodData);
}
return result;
}
private void modifyMethodData(@Nullable Map<String, PaymentMethodData> methodDataMap) {
if (!mIsGooglePayBridgeActivated || methodDataMap == null) return; if (!mIsGooglePayBridgeActivated || methodDataMap == null) return;
Map<String, PaymentMethodData> result = new ArrayMap<>(); Map<String, PaymentMethodData> result = new ArrayMap<>();
for (PaymentMethodData methodData : methodDataMap.values()) { for (PaymentMethodData methodData : methodDataMap.values()) {
...@@ -1278,8 +1233,8 @@ public class ChromePaymentRequestService ...@@ -1278,8 +1233,8 @@ public class ChromePaymentRequestService
mIsHasEnrolledInstrumentResponsePending = false; mIsHasEnrolledInstrumentResponsePending = false;
if (CanMakePaymentQuery.canQuery( if (CanMakePaymentQuery.canQuery(mWebContents, mTopLevelOrigin, mPaymentRequestOrigin,
mWebContents, mTopLevelOrigin, mPaymentRequestOrigin, mQueryForQuota)) { mPaymentRequestService.getQueryForQuota())) {
mPaymentRequestService.onHasEnrolledInstrument(response mPaymentRequestService.onHasEnrolledInstrument(response
? HasEnrolledInstrumentQueryResult.HAS_ENROLLED_INSTRUMENT ? HasEnrolledInstrumentQueryResult.HAS_ENROLLED_INSTRUMENT
: HasEnrolledInstrumentQueryResult.HAS_NO_ENROLLED_INSTRUMENT); : HasEnrolledInstrumentQueryResult.HAS_NO_ENROLLED_INSTRUMENT);
......
...@@ -4,12 +4,18 @@ ...@@ -4,12 +4,18 @@
package org.chromium.components.payments; package org.chromium.components.payments;
import androidx.annotation.Nullable;
import org.chromium.content_public.browser.WebContents;
import org.chromium.payments.mojom.PaymentDetails; import org.chromium.payments.mojom.PaymentDetails;
import org.chromium.payments.mojom.PaymentErrorReason; import org.chromium.payments.mojom.PaymentErrorReason;
import org.chromium.payments.mojom.PaymentMethodData; import org.chromium.payments.mojom.PaymentMethodData;
import org.chromium.payments.mojom.PaymentOptions;
import org.chromium.payments.mojom.PaymentRequest; import org.chromium.payments.mojom.PaymentRequest;
import org.chromium.payments.mojom.PaymentValidationErrors; import org.chromium.payments.mojom.PaymentValidationErrors;
import java.util.Map;
/** /**
* The browser part of the PaymentRequest implementation. The browser here can be either the * The browser part of the PaymentRequest implementation. The browser here can be either the
* Android Chrome browser or the WebLayer "browser". * Android Chrome browser or the WebLayer "browser".
...@@ -27,19 +33,6 @@ public interface BrowserPaymentRequest { ...@@ -27,19 +33,6 @@ public interface BrowserPaymentRequest {
PaymentRequestService paymentRequestService); PaymentRequestService paymentRequestService);
} }
/**
* Initialize the browser part of the {@link PaymentRequest} implementation and validate the raw
* payment request data coming from the untrusted mojo.
* @param methodData The supported methods specified by the merchant, cannot be null.
* @param details The payment details specified by the merchant, cannot be null.
* @param googlePayBridgeEligible True when the renderer process deems the current request
* eligible for the skip-to-GPay experimental flow. It is ultimately up to the browser
* process to determine whether to trigger it
* @return whether the initialization is successful.
*/
boolean initAndValidate(PaymentMethodData[] methodData, PaymentDetails details,
boolean googlePayBridgeEligible);
/** /**
* The browser part of the {@link PaymentRequest#show} implementation. * The browser part of the {@link PaymentRequest#show} implementation.
* @param isUserGesture Whether this method is triggered from a user gesture. * @param isUserGesture Whether this method is triggered from a user gesture.
...@@ -92,4 +85,49 @@ public interface BrowserPaymentRequest { ...@@ -92,4 +85,49 @@ public interface BrowserPaymentRequest {
* calling it. This method can be called within itself without causing infinite loop. * calling it. This method can be called within itself without causing infinite loop.
*/ */
void close(); void close();
/**
* Modifies the given method data.
* @param methodData A map of method names to PaymentMethodData, could be null. This parameter
* could be modified in place.
*/
default void modifyMethodData(@Nullable Map<String, PaymentMethodData> methodData) {}
/**
* Called when queryForQuota is created.
* @param queryForQuota The created queryForQuota, which could be modified in place.
*/
default void onQueryForQuotaCreated(Map<String, PaymentMethodData> queryForQuota) {}
/**
* Performs extra validation for the given input and disconnects the mojo pipe if failed.
* @param webContents The WebContents that represents the merchant page.
* @param methodData A map of the method data specified for the request.
* @param details The payment details specified for the request.
* @param paymentOptions The payment options specified for the request.
* @return Whether this method has disconnected the mojo pipe.
*/
default boolean disconnectIfExtraValidationFails(WebContents webContents,
Map<String, PaymentMethodData> methodData, PaymentDetails details,
PaymentOptions paymentOptions) {
return false;
}
/**
* Called when the PaymentRequestSpec is validated.
* @param spec The validated PaymentRequestSpec.
*/
default void onSpecValidated(PaymentRequestSpec spec) {}
/**
* Adds the PaymentAppFactory(s) specified by the implementers to the given PaymentAppService.
* @param service The PaymentAppService to be added with the factories.
*/
void addPaymentAppFactories(PaymentAppService service);
/** @return A PaymentAppFactoryDelegate to be used with the PaymentAppService. */
PaymentAppFactoryDelegate getPaymentAppFactoryDelegate();
default void onWhetherGooglePayBridgeEligible(boolean googlePayBridgeEligible,
WebContents webContents, PaymentMethodData[] rawMethodData) {}
} }
...@@ -8,10 +8,13 @@ import android.text.TextUtils; ...@@ -8,10 +8,13 @@ import android.text.TextUtils;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting; import androidx.annotation.VisibleForTesting;
import androidx.collection.ArrayMap;
import org.chromium.base.LocaleUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.components.autofill.EditableOption; import org.chromium.components.autofill.EditableOption;
import org.chromium.components.page_info.CertificateChainHelper; import org.chromium.components.page_info.CertificateChainHelper;
import org.chromium.components.payments.BrowserPaymentRequest.Factory;
import org.chromium.components.url_formatter.UrlFormatter; import org.chromium.components.url_formatter.UrlFormatter;
import org.chromium.content_public.browser.RenderFrameHost; import org.chromium.content_public.browser.RenderFrameHost;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
...@@ -30,7 +33,10 @@ import org.chromium.payments.mojom.PaymentResponse; ...@@ -30,7 +33,10 @@ import org.chromium.payments.mojom.PaymentResponse;
import org.chromium.payments.mojom.PaymentValidationErrors; import org.chromium.payments.mojom.PaymentValidationErrors;
import org.chromium.url.Origin; import org.chromium.url.Origin;
import java.util.Collections;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map;
/** /**
* {@link PaymentRequestService}, {@link MojoPaymentRequestGateKeeper} and * {@link PaymentRequestService}, {@link MojoPaymentRequestGateKeeper} and
...@@ -73,6 +79,16 @@ public class PaymentRequestService { ...@@ -73,6 +79,16 @@ public class PaymentRequestService {
// mBrowserPaymentRequest is null when it has closed or is uninitiated. // mBrowserPaymentRequest is null when it has closed or is uninitiated.
private BrowserPaymentRequest mBrowserPaymentRequest; private BrowserPaymentRequest mBrowserPaymentRequest;
/**
* A mapping of the payment method names to the corresponding payment method specific data. If
* STRICT_HAS_ENROLLED_AUTOFILL_INSTRUMENT is enabled, then the key "basic-card-payment-options"
* also maps to the following payment options:
* - requestPayerEmail
* - requestPayerName
* - requestPayerPhone
* - requestShipping
*/
private HashMap<String, PaymentMethodData> mQueryForQuota;
/** /**
* An observer interface injected when running tests to allow them to observe events. * An observer interface injected when running tests to allow them to observe events.
...@@ -270,9 +286,16 @@ public class PaymentRequestService { ...@@ -270,9 +286,16 @@ public class PaymentRequestService {
instance.close(); instance.close();
return null; return null;
} }
instance.startPaymentAppService();
return instance; return instance;
} }
private void startPaymentAppService() {
PaymentAppService service = PaymentAppService.getInstance();
mBrowserPaymentRequest.addPaymentAppFactories(service);
service.create(/*delegate=*/mBrowserPaymentRequest.getPaymentAppFactoryDelegate());
}
/** Abort the request, used before this class's instantiation. */ /** Abort the request, used before this class's instantiation. */
private static void abortBeforeInstantiation(@Nullable PaymentRequestClient client, private static void abortBeforeInstantiation(@Nullable PaymentRequestClient client,
@Nullable JourneyLogger journeyLogger, String debugMessage, int reason) { @Nullable JourneyLogger journeyLogger, String debugMessage, int reason) {
...@@ -335,9 +358,8 @@ public class PaymentRequestService { ...@@ -335,9 +358,8 @@ public class PaymentRequestService {
return sNativeObserverForTest; return sNativeObserverForTest;
} }
private boolean initAndValidate(BrowserPaymentRequest.Factory factory, private boolean initAndValidate(Factory factory, PaymentMethodData[] rawMethodData,
PaymentMethodData[] methodData, PaymentDetails details, PaymentDetails details, boolean googlePayBridgeEligible) {
boolean googlePayBridgeEligible) {
mBrowserPaymentRequest = factory.createBrowserPaymentRequest(this); mBrowserPaymentRequest = factory.createBrowserPaymentRequest(this);
mJourneyLogger.recordCheckoutStep(CheckoutFunnelStep.INITIATED); mJourneyLogger.recordCheckoutStep(CheckoutFunnelStep.INITIATED);
...@@ -364,7 +386,78 @@ public class PaymentRequestService { ...@@ -364,7 +386,78 @@ public class PaymentRequestService {
return false; return false;
} }
return mBrowserPaymentRequest.initAndValidate(methodData, details, googlePayBridgeEligible); mBrowserPaymentRequest.onWhetherGooglePayBridgeEligible(
googlePayBridgeEligible, mWebContents, rawMethodData);
@Nullable
Map<String, PaymentMethodData> methodData = getValidatedMethodData(rawMethodData);
mBrowserPaymentRequest.modifyMethodData(methodData);
if (methodData == null) {
mJourneyLogger.setAborted(
org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
org.chromium.components.payments.ErrorStrings.INVALID_PAYMENT_METHODS_OR_DATA,
PaymentErrorReason.USER_CANCEL);
return false;
}
methodData = Collections.unmodifiableMap(methodData);
mQueryForQuota = new HashMap<>(methodData);
mBrowserPaymentRequest.onQueryForQuotaCreated(mQueryForQuota);
if (!PaymentValidator.validatePaymentDetails(details)) {
mJourneyLogger.setAborted(
org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
org.chromium.components.payments.ErrorStrings.INVALID_PAYMENT_DETAILS,
PaymentErrorReason.USER_CANCEL);
return false;
}
if (mBrowserPaymentRequest.disconnectIfExtraValidationFails(
mWebContents, methodData, details, mPaymentOptions)) {
return false;
}
PaymentRequestSpec spec = new PaymentRequestSpec(mPaymentOptions, details,
methodData.values(), LocaleUtils.getDefaultLocaleString());
if (spec.getRawTotal() == null) {
mJourneyLogger.setAborted(
org.chromium.components.payments.AbortReason.INVALID_DATA_FROM_RENDERER);
mBrowserPaymentRequest.disconnectFromClientWithDebugMessage(
org.chromium.components.payments.ErrorStrings.TOTAL_REQUIRED,
PaymentErrorReason.USER_CANCEL);
return false;
}
mBrowserPaymentRequest.onSpecValidated(spec);
return true;
}
/**
* @return The queryForQuota, a mapping of the payment method names to the corresponding payment
* method specific data
*/
public Map<String, PaymentMethodData> getQueryForQuota() {
return mQueryForQuota;
}
/**
* @param methodDataList A list of PaymentMethodData.
* @return The validated method data, a mapping of method names to its PaymentMethodData(s);
* when the given method data is invalid, returns null.
*/
@Nullable
private static Map<String, PaymentMethodData> getValidatedMethodData(
PaymentMethodData[] methodDataList) {
// Payment methodData are required.
assert methodDataList != null;
if (methodDataList.length == 0) return null;
Map<String, PaymentMethodData> result = new ArrayMap<>();
for (PaymentMethodData methodData : methodDataList) {
String methodName = methodData.supportedMethod;
if (TextUtils.isEmpty(methodName)) return null;
result.put(methodName, methodData);
}
return result;
} }
/** /**
......
...@@ -5,10 +5,11 @@ ...@@ -5,10 +5,11 @@
package org.chromium.weblayer_private.payments; package org.chromium.weblayer_private.payments;
import org.chromium.components.payments.BrowserPaymentRequest; import org.chromium.components.payments.BrowserPaymentRequest;
import org.chromium.components.payments.PaymentAppFactoryDelegate;
import org.chromium.components.payments.PaymentAppService;
import org.chromium.components.payments.PaymentRequestService; import org.chromium.components.payments.PaymentRequestService;
import org.chromium.components.payments.PaymentRequestService.Delegate; import org.chromium.components.payments.PaymentRequestService.Delegate;
import org.chromium.payments.mojom.PaymentDetails; import org.chromium.payments.mojom.PaymentDetails;
import org.chromium.payments.mojom.PaymentMethodData;
import org.chromium.payments.mojom.PaymentValidationErrors; import org.chromium.payments.mojom.PaymentValidationErrors;
/** The WebLayer-specific part of the payment request service. */ /** The WebLayer-specific part of the payment request service. */
...@@ -23,13 +24,6 @@ public class WebLayerPaymentRequestService implements BrowserPaymentRequest { ...@@ -23,13 +24,6 @@ public class WebLayerPaymentRequestService implements BrowserPaymentRequest {
assert false : "Not implemented yet"; assert false : "Not implemented yet";
} }
@Override
public boolean initAndValidate(PaymentMethodData[] methodData, PaymentDetails details,
boolean googlePayBridgeEligible) {
assert false : "Not implemented yet";
return false;
}
@Override @Override
public void show(boolean isUserGesture, boolean waitForUpdatedDetails) { public void show(boolean isUserGesture, boolean waitForUpdatedDetails) {
assert false : "Not implemented yet"; assert false : "Not implemented yet";
...@@ -79,4 +73,15 @@ public class WebLayerPaymentRequestService implements BrowserPaymentRequest { ...@@ -79,4 +73,15 @@ public class WebLayerPaymentRequestService implements BrowserPaymentRequest {
public void close() { public void close() {
assert false : "Not implemented yet"; assert false : "Not implemented yet";
} }
@Override
public void addPaymentAppFactories(PaymentAppService service) {
assert false : "Not implemented yet";
}
@Override
public PaymentAppFactoryDelegate getPaymentAppFactoryDelegate() {
assert false : "Not implemented yet";
return null;
}
} }
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