setTimeout in WebViewJavascriptBridge is causing performance problems

[jordanpwood]

We use WebViewJavascriptBridge heavily in our app, and we are running into a performance problem. It is taking between 200 and 1000 milliseconds (occasionally more) to send a group of messages from Objective-C and get their callback responses. I was able to get approximately a 10-fold decrease in the amount of time it takes to get a response by removing the setTimeout wrapper in _dispatchMessageFromObjC. I would like to submit a PR to make it so we can run without the setTimeout. Do you have a suggestion on how? Remove it entirely? Make a flag a parameter to bridgeForWebView?

[marcuswestin]

Interesting.

I believe the reason it's there is because if you show an alert box in the JS without a setTimeout first then the app freezes.

Most likely the problem is that the JS message handler executes on the objc thread that passes in the message without a setTimeout, and it cases a lockup since the thread can't finish the message delivery without the alert box being dismissed, and the alert box can't be dismissed because the JavaScript UI thread never gets scheduled. Something like that.

I believe it may also have been an issue to send a message back to ObjC without a setTimeout happening first, for similar reasons.

This was a while back and may no longer be an issue. Can you experiment a bit please, preferably even adding some code to the example app which verifies that none of these two issues happen? If you do and the problem is gone, then I think we should merge your patch as the default behavior, and simply add a flag to go back to the previous behavior of needed in order to still support older iOS versions.

Whaddayasay?

-- while mobile

On Wed, Jun 3, 2015 at 10:34 PM, Jordan Wood notifications@github.com
wrote:

We use WebViewJavascriptBridge heavily in our app, and we are running into a performance problem. It is taking between 200 and 1000 milliseconds (occasionally more) to send a group of messages from Objective-C and get their callback responses. I was able to get approximately a 10-fold decrease in the amount of time it takes to get a response by removing the setTimeout wrapper in _dispatchMessageFromObjC. I would like to submit a PR to make it so we can run without the setTimeout. Do you have a suggestion on how? Remove it entirely? Make a flag a parameter to bridgeForWebView?

Reply to this email directly or view it on GitHub:
#132

[jordanpwood]

I have verified that it is still the case that the app freezes if you use an alert from the bridged Javascript. We use it exclusively for business logic and server communication, so we have no need for that feature. How about a property on WebViewJavascriptBridge such as @property (nonatomic, assign) BOOL bridgeUIEnabled; which defaults to true? That would need to be passed to the appropriate Javascript functions when they are called.

Thoughts?

[marcuswestin]

Thanks for doing that Jordan, really appreciate it.

My thoughts:

· Ideally we would have an "Options" object, but let's not add that right
now
· I think it should be a method - I recall there being something similar
for logging
· I think the method should be very descriptively named, e.g "[bridge
disableJavscriptAlertBoxSafetyTimeout]`
· The "disable flag" should be sent to the javascript runtime just once,
during setup

If you agree and decide to do this that would be fantastic.

If you disagree, let's continue the discussion.

Thanks again!

Cheers,
Marcus

On Thu, Jun 4, 2015 at 2:25 PM, Jordan Wood notifications@github.com
wrote:

I have verified that it is still the case that the app freezes if you use
an alert from the bridged Javascript. We use it exclusively for business
logic and server communication, so we have no need for that feature. How
about a property on WebViewJavascriptBridge such as @Property (nonatomic,
assign) BOOL bridgeUIEnabled; which defaults to true? That would need to
be passed to the appropriate Javascript functions when they are called.

Thoughts?

—
Reply to this email directly or view it on GitHub
#132 (comment)
.

[jordanpwood]

One thing about sending this message to Javascript only at setup is that it
doesn't allow for someone who might want to have the option of reenabling
this check, say when they add functionality later on which uses an alert
box. There is no technical reason why we have to force all the requests to
be either with or without the timeout. What about a method which takes a
boolean like: "[bridge disableJavscriptAlertBoxSafetyTimeout:YES];". We
forward that to the Javascript where we have a enable/disable flag which is
checked by _dispatchMessageFromObjC. I think it is more flexible, and not
at all disruptive to current users of the library.

what do you think?

On Thu, Jun 4, 2015 at 5:13 PM, Marcus Westin notifications@github.com
wrote:

Thanks for doing that Jordan, really appreciate it.

My thoughts:

· Ideally we would have an "Options" object, but let's not add that right
now
· I think it should be a method - I recall there being something similar
for logging
· I think the method should be very descriptively named, e.g "[bridge
disableJavscriptAlertBoxSafetyTimeout]`
· The "disable flag" should be sent to the javascript runtime just once,
during setup

