Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise.resolve not resolving because of MutationObserver (iOS, WebApp) #666

Closed
rdzar opened this issue Jun 19, 2015 · 13 comments
Closed

Promise.resolve not resolving because of MutationObserver (iOS, WebApp) #666

rdzar opened this issue Jun 19, 2015 · 13 comments

Comments

@rdzar
Copy link

rdzar commented Jun 19, 2015

Debugging from events to Bluebird for a while now on this problem, as it seems to be very environment specific. On iOS 8.3 (iPhone 6 as iPod & iPad last gen.) added a web page to the homescreen, creating a web app produces the following bug.

When you're scrolling in the web app and you let it resolve something based on the tap event it will not resolve the promise for 10 seconds (it seems the websocket made it trigger after 10 seconds due to heartbeat). It seems difficult to hit this point at first, but with a web app full of products (40/50) and scrolling trough it, tapping while scrolling is being done very easily.

The MutationObserver will not fire for a while unless you do something else on the webapp. Mostly people will try to tap again, causing the previous promise to resolve while handeling the new one too. The user is suddenly served with its old and new request.

I've build a test app to show what's going on. Hopefully it gives some insides on what's happening.
http://wehandcraft.com/bluebirdtest/

If you add this page to the homescreen (webapp) in iOS, swipe up (scroll down) and tap a product you'll see Promise resolve on the bottom left corner. This means the promise is being resolved, but as you will see, nothing is happening. After a while (if you're lucky) or after a new gesture on the webpage it will say 'resolved'.

I've left some of the console.logs (haha, yup!) in the bluebird.js to give a bit of indication to what I think is the problem.

Hopefully we'll be able to find a fix for this somehow!

@benjamingr
Copy link
Collaborator

Does using setScheduler solve the issue (setting the scheduler to setTimeout

@rdzar
Copy link
Author

rdzar commented Jun 19, 2015

We've indeed fixed it using setImmediate and setTimeout now. Didn't notice the Promise.setScheduler function to be honest, so I'll use that in combination with these. MutationObserver seems to be the problem here. Not in Safari Mobile though, only in App mode.

@benjamingr
Copy link
Collaborator

@renedx I agree, I just wanted to assert that this problem is mitigated with

Promise.setScheduler(function(fn){
    setTimeout(fn, 0);)
});

Solves the issue to asset it's a scheduler issue.

@benjamingr
Copy link
Collaborator

Pinging @stefanpenner are you guys over at asap.js aware of this?

@benjamingr
Copy link
Collaborator

@petkaantonov a race between MutationObserver and setTimeout sounds reasonable or simply using another method for mobile devices. What do you say?

@stefanpenner
Copy link
Contributor

Pinging @stefanpenner are you guys over at asap.js aware of this?

sorta

one thing to note, setTimeout also gets throttled on iOS during scrolling. rAF during scroll is the only thing that doesn't get throttled, but it comes with other obvious crappy issues.

For a mobile app, I wrote something that would move between mutationObserver and rAF when a using touches for a scroll. Solution worked, but was crappy.

If I recall, MessageChannel/PostMessage also suffered from the same throttling during scroll.

@benjamingr
Copy link
Collaborator

I'm not sure how I feel about a rAF, are you sure it's the only reliable solution on mobile? @stefanpenner

@stefanpenner
Copy link
Contributor

It was ages ago. It's worth revisiting. It obviously comes with a major caveat and that is when the tab is out of focus/backgrounded it stops altogether.

In the past switching between rAF and mutation observer depending on if the user is scrolling has worked. But maybe message channel or similar now don't suffer the same throttling during scroll.

It's also worth noting, that iOS resume can play havoc, especially if it occurs during a scroll.


One somewhat related thing is that mutation observers also appear fail to flush if triggered directly from the ie11 console. That is, until the user focuses the webview, or invokes setTimeout...

I believe the above mentioned issues are not addressed as part of the asap.js solution.. But likely worth adding to the mix.

@rdzar
Copy link
Author

rdzar commented Jun 29, 2015

setImmediate seems to work fine on iOS WebApp, no complains anymore and as a fallback I use setTimeout. Tested the full scroll behavior on it, can't reproduce blocked promises.

  // See Bluebird #666
  Promise.setScheduler(function(fn) {
    if (typeof setImmediate !== "undefined") {
      setImmediate(fn);
    } else if (typeof setTimeout !== "undefined") {
      setTimeout(fn, 0);
    }
  });

@benjamingr
Copy link
Collaborator

@renedx thanks a lot for updating us. We discussed this offline and we were thinking about:

Our workaround looked something like (minus being in internal APIs):

if(window.navigator.standalone) {
     Promise.setScheduler(function(fn) {
           setTimeout(fn, 0); // or requestAnimationFrame, as stef panner pointed out.
     });
}

Is your code always run as a standalone app?

@rdzar
Copy link
Author

rdzar commented Jun 29, 2015

Correct, our application is recommended to work in standalone mode. We request people to do so whenever they open it.

@benjamingr
Copy link
Collaborator

Cool :)

@petkaantonov do you want me to make a PR for those 5 lines? It's just changing:

} else if (typeof MutationObserver !== "undefined") {

To

} else if ((typeof MutationObserver !== "undefined") && 
          (!typeof window !== "undefined" && window.navigator && window.navigator.standalone)) {

In:
https://github.com/petkaantonov/bluebird/blob/master/src/schedule.js#L7-L13

@petkaantonov
Copy link
Owner

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants