ronantakizawa/codereview-bench huggingface dataset:
Repository: mozilla/pdf.js
File: web/base_pdf_page_view.js (JavaScript)

Code before:
        cont();
      };
      return;
    }
    cont();
  };

  _resetCanvas() {
    const { canvas } = this;
    if (!canvas) {
      return;
    }
    canvas.remove();
    canvas.width = canvas.height = 0;
    this.canvas = null;
    if (this.#tempCanvas) {
      this.#tempCanvas.width = this.#tempCanvas.height = 0;
      this.#tempCanvas = null;
    }
  }

  async _drawCanvas(options, onCancel, onFinish) {
    const renderTask = (this.renderTask = this.pdfPage.render(options));
    if (this.pdfPage._pdfBug && globalThis.StepperManager?.enabled) {
      this.#minDurationToUpdateCanvas = 0;
    }
    renderTask.onContinue = this.#renderContinueCallback;
    renderTask.onError = error => {
      if (error instanceof RenderingCancelledException) {
        onCancel();
        this.#renderError = null;
      }
    };

    let error = null;
    try {
      await renderTask.promise;
      this.#showCanvas?.(true);
    } catch (e) {
      // When zooming with a `drawingDelay` set, avoid temporarily showing
      // a black canvas if rendering was cancelled before the `onContinue`-
      // callback had been invoked at least once.
      if (e instanceof RenderingCancelledException) {
        return;
      }
      error = e;

      this.#showCanvas?.(true);
    } finally {
      this.#renderError = error;


Review comment (bug):
Do we need to check this here, can't we set the option together with the others when loading the debugger?
https://github.com/mozilla/pdf.js/blob/efc5c3c2314840098551c325cd7e31d60ef3bd53/web/app.js#L327-L337

Code after:
      this.resume = () => {
        this.renderingState = RenderingStates.RUNNING;
        cont();
      };
      return;
    }
    cont();
  };

  _resetCanvas() {
    const { canvas } = this;
    if (!canvas) {
      return;
    }
    canvas.remove();
    canvas.width = canvas.height = 0;
    this.canvas = null;
    this.#resetTempCanvas();
  }

  #resetTempCanvas() {
    if (this.#tempCanvas) {
      this.#tempCanvas.width = this.#tempCanvas.height = 0;
      this.#tempCanvas = null;
    }
  }

  async _drawCanvas(options, onCancel, onFinish) {
    const renderTask = (this.renderTask = this.pdfPage.render(options));
    renderTask.onContinue = this.#renderContinueCallback;
    renderTask.onError = error => {
      if (error instanceof RenderingCancelledException) {
        onCancel();
        this.#renderError = null;
      }
    };

    let error = null;
    try {
      await renderTask.promise;
      this.#showCanvas?.(true);
    } catch (e) {
      // When zooming with a `drawingDelay` set, avoid temporarily showing
      // a black canvas if rendering was cancelled before the `onContinue`-
      // callback had been invoked at least once.
      if (e instanceof RenderingCancelledException) {
        return;
      }
      error = e;

      this.#showCanvas?.(true);
Repository: mozilla/pdf.js
File: web/base_pdf_page_view.js (JavaScript)

Code before:
        this.renderingState = RenderingStates.RUNNING;
        cont();
      };
      return;
    }
    cont();
  };

  _resetCanvas() {
    const { canvas } = this;
    if (!canvas) {
      return;
    }
    canvas.remove();
    canvas.width = canvas.height = 0;
    this.canvas = null;
    if (this.#tempCanvas) {
      this.#tempCanvas.width = this.#tempCanvas.height = 0;
      this.#tempCanvas = null;
    }
  }

  async _drawCanvas(options, onCancel, onFinish) {
    const renderTask = (this.renderTask = this.pdfPage.render(options));
    renderTask.onContinue = this.#renderContinueCallback;
    renderTask.onError = error => {
      if (error instanceof RenderingCancelledException) {
        onCancel();
        this.#renderError = null;
      }
    };

    let error = null;
    try {
      await renderTask.promise;
      this.#showCanvas?.(true);
    } catch (e) {
      // When zooming with a `drawingDelay` set, avoid temporarily showing
      // a black canvas if rendering was cancelled before the `onContinue`-
      // callback had been invoked at least once.
      if (e instanceof RenderingCancelledException) {
        return;
      }
      error = e;

      this.#showCanvas?.(true);
    } finally {
      this.#renderError = error;

      // The renderTask may have been replaced by a new one, so only remove
      // the reference to the renderTask if it matches the one that is

Review comment (refactor):
Should we perhaps add a new helper method, e.g. `#resetTempCanvas`, to avoid repeating this code verbatim?

Code after:
      this.renderingState = RenderingStates.PAUSED;
      this.resume = () => {
        this.renderingState = RenderingStates.RUNNING;
        cont();
      };
      return;
    }
    cont();
  };

  _resetCanvas() {
    const { canvas } = this;
    if (!canvas) {
      return;
    }
    canvas.remove();
    canvas.width = canvas.height = 0;
    this.canvas = null;
    this.#resetTempCanvas();
  }

  #resetTempCanvas() {
    if (this.#tempCanvas) {
      this.#tempCanvas.width = this.#tempCanvas.height = 0;
      this.#tempCanvas = null;
    }
  }

  async _drawCanvas(options, onCancel, onFinish) {
    const renderTask = (this.renderTask = this.pdfPage.render(options));
    renderTask.onContinue = this.#renderContinueCallback;
    renderTask.onError = error => {
      if (error instanceof RenderingCancelledException) {
        onCancel();
        this.#renderError = null;
      }
    };

    let error = null;
    try {
      await renderTask.promise;
      this.#showCanvas?.(true);
    } catch (e) {
      // When zooming with a `drawingDelay` set, avoid temporarily showing
      // a black canvas if rendering was cancelled before the `onContinue`-
      // callback had been invoked at least once.
      if (e instanceof RenderingCancelledException) {
        return;
      }
      error = e;
Repository: mozilla/pdf.js
File: src/core/jpx.js (JavaScript)

Code before:
          OpenJPEG({
            warn,
            instantiateWasm: this.#instantiateWasm.bind(this, resolve),
          })
        );
      }
      this.#modulePromise = Promise.race(promises);
    }
    const module = await this.#modulePromise;

    if (!module) {
      throw new JpxError("OpenJPEG failed to initialize");
    }
    let ptr;

    try {
      const size = bytes.length;
      ptr = module._malloc(size);
      module.writeArrayToMemory(bytes, ptr);
      const ret = module._jp2_decode(
        ptr,
        size,
        numComponents > 0 ? numComponents : 0,
        !!isIndexedColormap,
        !!smaskInData,
        Math.max(0, Math.floor(reducePower ?? 0))
      );
      if (ret) {
        const { errorMessages } = module;
        if (errorMessages) {
          delete module.errorMessages;
          throw new JpxError(errorMessages);
        }
        throw new JpxError("Unknown error");
      }
      const { imageData } = module;
      module.imageData = null;

      return imageData;
    } finally {
      if (ptr) {
        module._free(ptr);
      }
    }
  }

  static cleanup() {
    this.#modulePromise = null;
  }

  static parseImageProperties(stream) {

Review comment (suggestion):
Can this be simplified, since the default value is zero (above) and it seems (as far as I can tell) that `getReducePowerForJPX` will always return a positive integer?

Would the following suffice?
```suggestion
        reducePower
```

Code after:
          OpenJPEG({
            warn,
            instantiateWasm: this.#instantiateWasm.bind(this, resolve),
          })
        );
      }
      this.#modulePromise = Promise.race(promises);
    }
    const module = await this.#modulePromise;

    if (!module) {
      throw new JpxError("OpenJPEG failed to initialize");
    }
    let ptr;

    try {
      const size = bytes.length;
      ptr = module._malloc(size);
      module.writeArrayToMemory(bytes, ptr);
      const ret = module._jp2_decode(
        ptr,
        size,
        numComponents > 0 ? numComponents : 0,
        !!isIndexedColormap,
        !!smaskInData,
        reducePower
      );
      if (ret) {
        const { errorMessages } = module;
        if (errorMessages) {
          delete module.errorMessages;
          throw new JpxError(errorMessages);
        }
        throw new JpxError("Unknown error");
      }
      const { imageData } = module;
      module.imageData = null;

      return imageData;
    } finally {
      if (ptr) {
        module._free(ptr);
      }
    }
  }

  static cleanup() {
    this.#modulePromise = null;
  }

  static parseImageProperties(stream) {
Repository: mozilla/pdf.js
File: src/display/display_utils.js (JavaScript)

Code before:
    maxPixels = OutputScale.capPixels(maxPixels, capAreaFactor);
    if (maxPixels > 0) {
      maxAreaScale = Math.sqrt(maxPixels / (width * height));
    }
    if (maxDim !== -1) {
      maxWidthScale = maxDim / width;
      maxHeightScale = maxDim / height;
    }
    const maxScale = Math.min(maxAreaScale, maxWidthScale, maxHeightScale);

    if (this.sx > maxScale || this.sy > maxScale) {
      this.sx = maxScale;
      this.sy = maxScale;
      return true;
    }
    return false;
  }

  static get pixelRatio() {
    return globalThis.devicePixelRatio || 1;
  }

  static capPixels(maxPixels, capAreaFactor) {
    if (capAreaFactor >= 0) {
      const winPixels =
        typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")
          ? Math.ceil(
              window.innerWidth *
                window.innerHeight *
                this.pixelRatio ** 2 *
                (1 + capAreaFactor / 100)
            )
          : Math.ceil(
              window.screen.availWidth *
                window.screen.availHeight *
                this.pixelRatio ** 2 *
                (1 + capAreaFactor / 100)
            );
      return maxPixels > 0 ? Math.min(maxPixels, winPixels) : winPixels;
    }
    return maxPixels;
  }
}

