Skip to content

Commit

Permalink
Add more off-by-default logging options for debug-mode builds: ReactE…
Browse files Browse the repository at this point in the history
…ditText, TextLayoutManager

Summary:
Put more logging behind debug-build-only build flags.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D24120153

fbshipit-source-id: 3b33db3e701a2bd3304c7c6502d58eb84e6bdc7f
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Oct 6, 2020
1 parent 77f7658 commit e41ee42
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.ReadableNativeMap;
import com.facebook.react.bridge.WritableArray;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactAccessibilityDelegate;
Expand All @@ -44,9 +45,9 @@
public class TextLayoutManager {

// TODO T67606397: Refactor configuration of fabric logs
private static final boolean ENABLE_MEASURE_LOGGING = false;
private static final boolean ENABLE_MEASURE_LOGGING = ReactBuildConfig.DEBUG && false;

private static final String TAG = "TextLayoutManager";
private static final String TAG = TextLayoutManager.class.getSimpleName();

// It's important to pass the ANTI_ALIAS_FLAG flag to the constructor rather than setting it
// later by calling setFlags. This is because the latter approach triggers a bug on Android 4.4.2.
Expand Down Expand Up @@ -82,10 +83,16 @@ public static boolean isRTL(ReadableMap attributedString) {
}

public static void setCachedSpannabledForTag(int reactTag, @NonNull Spannable sp) {
if (ENABLE_MEASURE_LOGGING) {
FLog.e(TAG, "Set cached spannable for tag[" + reactTag + "]: " + sp.toString());
}
sTagToSpannableCache.put(reactTag, sp);
}

public static void deleteCachedSpannableForTag(int reactTag) {
if (ENABLE_MEASURE_LOGGING) {
FLog.e(TAG, "Delete cached spannable for tag[" + reactTag + "]");
}
sTagToSpannableCache.remove(reactTag);
}

Expand Down Expand Up @@ -360,9 +367,18 @@ public static long measureText(
Spannable text;
if (attributedString.hasKey("cacheId")) {
int cacheId = attributedString.getInt("cacheId");
if (ENABLE_MEASURE_LOGGING) {
FLog.e(TAG, "Get cached spannable for cacheId[" + cacheId + "]");
}
if (sTagToSpannableCache.containsKey(cacheId)) {
text = sTagToSpannableCache.get(cacheId);
if (ENABLE_MEASURE_LOGGING) {
FLog.e(TAG, "Text for spannable found for cacheId[" + cacheId + "]: " + text.toString());
}
} else {
if (ENABLE_MEASURE_LOGGING) {
FLog.e(TAG, "No cached spannable found for cacheId[" + cacheId + "]");
}
return 0;
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@
import androidx.appcompat.widget.AppCompatEditText;
import androidx.core.view.AccessibilityDelegateCompat;
import androidx.core.view.ViewCompat;
import com.facebook.common.logging.FLog;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.uimanager.FabricViewStateManager;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.views.text.CustomLetterSpacingSpan;
Expand Down Expand Up @@ -70,8 +72,10 @@
*/
public class ReactEditText extends AppCompatEditText
implements FabricViewStateManager.HasFabricViewStateManager {

private final InputMethodManager mInputMethodManager;
private final String TAG = ReactEditText.class.getSimpleName();
public static final boolean DEBUG_MODE = ReactBuildConfig.DEBUG && false;

// This flag is set to true when we set the text of the EditText explicitly. In that case, no
// *TextChanged events should be triggered. This is less expensive than removing the text
// listeners and adding them back again after the text change is completed.
Expand Down Expand Up @@ -169,6 +173,9 @@ public boolean performAccessibilityAction(View host, int action, Bundle args) {

@Override
protected void finalize() {
if (DEBUG_MODE) {
FLog.e(TAG, "finalize[" + getId() + "] delete cached spannable");
}
TextLayoutManager.deleteCachedSpannableForTag(getId());
}

Expand Down Expand Up @@ -326,11 +333,18 @@ public void maybeSetSelection(int eventCounter, int start, int end) {

@Override
public void setSelection(int start, int end) {
if (DEBUG_MODE) {
FLog.e(TAG, "setSelection[" + getId() + "]: " + start + " " + end);
}
super.setSelection(start, end);
}

@Override
protected void onSelectionChanged(int selStart, int selEnd) {
if (DEBUG_MODE) {
FLog.e(TAG, "onSelectionChanged[" + getId() + "]: " + selStart + " " + selEnd);
}

super.onSelectionChanged(selStart, selEnd);
if (!mIsSettingTextFromCacheUpdate && mSelectionWatcher != null && hasFocus()) {
mSelectionWatcher.onSelectionChanged(selStart, selEnd);
Expand Down Expand Up @@ -502,6 +516,17 @@ public void maybeSetText(ReactTextUpdate reactTextUpdate) {
return;
}

if (DEBUG_MODE) {
FLog.e(
TAG,
"maybeSetText["
+ getId()
+ "]: current text: "
+ getText()
+ " update: "
+ reactTextUpdate.getText());
}

// The current text gets replaced with the text received from JS. However, the spans on the
// current text need to be adapted to the new text. Since TextView#setText() will remove or
// reset some of these spans even if they are set directly, SpannableStringBuilder#replace() is
Expand Down Expand Up @@ -1004,6 +1029,11 @@ public void beforeTextChanged(CharSequence s, int start, int count, int after) {

@Override
public void onTextChanged(CharSequence s, int start, int before, int count) {
if (DEBUG_MODE) {
FLog.e(
TAG, "onTextChanged[" + getId() + "]: " + s + " " + start + " " + before + " " + count);
}

if (!mIsSettingTextFromCacheUpdate) {
if (!mIsSettingTextFromJS && mListeners != null) {
for (TextWatcher listener : mListeners) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,10 @@ protected EditText createInternalEditText(ThemedReactContext themedReactContext)
public Object updateState(
ReactEditText view, ReactStylesDiffMap props, @Nullable StateWrapper stateWrapper) {

if (ReactEditText.DEBUG_MODE) {
FLog.e(TAG, "updateState: [" + view.getId() + "]");
}

view.getFabricViewStateManager().setStateWrapper(stateWrapper);

ReadableNativeMap state = stateWrapper.getState();
Expand Down

0 comments on commit e41ee42

Please sign in to comment.