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.

Sign up for GitHub

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

Add support for indexeddb sync in webworker #3578

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f187a35
Add support for indexeddb sync in webworker
dbkr Apr 6, 2017
515b57b
Update for rename of worker
dbkr Apr 7, 2017
b65466b
Use dedicated indexeddb worker import
dbkr Apr 7, 2017
ec87003
Don't use endsWith as it'll break on old node
dbkr Apr 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/vector/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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">
Expand Down
4 changes: 4 additions & 0 deletions src/vector/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down
26 changes: 26 additions & 0 deletions src/vector/indexedbd-worker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, thank you.


const remoteWorker = new IndexedDbStoreWorker(postMessage);

onmessage = remoteWorker.onMessage;
1 change: 1 addition & 0 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down