Skip to content
This repository has been archived by the owner on Aug 9, 2021. It is now read-only.

Investigate if the iframe can be added back in with wider support. #267

Closed
zachferland opened this issue Jan 18, 2019 · 15 comments
Closed
Assignees
Milestone

Comments

@zachferland
Copy link
Contributor

zachferland commented Jan 18, 2019

(From Discord Chat) Have to address two problems, iframes may not work in dapp browsers and iframe proxy implementations are causing unpredictable behavior.

  • determine if issues in webviews in dapp browsers are also on android (Known to be issue on IOS)
  • understand the issue better in pinpoint, determine if just configs, browser limits/differences. And if long term constraints, are fallback solutions possible.
  • implement a rpc interface wrapper with postmessage over the storage module in ipfs, which allows you to pass in different storage layers. This an alternative to using ipfs-post-message-proxy which wraps the entire ipfs lib, creating a huge surface areas for issues. While alternatively the storage interface is extremely minimal. (Doing this is similar to what we already did for orbit-db, wrapped just the storage layer, instead of entire lib)
@zachferland zachferland added this to the Sprint 12 milestone Jan 18, 2019
@zachferland zachferland self-assigned this Jan 18, 2019
@zachferland
Copy link
Contributor Author

zachferland commented Jan 18, 2019

Started an issue over at status, adding the follow up there - status-im/status-mobile#7292

As that issues describes it may be when using IndexedDB in a iframe of differing origin in a webview on IOS (safari, but not general safari browser).

If this is the case, it may be possible to implement fallbacks to localstorage. Fairly easy for orbitdb since they already use and abstract-leveldown compliant store interface, and there exist libs to use localstorage over the same interface. For ipfs not sure at the moment, don't think they implement the same interface, although you can pass in your own storage layers, so something we could technically implement (would have to determine the effort).

Would be some performance loss using localstorage. Negligible for orbitedb, but could be noticeable in ipfs. Indexeddb much more friendly for data blobs/binary data and larger data.

Notes for other work

  • looks like web workers using indexeddb would suffer the same issue
  • sounds like localstorage doesn't persist in webviews (mobile) so either way our lib is going to be really slow in dapp browsers. (Don't know if this another webview constraint or config or something)
  • This also includes on some additional constraints - https://notes.status.im/s/rJ0SO5tpQ

@zachferland
Copy link
Contributor Author

zachferland commented Jan 18, 2019

Need to setup minimized example that can test exact assumption and issues. Also so that we have something to communicate to dapp browser teams for their own testing. First to test indexeddb in iframe with different origin. Also test localstorage persistence and eventually webworker functionality available. Creating another story for this, will be useful for testing assumptions in later features as well.

@zachferland
Copy link
Contributor Author

Here is a minimal example to quickly test these kind of assumption in the future - https://github.com/3box/3box-iframe-browser-test

@zachferland
Copy link
Contributor Author

Was able to confirm that the original concern is true generally in safari not just webviews on IOS.

So in safari and thus all mobile dapp browser on IOS, you can not use IndexedDB in an iframe with a differing origin. You can use localstorage though.

@zachferland
Copy link
Contributor Author

So the solution is to implement proper fallbacks upon detecting the lib was loaded in safari. The simplest is to turn loading the iframe off/on based on if safari or not. So we will get iframe benefits across everything except for mobile dapp browsers on IOS.

A second option, is the scenario described above, where the iframe is always used but the iframe implements a fallback to localstorage if indexedDB is not available. This is a greater effort (as described above) and the tradeoffs are not clear yet. Localstorage could see a visible performance drop over indexeddb for ipfs (but on the other hand these data sets are not large, so maybe negligible). Would get benefits of shared local store, with decreased round trips and bandwidth. Increase implementation complexity.

Optimal path would be option 1, with option 2 later revisited. Also implementing a post message proxy over only the storage layer of ipfs (as described by original issue) may also make option 2 easier.

@zachferland
Copy link
Contributor Author

@michaelsena @oed any thoughts above? make sense to you guys to still implement the iframe given the constraint above. To me the benefits across other environments still make it worth it, as well as the possibility of working towards decent solution in safari

@oed
Copy link
Member

oed commented Jan 19, 2019

@zachferland Did you get any clarity on localstorage, and indexDB persistance? Option 2 seems unneeded if that doesn't work on ios anyway.
Could you use localstorage in the webworker? Not sure if you tried this though.
Seems like 1 is a good option, but we need to decide on a time line for it.

@zachferland
Copy link
Contributor Author

@oed testing that next, but likely not persistent at moment (in status at least, will determine if general) and then local storage likely works in webworker, but will get that too.

Timeline

Option 1 is simple, like half day or less. A bit more work to then try out wrapping just ipfs storage in a proxy post message, maybe like a day (to at least confirm it mostly works). Also later option 2 aligns with this work, so option 2 is a progression in the future if we want to try it ever. Lastly the most work is testing again, to really determine if working well, either a week of slow coordination like last time (of course not a week of constant work), or maybe a few days spent creating some integration tests.

@oed
Copy link
Member

oed commented Jan 21, 2019

@zachferland ok thanks. I don't think we should do option 1 without doing it the ipfs storage way. I'm pretty sure the ipfs proxy was what caused really stragne problems for orbitdb.
Don't remember if I told you about it, but basically entries from DB A could be added to DB B and vice versa for unknown reasons.

@zachferland
Copy link
Contributor Author

From status issue, local storage does not persist, (likely indexeddb too, but will confirm). But local storage support will be added in the future. This was my understanding as well, that this was something specific to config & implementation, that could be supported.

@oed
Copy link
Member

oed commented Jan 21, 2019

@zachferland ok, I guess that's true for coinbase wallet as well?

@zachferland
Copy link
Contributor Author

for local storage not necessarily, because like mentioned I think it can be implemented/configured by app, so maybe implemented in coinbase, but probs not, so going to try in coinbase wallet later when I revisit

@zachferland
Copy link
Contributor Author

Ok, actually looks like status wallet DOES support local storage, at least from my tests. Can load coinbase wallet right now, so can't test

@zachferland
Copy link
Contributor Author

Also DOES work for indexedDB (status wallet, don't know about coinbase wallet)

@zachferland
Copy link
Contributor Author

With enough clarity to move forward, going to close this issue, and add stories for bringing the iframe back.

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

No branches or pull requests

2 participants