resourceloader: Restore mw.loader.store update postponing logic
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 23 Aug 2018 02:16:06 +0000 (03:16 +0100)
committerRoan Kattouw <roan.kattouw@gmail.com>
Mon, 27 Aug 2018 21:16:05 +0000 (14:16 -0700)
* Document the conditional that disables mw.loader.store if
  localStorage is unavailable. `raw === undefined` is correct,
  but can be confusing given it can also be null, and we
  specifically don't want to disable the store in that case.
  We only disable the store if raw is undefined.

* Remove the call to mw.loader.store.update() in init().
  If 0 modules were to load on the current page, there is little
  value in flushing an empty `items` object.
  If any modules load, they will be set() in the store and
  schedule an update() at that time, which is preferred and avoids
  unexpected overhead.

* Remove store.enabled check from prune(). It is only called
  from update(), which checks it already.
* Remove boolean return from set(). Not used.

* Remove boolean return from prune(). Not used.

* Restore the 2-second setTimeout debounce from 2013 (c719401661e),
  which got lost in the 2015 refactor (4174b662f623).
  This makes the documentation true again.

* Document the store singleton as @private. It will still be
  indexed by JSDuck and listed in the sidebar (under "private"),
  but will not be listed on the homepage, and the class' page
  will have a notice about it being a private API.

Bug: T202598
Change-Id: Ic0d4a15c241df391ab5f824ca9e754c3938ea108

maintenance/jsduck/categories.json
resources/src/startup/mediawiki.js

index bebee85..4ec3460 100644 (file)
@@ -8,7 +8,6 @@
                                        "mw",
                                        "mw.Message",
                                        "mw.loader",
-                                       "mw.loader.store",
                                        "mw.html",
                                        "mw.html.Cdata",
                                        "mw.html.Raw",
index f9a69b8..30b223e 100644 (file)
                                 * modules and cache each of them separately, using each module's versioning scheme
                                 * to determine when the cache should be invalidated.
                                 *
+                                * @private
                                 * @singleton
                                 * @class mw.loader.store
                                 */
                                         * @return {string} String of concatenated vary conditions.
                                         */
                                        getVary: function () {
-                                               return [
-                                                       mw.config.get( 'skin' ),
-                                                       mw.config.get( 'wgResourceLoaderStorageVersion' ),
-                                                       mw.config.get( 'wgUserLanguage' )
-                                               ].join( ':' );
+                                               return mw.config.get( 'skin' ) + ':' +
+                                                       mw.config.get( 'wgResourceLoaderStorageVersion' ) + ':' +
+                                                       mw.config.get( 'wgUserLanguage' );
                                        },
 
                                        /**
                                                }
 
                                                try {
+                                                       // This a string we stored, or `null` if the key does not (yet) exist.
                                                        raw = localStorage.getItem( mw.loader.store.getStoreKey() );
                                                        // If we get here, localStorage is available; mark enabled
                                                        mw.loader.store.enabled = true;
+                                                       // If null, JSON.parse() will cast to string and re-parse, still null.
                                                        data = JSON.parse( raw );
                                                        if ( data && typeof data.items === 'object' && data.vary === mw.loader.store.getVary() ) {
                                                                mw.loader.store.items = data.items;
                                                        } );
                                                }
 
+                                               // If we get here, one of four things happened:
+                                               //
+                                               // 1. localStorage did not contain our store key.
+                                               //    This means `raw` is `null`, and we're on a fresh page view (cold cache).
+                                               //    The store was enabled, and `items` starts fresh.
+                                               //
+                                               // 2. localStorage contained parseable data under our store key,
+                                               //    but it's not applicable to our current context (see getVary).
+                                               //    The store was enabled, and `items` starts fresh.
+                                               //
+                                               // 3. JSON.parse threw (localStorage contained corrupt data).
+                                               //    This means `raw` contains a string.
+                                               //    The store was enabled, and `items` starts fresh.
+                                               //
+                                               // 4. localStorage threw (disabled or otherwise unavailable).
+                                               //    This means `raw` was never assigned.
+                                               //    We will disable the store below.
                                                if ( raw === undefined ) {
                                                        // localStorage failed; disable store
                                                        mw.loader.store.enabled = false;
-                                               } else {
-                                                       mw.loader.store.update();
                                                }
                                        },
 
                                         *
                                         * @param {string} module Module name
                                         * @param {Object} descriptor The module's descriptor as set in the registry
-                                        * @return {boolean} Module was set
                                         */
                                        set: function ( module, descriptor ) {
                                                var args, key, src;
 
                                                if ( !mw.loader.store.enabled ) {
-                                                       return false;
+                                                       return;
                                                }
 
                                                key = getModuleKey( module );
                                                                descriptor.templates ].indexOf( undefined ) !== -1
                                                ) {
                                                        // Decline to store
-                                                       return false;
+                                                       return;
                                                }
 
                                                try {
                                                                exception: e,
                                                                source: 'store-localstorage-json'
                                                        } );