// See https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types
// to know which types are supported by the browser.
const SupportedImageMimeTypes = [
  "image/apng",
  "image/avif",
  "image/bmp",
  "image/gif",

Review comment (refactor):
could we simplify into `Math.ceil((typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING") ? window.innerWidth * window.innerHeight  :  window.screen.availWidth * window.screen.availHeight)...` ?

Code after:
    maxPixels = OutputScale.capPixels(maxPixels, capAreaFactor);
    if (maxPixels > 0) {
      maxAreaScale = Math.sqrt(maxPixels / (width * height));
    }
    if (maxDim !== -1) {
      maxWidthScale = maxDim / width;
      maxHeightScale = maxDim / height;
    }
    const maxScale = Math.min(maxAreaScale, maxWidthScale, maxHeightScale);

    if (this.sx > maxScale || this.sy > maxScale) {
      this.sx = maxScale;
      this.sy = maxScale;
      return true;
    }
    return false;
  }

  static get pixelRatio() {
    return globalThis.devicePixelRatio || 1;
  }

  static capPixels(maxPixels, capAreaFactor) {
    if (capAreaFactor >= 0) {
      const winPixels = Math.ceil(
        (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")
          ? window.innerWidth * window.innerHeight
          : window.screen.availWidth * window.screen.availHeight) *
          this.pixelRatio ** 2 *
          (1 + capAreaFactor / 100)
      );
      return maxPixels > 0 ? Math.min(maxPixels, winPixels) : winPixels;
    }
    return maxPixels;
  }
}

