[Android] Synchronize content retrieval over the bridge (#36072)

* Synchronize content retrieval over the bridge

This synchronizes the content retrieval mechanism for serializing
content from the editor, and includes title and content in a single
event.

* Log timed out requests for content

This renames the interruption callback and logs when the latch times out
prior to a response from the editor.

* Add logging for when React context is missing

This adds logging (resolving some TODOs) and also uses an already
extracted method to check if React context is null.

* Rename interface method

This was missed during an earlier refactor

* Remove getTitle method and update javadoc

The getTitle method is no longer used, so we can remove it

* Add note to changelog

packages/react-native-bridge/android/react-native-bridge/src/main/java/org/wordpress/mobile/WPAndroidGlue/WPAndroidGlueCode.java

@@ -15,6 +15,7 @@
 
 import androidx.annotation.Nullable;
 import androidx.core.util.Consumer;
+import androidx.core.util.Pair;
 import androidx.fragment.app.Fragment;
 
 import com.brentvatne.react.ReactVideoPackage;
@@ -49,6 +50,7 @@
 import org.reactnative.maskedview.RNCMaskedViewPackage;
 
 import org.wordpress.android.util.AppLog;
+import org.wordpress.android.util.AppLog.T;
 import org.wordpress.mobile.ReactNativeAztec.ReactAztecPackage;
 import org.wordpress.mobile.ReactNativeGutenbergBridge.BuildConfig;
 import org.wordpress.mobile.ReactNativeGutenbergBridge.GutenbergBridgeJS2Parent;
@@ -667,7 +669,7 @@ public void attachToContainer(ViewGroup viewGroup,
  viewGroup.addView(mReactRootView, 0,
  new LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT));
 
- if (mReactContext != null) {
+ if (hasReactContext()) {
  setPreferredColorScheme(isDarkMode);
  }
 
@@ -844,7 +846,7 @@ private void updateContent(String title, String content) {
  if (title != null) {
  mTitle = title;
  }
- if (mReactContext != null) {
+ if (hasReactContext()) {
  if (content != null) {
  mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().setHtmlInJS(content);
  }
@@ -854,72 +856,96 @@ private void updateContent(String title, String content) {
  }
  }
 
- public interface OnGetContentTimeout {
- void onGetContentTimeout(InterruptedException ie);
+ public interface OnGetContentInterrupted {
+ void onGetContentInterrupted(InterruptedException ie);
  }
 
- public CharSequence getContent(CharSequence originalContent, OnGetContentTimeout onGetContentTimeout) {
- if (mReactContext != null) {
+ public synchronized CharSequence getContent(CharSequence originalContent,
+ OnGetContentInterrupted onGetContentInterrupted) {
+ if (hasReactContext()) {
  mGetContentCountDownLatch = new CountDownLatch(1);
 
  mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().getHtmlFromJS();
 
  try {
- mGetContentCountDownLatch.await(10, TimeUnit.SECONDS);
+ boolean success = mGetContentCountDownLatch.await(10, TimeUnit.SECONDS);
+ if (!success) {
+ AppLog.e(T.EDITOR, "Timeout reached before response from requestGetHtml.");
+ }
  } catch (InterruptedException ie) {
- onGetContentTimeout.onGetContentTimeout(ie);
+ onGetContentInterrupted.onGetContentInterrupted(ie);
  }
 
  return mContentChanged ? (mContentHtml == null ? "" : mContentHtml) : originalContent;
  } else {
- // TODO: Add app logging here
+ AppLog.e(T.EDITOR, "getContent was called when there was no React context.");
  }
 
  return originalContent;
  }
 
- public CharSequence getTitle(OnGetContentTimeout onGetContentTimeout) {
- if (mReactContext != null) {
+ /** This method retrieves both the title and the content from the Gutenberg editor by the emission of a single
+ * event. This is useful to avoid redundant events, since {@link #getContent} already retrieves the title as well,
+ * using same event, and also shares the same mechanism to suspend execution until a response is received (or a
+ * timeout is reached).
+ * @param originalContent fallback content to return in case the timeout is reached, or the thread is interrupted
+ * @param onGetContentInterrupted callback to invoke if thread is interrupted before the timeout
+ * @return A Pair of CharSequence with the first being the title and the second being the content
+ */
+ public synchronized Pair<CharSequence, CharSequence> getTitleAndContent(CharSequence originalContent,
+ OnGetContentInterrupted onGetContentInterrupted) {
+ if (hasReactContext()) {
  mGetContentCountDownLatch = new CountDownLatch(1);
 
  mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().getHtmlFromJS();
 
  try {
- mGetContentCountDownLatch.await(10, TimeUnit.SECONDS);
+ boolean success = mGetContentCountDownLatch.await(10, TimeUnit.SECONDS);
+ if (!success) {
+ AppLog.e(T.EDITOR, "Timeout reached before response from requestGetHtml.");
+ }
  } catch (InterruptedException ie) {
- onGetContentTimeout.onGetContentTimeout(ie);
+ onGetContentInterrupted.onGetContentInterrupted(ie);
  }
 
- return mTitle == null ? "" : mTitle;
+ return new Pair<>(
+ mTitle == null ? "" : mTitle,
+ mContentChanged ? (mContentHtml == null ? "" : mContentHtml) : originalContent
+ );
  } else {
- // TODO: Add app logging here
+ AppLog.e(T.EDITOR, "getTitleAndContent was called when there was no React context.");
  }
 
- return "";
+ return new Pair<>("", originalContent);
  }
 
  public boolean triggerGetContentInfo(OnContentInfoReceivedListener onContentInfoReceivedListener) {
- if (mReactContext != null && (mGetContentCountDownLatch == null || mGetContentCountDownLatch.getCount() == 0)) {
+ if (hasReactContext() && (mGetContentCountDownLatch == null || mGetContentCountDownLatch.getCount() == 0)) {
  if (!mIsEditorMounted) {
  onContentInfoReceivedListener.onEditorNotReady();
  return false;
  }
-
- mGetContentCountDownLatch = new CountDownLatch(1);
-
- mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().getHtmlFromJS();
-
  new Thread(new Runnable() {
  @Override public void run() {
- try {
- mGetContentCountDownLatch.await(5, TimeUnit.SECONDS);
- if (mContentInfo == null) {
+ // We need to synchronize access to (and overwriting of) the latch to avoid race conditions
+ synchronized (WPAndroidGlueCode.this) {
+ mGetContentCountDownLatch = new CountDownLatch(1);
+
+ mRnReactNativeGutenbergBridgePackage.getRNReactNativeGutenbergBridgeModule().getHtmlFromJS();
+
+ try {
+ boolean success = mGetContentCountDownLatch.await(5, TimeUnit.SECONDS);
+ if (!success) {
+ AppLog.e(T.EDITOR, "Timeout reached before response from requestGetHtml.");
+ }
+ if (mContentInfo == null) {
+ onContentInfoReceivedListener.onContentInfoFailed();
+ } else {
+ onContentInfoReceivedListener.onContentInfoReceived(mContentInfo.toHashMap());
+ }
+ } catch (InterruptedException ie) {
  onContentInfoReceivedListener.onContentInfoFailed();
- } else {
- onContentInfoReceivedListener.onContentInfoReceived(mContentInfo.toHashMap());
  }
- } catch (InterruptedException ie) {
- onContentInfoReceivedListener.onContentInfoFailed();
  }
  }
  }).start();

packages/react-native-editor/CHANGELOG.md

@@ -11,6 +11,7 @@ For each user feature we should also add a importance categorization label to i
 
 ## Unreleased
 - [**] [Image block] Add ability to quickly link images to Media Files and Attachment Pages [#34846]
+- [*] Fixed a race condition when autosaving content (Android) [#36072]
 
 ## 1.65.0
 - [**] Search block - Text and background color support [#35511]