Even for queries with "persistent" set to true, CefQueryCallback->Success clears the reference to the callback

[magreenblatt]

Original report by Harsha Hapuarachchi (Bitbucket: harsharest).

---

CEF_VERSION - 87.1.12

The CEF message router docs describe multiple workflows for queries, one being the "subscription" style where the "persistent" flag is set to true in JS. In the "subscription" style workflow, the success callback can be repeatedly called.

Currently JCEF does not take this persistent value into account, and clears the reference to the CefMessageRouterBrowserSide::Callback when success is called. This results in a somewhat surprising behavior as the the callback's success method can be called repeatedly from java, however it does not result in the callback being called again.

‌

Same issue as : #186/even-for-queries-with-persistent-set-to

[magreenblatt]

Original comment by Hon Hun (Bitbucket: Hon Hun).

---

I have met the same problem.

I think the reason lies here: https://bitbucket.org/chromiumembedded/java-cef/src/master/native/CefQueryCallback_N.cpp

in Java_org_cef_callback_CefQueryCallback_1N_N_1Success at line 34, it always calls the ClearSelf(env, obj) to clear the callback object regardless of whether the persistent is true or not

‌

[STofone]

Can it be fixed?

[STofone]

fakeChat(
    url = "",
    data = null,
    successCallback,
    failCallback = function (e) {
      console.log(e);
    },
    id = null
  ) {
    let json;
    if (id === null) {
      id = getHashcode().toString();
    }
    if (data == null) {
      json = JSON.stringify({
        url: url,
      });
    } else {
      json = JSON.stringify({
        url: url,
        fakePersistent: id,
        data: data,
      });
    }
    window.java({
      request: json,
      persistent: false,
      onSuccess: function (response) {
        if (response !== id + "-cancel") {
          javaApi.fakeChat(url, data, successCallback, failCallback, id);
          let res;
          try {
            res = JSON.parse(response);
          } catch (error) {
            res = response;
          }
          successCallback(res);
        }
      },
      onFailure: function (error_code, error_message) {
        failCallback({ code: error_code, msg: error_message });
      },
    });
  },

I'm using this to implement the same function by starting another chat when onSuccess

[MarshallVielmetti]

Hello, I was wondering if any progress had been made on this issue, as this functionality is very important to a project I am involved with.

[MarshallVielmetti-jhuapl]

I'm going to take a shot at this. I am not very familiar with JNI so may need some support from @magreenblatt
This is also my first shot at open source so please bear with me.

I recreated an (extremely) minimal example of the issue in a fork here. by modifying the simple example.

To replicate:
Modify tests/simple/MainFrame::main with the correct absolute path to the html file (again very minimal example)

Build and compile JCEF then run the simple example

Expected behavior: browser should receive callback from Java every 3000ms and log to console

Actual behavior: browser receives only the first callback, despite it being persistent. Java continues to call the callback as evidenced by messages in stdout, but the browser-side callback is not actually executed.

[MarshallVielmetti-jhuapl]

Keeping this here just for my own documentation:

Upon further analysis, I think setting persistent: true I believe is causing a memory leak (or something analogous)--

When persistent: true is set:

JNI will clear the native reference to the callback. (CefQueryCallback_N.cpp:Success)
However, the callback destructor will not be called, because a QueryInfo object stored by the router will maintain a reference to this callback, until OnCallbackFailure(453) is called.

However, because success removes the JNI reference regardless of the value of persistence, it is impossible to call the Failure function.

Therefore, the QueryInfo will persist until program termination, where it may (?) be cleaned up, however as there is no way to interact with it from Java I would consider this to be an invalid case.

[MarshallVielmetti-jhuapl]

Here is an overview of my proposed changes:

1. Add two private methods to CefQueryCallback_N.java, makePersistent and getIsPersistent, and a private boolean variable persistent_
2. In MessageRouterHandler::OnQuery (message_router_handler.cpp:28), check the value of persistent, if true use JNI_CALL_VOID_METHOD to call makePersistent. This will set a boolean value to true in CefQueryCallback_N
3. In CefQueryCallback_N.cpp::Success, use JNI_CALL_BOOLEAN_METHOD to call getIsPersistent, check the boolean value, and if its true then do not call ClearSelf

I am going to clean up my fork then will PR