-                                                       return false;
+                                                       return;
                                                }
 
                                                src = 'mw.loader.implement(' + args.join( ',' ) + ');';
                                                if ( src.length > mw.loader.store.MODULE_SIZE_MAX ) {
-                                                       return false;
+                                                       return;
                                                }
                                                mw.loader.store.items[ key ] = src;
                                                mw.loader.store.update();
-                                               return true;
                                        },
 
                                        /**
                                         * Iterate through the module store, removing any item that does not correspond
                                         * (in name and version) to an item in the module registry.
-                                        *
-                                        * @return {boolean} Store was pruned
                                         */
                                        prune: function () {
                                                var key, module;
 
-                                               if ( !mw.loader.store.enabled ) {
-                                                       return false;
-                                               }
-
                                                for ( key in mw.loader.store.items ) {
                                                        module = key.slice( 0, key.indexOf( '@' ) );
                                                        if ( getModuleKey( module ) !== key ) {
                                                                delete mw.loader.store.items[ key ];
                                                        }
                                                }
-                                               return true;
                                        },
 
                                        /**
                                                mw.loader.store.items = {};
                                                try {
                                                        localStorage.removeItem( mw.loader.store.getStoreKey() );
-                                               } catch ( ignored ) {}
+                                               } catch ( e ) {}
                                        },
 
                                        /**
                                         * Sync in-memory store back to localStorage.
                                         *
-                                        * This function debounces updates. When called with a flush already pending,
-                                        * the call is coalesced into the pending update. The call to
-                                        * localStorage.setItem will be naturally deferred until the page is quiescent.
+                                        * This function debounces updates. When called with a flush already pending, the
+                                        * scheduled flush is postponed. The call to localStorage.setItem will be keep
+                                        * being deferred until the page is quiescent for 2 seconds.
                                         *
                                         * Because localStorage is shared by all pages from the same origin, if multiple
                                         * pages are loaded with different module sets, the possibility exists that
-                                        * modules saved by one page will be clobbered by another. But the impact would
-                                        * be minor and the problem would be corrected by subsequent page views.
+                                        * modules saved by one page will be clobbered by another. The only impact of this
+                                        * is minor (merely a less efficient cache use) and the problem would be corrected
+                                        * by subsequent page views.
                                         *
                                         * @method
                                         */
                                        update: ( function () {
-                                               var hasPendingWrite = false;
+                                               var timer, hasPendingWrites = false;
 
                                                function flushWrites() {
                                                        var data, key;
-                                                       if ( !hasPendingWrite || !mw.loader.store.enabled ) {
+                                                       if ( !mw.loader.store.enabled ) {
                                                                return;
                                                        }
 
                                                                } );
                                                        }
 
-                                                       hasPendingWrite = false;
+                                                       hasPendingWrites = false;
                                                }
 
-                                               return function () {
-                                                       if ( !hasPendingWrite ) {
-                                                               hasPendingWrite = true;
+                                               function request() {
+                                                       // If another mw.loader.store.set()/update() call happens in the narrow
+                                                       // time window between requestIdleCallback() and flushWrites firing, ignore it.
+                                                       // It'll be saved by the already-scheduled flush.
+                                                       if ( !hasPendingWrites ) {
+                                                               hasPendingWrites = true;
                                                                mw.requestIdleCallback( flushWrites );
                                                        }
+                                               }
+
+                                               return function () {
+                                                       // Cancel the previous timer (if it hasn't fired yet)
+                                                       clearTimeout( timer );
+                                                       timer = setTimeout( request, 2000 );
                                                };
                                        }() )
                                }