If you agree and decide to do this that would be fantastic.

If you disagree, let's continue the discussion.

Thanks again!

Cheers,
Marcus

On Thu, Jun 4, 2015 at 2:25 PM, Jordan Wood notifications@github.com
wrote:

I have verified that it is still the case that the app freezes if you use
an alert from the bridged Javascript. We use it exclusively for business
logic and server communication, so we have no need for that feature. How
about a property on WebViewJavascriptBridge such as @Property (nonatomic,
assign) BOOL bridgeUIEnabled; which defaults to true? That would need to
be passed to the appropriate Javascript functions when they are called.

Thoughts?

—
Reply to this email directly or view it on GitHub
<
#132 (comment)

.

—
Reply to this email directly or view it on GitHub
#132 (comment)
.

[jordanpwood]

Okay, I've discovered an issue with doing it the way I described before. The problem with keeping the safety enabled/disabled flag in the Javascript code is that if you call it too early (before the Javascript object is set up), then that setting gets lost. We are making this change as a hotfix to our app, so I think I will leave what I have coded up for now, and figure out the best way to do the internals later.

[marcuswestin]

I like your idea, and think it could be done by requiring that the method is called only after bridge setup.

-- while mobile

On Sat, Jun 6, 2015 at 10:05 AM, Jordan Wood notifications@github.com
wrote:

Okay, I've discovered an issue with doing it the way I described before. The problem with keeping the safety enabled/disabled flag in the Javascript code is that if you call it too early (before the Javascript object is set up), then that setting gets lost. We are making this change as a hotfix to our app, so I think I will leave what I have coded up for now, and figure out the best way to do the internals later.

Reply to this email directly or view it on GitHub:
#132 (comment)

[jordanpwood]

I ended up coding up the solution I described, with the addition of a flag
passed in the initialize function. I'll submit a pull request for it on
Monday.

On Saturday, June 6, 2015, Marcus Westin notifications@github.com wrote:

I like your idea, and think it could be done by requiring that the method
is called only after bridge setup.

-- while mobile

On Sat, Jun 6, 2015 at 10:05 AM, Jordan Wood <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

Okay, I've discovered an issue with doing it the way I described before.
The problem with keeping the safety enabled/disabled flag in the Javascript
code is that if you call it too early (before the Javascript object is set
up), then that setting gets lost. We are making this change as a hotfix to
our app, so I think I will leave what I have coded up for now, and figure

out the best way to do the internals later.

Reply to this email directly or view it on GitHub:

#132 (comment)

—
Reply to this email directly or view it on GitHub
#132 (comment)
.

[marcuswestin]

Awesome, thanks Jordan.

-- while mobile

On Sat, Jun 6, 2015 at 9:19 PM, Jordan Wood notifications@github.com
wrote:

I ended up coding up the solution I described, with the addition of a flag
passed in the initialize function. I'll submit a pull request for it on
Monday.
On Saturday, June 6, 2015, Marcus Westin notifications@github.com wrote:

I like your idea, and think it could be done by requiring that the method
is called only after bridge setup.

-- while mobile

On Sat, Jun 6, 2015 at 10:05 AM, Jordan Wood <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

Okay, I've discovered an issue with doing it the way I described before.
The problem with keeping the safety enabled/disabled flag in the Javascript
code is that if you call it too early (before the Javascript object is set
up), then that setting gets lost. We are making this change as a hotfix to
our app, so I think I will leave what I have coded up for now, and figure

out the best way to do the internals later.

Reply to this email directly or view it on GitHub:

#132 (comment)

—
Reply to this email directly or view it on GitHub
#132 (comment)
.

---

Reply to this email directly or view it on GitHub:
#132 (comment)

[jordanpwood]

My changes are in PR 133.

[marcuswestin]

Closing after merge in #133. Thanks @jordanpwood!