// See https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types
// to know which types are supported by the browser.
const SupportedImageMimeTypes = [
  "image/apng",
  "image/avif",
  "image/bmp",
  "image/gif",
  "image/jpeg",
  "image/png",
  "image/svg+xml",
  "image/webp",
  "image/x-icon",
];
Repository: mozilla/pdf.js
File: test/integration/document_properties_spec.mjs (JavaScript)

Code before:
import { closePages, loadAndWait } from "./test_utils.mjs";

const FIELDS = [
  "fileName",
  "fileSize",
  "title",
  "author",
  "subject",
  "keywords",
  "creationDate",
  "modificationDate",
  "creator",
  "producer",
  "version",
  "pageCount",
  "pageSize",
  "linearized",
];

// Unicode bidi isolation characters, Fluent adds these markers to the text.
const FSI = "\u2068";
const PDI = "\u2069";

describe("PDFDocumentProperties", () => {
  function waitForDocumentProperties(page) {
    return page.evaluate(() => [
      new Promise(resolve => {
        const { eventBus } = window.PDFViewerApplication;
        eventBus.on("documentpropertiesrendered", () => resolve, {
          once: true,
        });
      }),
    ]);
  }

  async function getFieldProperties(page) {
    const promises = [];

    for (const name of FIELDS) {
      promises.push(
        page.evaluate(
          n => [n, document.getElementById(`${n}Field`).textContent],
          name
        )
      );
    }
    return Object.fromEntries(await Promise.all(promises));
  }

  describe("Document with both /Info and /Metadata", () => {
    let pages;

Review comment (suggestion):
You should use `createPromise` (and `awaitPromise` when awaiting). When awaiting, you're awaiting on the evaluate itself not on the promise you created in the browser.

Code after:
import { closePages, FSI, loadAndWait, PDI } from "./test_utils.mjs";

const FIELDS = [
  "fileName",
  "fileSize",
  "title",
  "author",
  "subject",
  "keywords",
  "creationDate",
  "modificationDate",
  "creator",
  "producer",
  "version",
  "pageCount",
  "pageSize",
  "linearized",
];

describe("PDFDocumentProperties", () => {
  async function getFieldProperties(page) {
    const promises = [];

    for (const name of FIELDS) {
      promises.push(
        page.evaluate(
          n => [n, document.getElementById(`${n}Field`).textContent],
          name
        )
      );
    }
    return Object.fromEntries(await Promise.all(promises));
  }

  describe("Document with both /Info and /Metadata", () => {
    let pages;

    beforeEach(async () => {
      pages = await loadAndWait("basicapi.pdf", ".textLayer .endOfContent");
    });

    afterEach(async () => {
      await closePages(pages);
    });

    it("must check that the document properties dialog has the correct information", async () => {
      await Promise.all(
        pages.map(async ([browserName, page]) => {
          await page.click("#secondaryToolbarToggleButton");
          await page.waitForSelector("#secondaryToolbar", { hidden: false });
Repository: mozilla/pdf.js
File: test/integration/document_properties_spec.mjs (JavaScript)

Code before:
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

import { closePages, loadAndWait } from "./test_utils.mjs";

const FIELDS = [
  "fileName",
  "fileSize",
  "title",
  "author",
  "subject",
  "keywords",
  "creationDate",
  "modificationDate",
  "creator",
  "producer",
  "version",
  "pageCount",
  "pageSize",
  "linearized",
];

// Unicode bidi isolation characters, Fluent adds these markers to the text.
const FSI = "\u2068";
const PDI = "\u2069";

describe("PDFDocumentProperties", () => {
  function waitForDocumentProperties(page) {
    return page.evaluate(() => [
      new Promise(resolve => {
        const { eventBus } = window.PDFViewerApplication;
        eventBus.on("documentpropertiesrendered", () => resolve, {
          once: true,
        });
      }),
    ]);
  }

  async function getFieldProperties(page) {
    const promises = [];

    for (const name of FIELDS) {
      promises.push(
        page.evaluate(
          n => [n, document.getElementById(`${n}Field`).textContent],
          name
        )
      );
    }
    return Object.fromEntries(await Promise.all(promises));

Review comment (suggestion):
We should add these two constants in test_utils since they're used too in signature_editor_spec. Maybe something better would be to add a function `makeFluentString` or something like in order to add the markers at the right place without having to do it manually.

Code after:
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

import { closePages, FSI, loadAndWait, PDI } from "./test_utils.mjs";

const FIELDS = [
  "fileName",
  "fileSize",
  "title",
  "author",
  "subject",
  "keywords",
  "creationDate",
  "modificationDate",
  "creator",
  "producer",
  "version",
  "pageCount",
  "pageSize",
  "linearized",
];

describe("PDFDocumentProperties", () => {
  async function getFieldProperties(page) {
    const promises = [];

    for (const name of FIELDS) {
      promises.push(
        page.evaluate(
          n => [n, document.getElementById(`${n}Field`).textContent],
          name
        )
      );
    }
    return Object.fromEntries(await Promise.all(promises));
  }

  describe("Document with both /Info and /Metadata", () => {
    let pages;

    beforeEach(async () => {
      pages = await loadAndWait("basicapi.pdf", ".textLayer .endOfContent");
    });

    afterEach(async () => {
      await closePages(pages);
    });

    it("must check that the document properties dialog has the correct information", async () => {
      await Promise.all(
Repository: mozilla/pdf.js
File: gulpfile.mjs (JavaScript)

Code before:
          `${chromePrefs.properties[key].default} !== ${value}`
      );
    }
  }

  // Verify that preferences_schema.json does not contain entries that are not
  // in webPrefs (app_options.js).
  for (const key of chromePrefsKeys) {
    if (!(key in webPrefs)) {
      ret = false;
      console.log(
        `Warning: ${chromePrefsPath} contains an unrecognized pref: ${key}. ` +
          `Remove it, or prepend "DEPRECATED. " and add migration logic to ` +
          `extensions/chromium/options/migration.js and web/chromecom.js.`
      );
    }
  }
  return ret;
}

function createMainBundle(defines) {
  const mainFileConfig = createWebpackConfig(defines, {
    filename:
      defines.MINIFIED && !defines.MOZCENTRAL && !defines.GECKOVIEW
        ? "pdf.min.mjs"
        : "pdf.mjs",
    library: {
      type: "module",
    },
  });
  return gulp
    .src("./src/pdf.js", { encoding: false })
    .pipe(webpack2Stream(mainFileConfig));
}

function createScriptingBundle(defines, extraOptions = undefined) {
  const scriptingFileConfig = createWebpackConfig(
    defines,
    {
      filename: "pdf.scripting.mjs",
      library: {
        type: "module",
      },
    },
    extraOptions
  );
  return gulp
    .src("./src/pdf.scripting.js", { encoding: false })
    .pipe(webpack2Stream(scriptingFileConfig));
}


Review comment (suggestion):
Can't we update to use the new filename in the HTML instead?
https://github.com/mozilla/pdf.js/blob/b070a502c33aa08788e31e8f2afb675df1405374/web/viewer-snippet-firefox-extension.html#L2

Code after:
          `${chromePrefs.properties[key].default} !== ${value}`
      );
    }
  }

  // Verify that preferences_schema.json does not contain entries that are not
  // in webPrefs (app_options.js).
  for (const key of chromePrefsKeys) {
    if (!(key in webPrefs)) {
      ret = false;
      console.log(
        `Warning: ${chromePrefsPath} contains an unrecognized pref: ${key}. ` +
          `Remove it, or prepend "DEPRECATED. " and add migration logic to ` +
          `extensions/chromium/options/migration.js and web/chromecom.js.`
      );
    }
  }
  return ret;
}

function createMainBundle(defines) {
  const mainFileConfig = createWebpackConfig(defines, {
    filename:
      defines.MINIFIED && !defines.MOZCENTRAL ? "pdf.min.mjs" : "pdf.mjs",
    library: {
      type: "module",
    },
  });
  return gulp
    .src("./src/pdf.js", { encoding: false })
    .pipe(webpack2Stream(mainFileConfig));
}

function createScriptingBundle(defines, extraOptions = undefined) {
  const scriptingFileConfig = createWebpackConfig(
    defines,
    {
      filename: "pdf.scripting.mjs",
      library: {
        type: "module",
      },
    },
    extraOptions
  );
  return gulp
    .src("./src/pdf.scripting.js", { encoding: false })
    .pipe(webpack2Stream(scriptingFileConfig));
}

function createSandboxExternal(defines) {
  const licenseHeader = fs.readFileSync("./src/license_header.js").toString();
Repository: mozilla/pdf.js
File: gulpfile.mjs (JavaScript)

Code before:
    PDF_SCRIPTING_JS_SOURCE: fs.readFileSync(scriptingPath).toString(),
  };
  fs.unlinkSync(scriptingPath);

  const sandboxFileConfig = createWebpackConfig(
    sandboxDefines,
    {
      filename: sandboxDefines.MINIFIED
        ? "pdf.sandbox.min.mjs"
        : "pdf.sandbox.mjs",
      library: {
        type: "module",
      },
    },
    extraOptions
  );

  return gulp
    .src("./src/pdf.sandbox.js", { encoding: false })
    .pipe(webpack2Stream(sandboxFileConfig));
}

function createWorkerBundle(defines) {
  const workerFileConfig = createWebpackConfig(defines, {
    filename:
      defines.MINIFIED && !defines.MOZCENTRAL && !defines.GECKOVIEW
        ? "pdf.worker.min.mjs"
        : "pdf.worker.mjs",
    library: {
      type: "module",
    },
  });
  return gulp
    .src("./src/pdf.worker.js", { encoding: false })
    .pipe(webpack2Stream(workerFileConfig));
}

function createWebBundle(defines, options) {
  const viewerFileConfig = createWebpackConfig(
    defines,
    {
      filename: "viewer.mjs",
      library: {
        type: "module",
      },
    },
    {
      defaultPreferencesDir: options.defaultPreferencesDir,
    }
  );
  return gulp

Review comment (suggestion):
Similarly, can we change the following instead?
https://github.com/mozilla/pdf.js/blob/b070a502c33aa08788e31e8f2afb675df1405374/web/app_options.js#L515

Code after:
  fs.unlinkSync(scriptingPath);

  const sandboxFileConfig = createWebpackConfig(
    sandboxDefines,
    {
      filename: sandboxDefines.MINIFIED
        ? "pdf.sandbox.min.mjs"
        : "pdf.sandbox.mjs",
      library: {
        type: "module",
      },
    },
    extraOptions
  );

  return gulp
    .src("./src/pdf.sandbox.js", { encoding: false })
    .pipe(webpack2Stream(sandboxFileConfig));
}

function createWorkerBundle(defines) {
  const workerFileConfig = createWebpackConfig(defines, {
    filename:
      defines.MINIFIED && !defines.MOZCENTRAL
        ? "pdf.worker.min.mjs"
        : "pdf.worker.mjs",
    library: {
      type: "module",
    },
  });
  return gulp
    .src("./src/pdf.worker.js", { encoding: false })
    .pipe(webpack2Stream(workerFileConfig));
}

function createWebBundle(defines, options) {
  const viewerFileConfig = createWebpackConfig(
    defines,
    {
      filename: "viewer.mjs",
      library: {
        type: "module",
      },
    },
    {
      defaultPreferencesDir: options.defaultPreferencesDir,
    }
  );
  return gulp
    .src("./web/viewer.js", { encoding: false })
    .pipe(webpack2Stream(viewerFileConfig));