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

Add support for indexeddb sync in webworker #3578

Merged
merged 4 commits into from Apr 7, 2017
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -37,8 +37,19 @@
<body style="height: 100%;">
<section id="matrixchat" style="height: 100%;"></section>
<noscript>Sorry, Riot requires JavaScript to be enabled.</noscript>
<% for (var i=0; i < htmlWebpackPlugin.files.js.length; i++) {%>
<script src="<%= htmlWebpackPlugin.files.js[i] %>"></script>
<% for (var i=0; i < htmlWebpackPlugin.files.js.length; i++) {
// Not a particularly graceful way of not putting the indexeddb worker script
// into the main page
if (htmlWebpackPlugin.files.js[i].endsWith('indexeddb-worker.js')) {

This comment has been minimized.

Copy link
@Kegsay

Kegsay Apr 7, 2017

Contributor

I'm fairly sure Babel doesn't get applied to this file. If it doesn't, then you cannot use endsWith as that is ES6 only :( https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/String/endsWith

This comment has been minimized.

Copy link
@dbkr

dbkr Apr 7, 2017

Author Member

Good point, this probably would have broken on older Node.

%>
<script>
window.vector_indexeddb_worker_script = '<%= htmlWebpackPlugin.files.js[i] %>';
</script>
<%
continue;
}
%>
<script src="<%= htmlWebpackPlugin.files.js[i] %>"></script>
<% } %>
<img src="img/warning.svg" width="24" height="23" style="visibility: hidden; position: absolute; top: 0px; left: 0px;"/>
<audio id="messageAudio">
@@ -68,11 +68,15 @@ import url from 'url';
import {parseQs, parseQsFromFragment} from './url_utils';
import Platform from './platform';

import MatrixClientPeg from 'matrix-react-sdk/lib/MatrixClientPeg';

var lastLocationHashSet = null;

var CallHandler = require("matrix-react-sdk/lib/CallHandler");
CallHandler.setConferenceHandler(VectorConferenceHandler);

MatrixClientPeg.setIndexedDbWorkerScript(window.vector_indexeddb_worker_script);

function checkBrowserFeatures(featureList) {
if (!window.Modernizr) {
console.error("Cannot check features - Modernizr global is missing.");
@@ -0,0 +1,26 @@
/*
Copyright 2017 Vector Creations Ltd
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// The prescribed way of doing this import would be:
//import {IndexedDbStoreWorker} from 'matrix-js-sdk';
// However, this still pulls in all of the js-sdk and we only use a tiny fraction
// of it. It also causes an Olm error to appear because we don't have an Olm in scope.
// Instead, we do this:
import IndexedDbStoreWorker from 'matrix-js-sdk/lib/store/indexeddb-remote-worker';

This comment has been minimized.

Copy link
@Kegsay

Kegsay Apr 7, 2017

Contributor

Please don't advocate this style. I understand why you're doing it this case, but it prevents us from changing the position of files in the JS SDK. I don't believe we do pull in all of the JS SDK because of the optimisation pass which babel goes through in production mode. We should fix the Olm error rather than work around it like this.

It sounds like it will work if you do it properly, so please just do that.

This comment has been minimized.

Copy link
@dbkr

dbkr Apr 7, 2017

Author Member

Sort of - the optimisation may be able to remove the unreferenced classes etc. but anything that's run in the root of the script (like the Olm detection or polyfills) can't be removed.

How about a compromise: I've moved the worker out to a separate import in src. It still needs to be imported via matrix-js-sdk/lib/indexeddb-worker (because you can only have one default) but it keeps the worker separate from everything else because it is logically separate, ie. you'd never use the worker class and the rest of the SDK in the same script. We're still importing via an import script rather than reaching right into the guts of the js-sdk though.

This comment has been minimized.

Copy link
@Kegsay

Kegsay Apr 7, 2017

Contributor

SGTM, thank you.


const remoteWorker = new IndexedDbStoreWorker(postMessage);

onmessage = remoteWorker.onMessage;
@@ -6,6 +6,7 @@ var HtmlWebpackPlugin = require('html-webpack-plugin');
module.exports = {
entry: {
"bundle": "./src/vector/index.js",
"indexeddb-worker": "./src/vector/indexedbd-worker.js",

// We ship olm.js as a separate lump of javascript. This makes it get
// loaded via a separate <script/> tag in index.html (which loads it
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.