From 49f78bf8a0f5c6e5d107d02a9e4923d7df821de4 Mon Sep 17 00:00:00 2001 From: Lars Date: Mon, 29 Mar 2021 18:36:20 +0200 Subject: [PATCH] Fix race condition which broke API access in some circumstances The function `r2WatcherContentStart` did exit in some circumstances (e.g. when there are few posts on the page), causing `Context.establish` to not run. --- lib/core/init.js | 3 --- lib/environment/foreground/context.js | 5 ++--- lib/utils/watchers.js | 28 ++++++++++++++------------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/core/init.js b/lib/core/init.js index 6de2a6c95b..5afa7cdb53 100644 --- a/lib/core/init.js +++ b/lib/core/init.js @@ -2,7 +2,6 @@ import { once } from 'lodash-es'; import { _loadI18n } from '../environment'; -import * as Context from '../environment/foreground/context'; import { BodyClasses, PagePhases, @@ -69,5 +68,3 @@ export const afterLoad: Promise = Promise.all([go, PagePhases.loadComplete // BodyClasses may have been added before document.body was ready Promise.all([onInit, PagePhases.bodyStart]).then(BodyClasses.addMissing); - -Promise.all([onInit, PagePhases.bodyStart]).then(() => { Context.establish(contentStart); }); diff --git a/lib/environment/foreground/context.js b/lib/environment/foreground/context.js index afa31cdca6..2b7649f94a 100644 --- a/lib/environment/foreground/context.js +++ b/lib/environment/foreground/context.js @@ -4,6 +4,7 @@ import { loggedInUser, loggedInUserHash, } from '../../utils/user'; +import { contentStart } from '../../utils/pagePhases'; import { waitForEvent } from '../../utils/dom'; export const data: {| @@ -18,9 +19,7 @@ export const data: {| pathname: location.pathname, }; -export function establish(contentStart: *) { - if (!location.protocol.startsWith('http')) return; - +if (location.protocol.startsWith('http')) { data.origin = location.origin; contentStart.then(() => { diff --git a/lib/utils/watchers.js b/lib/utils/watchers.js index 80e5759c81..c045248342 100644 --- a/lib/utils/watchers.js +++ b/lib/utils/watchers.js @@ -4,6 +4,7 @@ import { memoize, once } from 'lodash-es'; import { Thing } from './Thing'; import { isPageType } from './location'; import { isLastNodeInDOM, watchForFutureChildren, watchForFutureDescendants, waitForAttach, waitForDescendantChange, waitForSelectorMatch, watchForChildren } from './dom'; +import { contentLoaded } from './pagePhases'; import { MINUTE } from './time'; type WatcherOptions = {| immediate?: boolean, id?: mixed |}; @@ -119,7 +120,6 @@ export function registerPage(page: HTMLElement) { } const HIDE_FOLLOWING_CLASS = 'res-hide-following'; -let contentLoaded = false; const cleanHideClasses = () => { for (const ele of [...document.getElementsByClassName(HIDE_FOLLOWING_CLASS)]) ele.classList.remove(HIDE_FOLLOWING_CLASS); @@ -151,7 +151,7 @@ export async function r2WatcherContentStart() { const queue = []; let i = 0; // Reset every half minute so new things are processed fast when loading additional things via neverEndingReddit / "more comments" etc - setInterval(() => { if (contentLoaded) { queue.length = 0; i = 0; } }, 0.5 * MINUTE); + contentLoaded.then(() => { setInterval(() => { queue.length = 0; i = 0; }, 0.5 * MINUTE); }); return thing => { const n = i++; const check = queue[n] = async () => { @@ -209,7 +209,6 @@ export async function r2WatcherContentStart() { } cleanHideClasses(); - // console.log('hiding',last, _last) // Hide the following elements until 'contentLoaded', // to prevent them being painted without the 'immediate' watchers having been invoked on them do { _last.classList.add(HIDE_FOLLOWING_CLASS); } while ((_last = _last.parentElement) && _last !== container); @@ -219,25 +218,28 @@ export async function r2WatcherContentStart() { }; while (true) { // eslint-disable-line no-constant-condition - // Stop if the next phase has started by now - if (contentLoaded) return; - const thingElements = getReadyThingElements(); for (const e of thingElements) registerThing(e); if (thingElements.length >= initialProcessSize) return; - await Promise.all([ // eslint-disable-line no-await-in-loop - // Give the browser some time to load additional posts into the DOM before continuing if we've already processed some - thingElements.length ? (new Promise(res => { setTimeout(res, 100); })) : Promise.resolve(), - waitForDescendantChange(container, Thing.thingSelector), - ]); + try { + await Promise.race([ // eslint-disable-line no-await-in-loop + contentLoaded.then(() => Promise.reject()), // eslint-disable-line prefer-promise-reject-errors + Promise.all([ + // Give the browser some time to load additional posts into the DOM before continuing if we've already processed some + thingElements.length ? (new Promise(res => { setTimeout(res, 100); })) : Promise.resolve(), + waitForDescendantChange(container, Thing.thingSelector), + ]), + ]); + } catch (e) { + // `r2WatcherContentLoaded` has run by now + break; + } } } export function r2WatcherContentLoaded() { - contentLoaded = true; - cleanHideClasses(); // Things may be edited/loaded using the Reddit interface