Skip to content

Commit

Permalink
Fix race condition which broke API access in some circumstances
Browse files Browse the repository at this point in the history
The function `r2WatcherContentStart` did exit in some circumstances (e.g. when there are few posts on the page), causing `Context.establish` to not run.
  • Loading branch information
larsjohnsen committed Mar 29, 2021
1 parent 7106944 commit 49f78bf
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
3 changes: 0 additions & 3 deletions lib/core/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import { once } from 'lodash-es';
import { _loadI18n } from '../environment';
import * as Context from '../environment/foreground/context';
import {
BodyClasses,
PagePhases,
Expand Down Expand Up @@ -69,5 +68,3 @@ export const afterLoad: Promise<void> = 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); });
5 changes: 2 additions & 3 deletions lib/environment/foreground/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
loggedInUser,
loggedInUserHash,
} from '../../utils/user';
import { contentStart } from '../../utils/pagePhases';
import { waitForEvent } from '../../utils/dom';

export const data: {|
Expand All @@ -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(() => {
Expand Down
28 changes: 15 additions & 13 deletions lib/utils/watchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 |};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down

0 comments on commit 49f78bf

Please sign in to comment.