Skip to content

Commit

Permalink
Reland "Mojo JS: Use BigInt consistently for 64-bit int fields"
Browse files Browse the repository at this point in the history
This is a reland of ea79bdd

Fix is in the delta from PS1 -> PS3

Original change's description:
> Mojo JS: Use BigInt consistently for 64-bit int fields
>
> This changes 64-bit integer fields to be deserialized as
> BigInt values consistently, rather than sometimes being
> BigInts and sometimes being Numbers.
>
> Fixed: 1102217,1009262
> Change-Id: I9cc8d767058938e9c39f62f139737e512f47cdb2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466398
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Reviewed-by: Becca Hughes <beccahughes@chromium.org>
> Reviewed-by: Shik Chen <shik@chromium.org>
> Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
> Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> Commit-Queue: Ken Rockot <rockot@google.com>
> Cr-Commit-Position: refs/heads/master@{#818611}

Change-Id: I393a33b44b1a3e1746f7b9ee021dbe06c3ff7b07
Tbr: shik@chromium.org
Tbr: fdoray@chromium.org
Tbr: ryansturm@chromium.org
Tbr: mahmadi@chromium.org
Tbr: beccahughes@chromium.org
Tbr: oksamyt@chromium.org
Tbr: ortuno@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485749
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: Dan H <harringtond@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Auto-Submit: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#819004}
  • Loading branch information
krockot authored and Commit Bot committed Oct 20, 2020
1 parent 828f6e4 commit 0092862
Show file tree
Hide file tree
Showing 28 changed files with 144 additions and 212 deletions.
5 changes: 3 additions & 2 deletions chrome/browser/resources/discards/database_tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -400,11 +400,12 @@ Polymer({
if (feature.useTimestamp) {
const nowSecondsFromEpoch = Math.round(Date.now() / 1000);
return 'Used ' +
durationToString(nowSecondsFromEpoch - feature.useTimestamp);
durationToString(
Number(BigInt(nowSecondsFromEpoch) - feature.useTimestamp));
}

if (feature.observationDuration) {
return secondsToString(feature.observationDuration);
return secondsToString(Number(feature.observationDuration));
}

return 'N/A';
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/resources/discards/discards_tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,9 @@ Polymer({
case mojom.LifecycleUnitState.DISCARDED:
return 'discarded (' + this.discardReasonToString_(reason) + ')' +
((reason === mojom.LifecycleUnitDiscardReason.URGENT) ? ' at ' +
// Must convert since Date constructor takes
// milliseconds.
(new Date(stateChangeTime.microseconds / 1000))
.toLocaleString() :
// Must convert since Date constructor takes milliseconds.
(new Date(Number(stateChangeTime.microseconds) / 1000)
.toLocaleString()) :
'');
}
assertNotReached('Unknown lifecycle state: ' + state);
Expand Down
21 changes: 12 additions & 9 deletions chrome/browser/resources/discards/graph_doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class ToolTip {
/** @implements {d3.ForceNode} */
class GraphNode {
constructor(id) {
/** @type {number} */
/** @type {bigint} */
this.id = id;
/** @type {string} */
this.color = 'black';
Expand Down Expand Up @@ -316,7 +316,7 @@ class GraphNode {
return -200;
}

/** @return {!Array<number>} an array of node ids. */
/** @return {!Array<bigint>} an array of node ids. */
get linkTargets() {
return [];
}
Expand All @@ -326,19 +326,22 @@ class GraphNode {
* things, but be owned by exactly one (per relationship type). As such, the
* relationship is expressed on the *owned* object. These links are drawn with
* an arrow at the beginning of the link, pointing to the owned object.
* @return {!Array<number>} an array of node ids.
* @return {!Array<bigint>} an array of node ids.
*/
get dashedLinkTargets() {
return [];
}

/**
* Selects a color string from an id.
* @param {number} id The id the returned color is selected from.
* @param {bigint} id The id the returned color is selected from.
* @return {string}
*/
selectColor(id) {
return d3.schemeSet3[Math.abs(id) % 12];
if (id < 0) {
id = -id;
}
return d3.schemeSet3[Number(id % BigInt(12))];
}
}

Expand Down Expand Up @@ -627,7 +630,7 @@ class Graph {
*/
this.dashedLinkGroup_ = null;

/** @private {!Map<number, !GraphNode>} */
/** @private {!Map<bigint, !GraphNode>} */
this.nodes_ = new Map();

/**
Expand Down Expand Up @@ -846,7 +849,7 @@ class Graph {
*/
nodeDescriptions_(nodeDescriptions) {
for (const nodeId in nodeDescriptions) {
const node = this.nodes_.get(Number.parseInt(nodeId, 10));
const node = this.nodes_.get(BigInt(nodeId));
if (node && node.tooltip) {
node.tooltip.onDescription(nodeDescriptions[nodeId]);
}
Expand Down Expand Up @@ -889,7 +892,7 @@ class Graph {
}

const type = /** @type {string} */ (event.data[0]);
const data = /** @type {Object|number} */ (event.data[1]);
const data = /** @type {Object|number|bigint} */ (event.data[1]);
switch (type) {
case 'frameCreated':
this.frameCreated(
Expand Down Expand Up @@ -928,7 +931,7 @@ class Graph {
/** @type {!discards.mojom.WorkerInfo} */ (data));
break;
case 'nodeDeleted':
this.nodeDeleted(/** @type {number} */ (data));
this.nodeDeleted(/** @type {bigint} */ (data));
break;
case 'nodeDescriptions':
this.nodeDescriptions_(/** @type {!Object<string>} */ (data));
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/resources/discards/graph_tab_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class DiscardsGraphChangeStreamImpl {

/**
* @param {string} type
* @param {Object|number} data
* @param {Object|number|bigint} data
*/
postMessage_(type, data) {
this.contentWindow_.postMessage([type, data], '*');
Expand Down Expand Up @@ -118,12 +118,12 @@ Polymer({
*/
onMessage_(event) {
const type = /** @type {string} */ (event.data[0]);
const data = /** @type {Object|number} */ (event.data[1]);
const data = /** @type {Object|number|bigint} */ (event.data[1]);
switch (type) {
case 'requestNodeDescriptions':
// Forward the request through the mojoms and bounce the reply back.
this.graphDump_
.requestNodeDescriptions(/** @type {!Array<number>} */ (data))
.requestNodeDescriptions(/** @type {!Array<bigint>} */ (data))
.then(
(descriptions) => this.contentWindow_.postMessage(
['nodeDescriptions', descriptions.nodeDescriptionsJson],
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/resources/feed_internals/feed_internals.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ function setLinkNode(node, url) {
* @return {string}
*/
function toDateString(timeSinceEpoch) {
return timeSinceEpoch.microseconds === 0 ?
'' :
new Date(timeSinceEpoch.microseconds / 1000).toLocaleString();
const microseconds = Number(timeSinceEpoch.microseconds);
return microseconds === 0 ? '' :
new Date(microseconds / 1000).toLocaleString();
}

/**
Expand Down
13 changes: 7 additions & 6 deletions chrome/browser/resources/interventions_internals/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,15 +465,15 @@ InterventionsInternalPageImpl.prototype = {
*/
logNewMessage(log) {
insertMessageRowToMessageLogTable(
log.time, log.type, log.description, log.url.url, log.pageId);
Number(log.time), log.type, log.description, log.url.url, log.pageId);
},

/**
* Update new blocklisted host to the web page.
*
* @override
* @param {!string} host The blocklisted host.
* @param {number} time The time when the host was blocklisted in milliseconds
* @param {bigint} time The time when the host was blocklisted in milliseconds
* since Unix epoch.
*/
onBlocklistedHost(host, time) {
Expand All @@ -487,7 +487,7 @@ InterventionsInternalPageImpl.prototype = {

const timeTd = document.createElement('td');
timeTd.setAttribute('class', 'host-blocklisted-time');
timeTd.textContent = getTimeFormat(time);
timeTd.textContent = getTimeFormat(Number(time));
row.appendChild(timeTd);

// TODO(thanhdle): Insert row at correct index. crbug.com/776105.
Expand All @@ -511,12 +511,12 @@ InterventionsInternalPageImpl.prototype = {
* Update the blocklist cleared status on the page.
*
* @override
* @param {number} time The time of the event in milliseconds since Unix
* @param {bigint} time The time of the event in milliseconds since Unix
* epoch.
*/
onBlocklistCleared(time) {
const blocklistClearedStatus = $('blocklist-last-cleared-time');
blocklistClearedStatus.textContent = getTimeFormat(time);
blocklistClearedStatus.textContent = getTimeFormat(Number(time));

// Remove hosts from table.
const blocklistedHostsTable = $('blocklisted-hosts-table');
Expand All @@ -529,7 +529,8 @@ InterventionsInternalPageImpl.prototype = {

// Log event message.
insertMessageRowToMessageLogTable(
time, 'Blocklist', 'Blocklist Cleared', '' /* URL */, 0 /* pageId */);
Number(time), 'Blocklist', 'Blocklist Cleared', '' /* URL */,
0 /* pageId */);
},

/**
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/resources/media/media_feeds.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ class MediaFeedsTableDelegate {
* @returns {number}
*/
function timeDeltaToSeconds(timeDelta) {
return timeDelta.microseconds / 1000 / 1000;
return Number(timeDelta.microseconds) / 1000 / 1000;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/resources/new_tab_page/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function mojoString16(str) {
* @returns {!mojoBase.mojom.TimeDelta}
*/
export function mojoTimeDelta(timeDelta) {
return {microseconds: Math.floor(timeDelta * 1000)};
return {microseconds: BigInt(Math.floor(timeDelta * 1000))};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class FakePageHandler extends TestBrowserProxy {

/** @override */
install(diskSize, username) {
this.methodCalled('install', [diskSize, username]);
this.methodCalled('install', [Number(diskSize), username]);
}

/** @override */
Expand Down Expand Up @@ -236,7 +236,7 @@ suite('<crostini-installer-app>', () => {
await clickInstall();
await fakeBrowserProxy.handler.whenCalled('install').then(
([diskSize, username]) => {
assertEquals(diskSize, diskTicks[defaultIndex].value);
assertEquals(Number(diskSize), diskTicks[defaultIndex].value);
});
expectEquals(fakeBrowserProxy.handler.getCallCount('install'), 1);
});
Expand All @@ -261,7 +261,7 @@ suite('<crostini-installer-app>', () => {
await clickInstall();
await fakeBrowserProxy.handler.whenCalled('install').then(
([diskSize, username]) => {
assertEquals(diskSize, diskTicks[1].value);
assertEquals(Number(diskSize), diskTicks[1].value);
});
expectEquals(fakeBrowserProxy.handler.getCallCount('install'), 1);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ suite('ConfirmatonPageTest', function() {
test('renders share target name', function() {
const name = 'Device Name';
confirmationPageElement.shareTarget = {
id: {high: 0, low: 0},
id: {high: BigInt(0), low: BigInt(0)},
name,
type: nearbyShare.mojom.ShareTargetType.kPhone,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ suite('DiscoveryPageTest', function() {
*/
function createShareTarget(name) {
return {
id: {high: 0, low: nextId++},
id: {high: BigInt(0), low: BigInt(nextId++)},
name,
type: nearbyShare.mojom.ShareTargetType.kPhone,
};
Expand Down
12 changes: 6 additions & 6 deletions chrome/test/data/webui/new_tab_page/most_visited_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ suite('NewTabPageMostVisitedTest', () => {
source: i,
titleSource: i,
isQueryTile: false,
dataGenerationTime: {internalValue: 0},
dataGenerationTime: {internalValue: BigInt(0)},
};
});
const tilesRendered = eventToPromise('dom-change', mostVisited.$.tiles);
Expand Down Expand Up @@ -734,7 +734,7 @@ suite('NewTabPageMostVisitedTest', () => {
source: 0,
titleSource: 0,
isQueryTile: false,
dataGenerationTime: {internalValue: 0},
dataGenerationTime: {internalValue: BigInt(0)},
}],
visible: true,
});
Expand All @@ -756,7 +756,7 @@ suite('NewTabPageMostVisitedTest', () => {
source: 0,
titleSource: 0,
isQueryTile: false,
dataGenerationTime: {internalValue: 0},
dataGenerationTime: {internalValue: BigInt(0)},
}],
visible: true,
});
Expand Down Expand Up @@ -828,7 +828,7 @@ suite('NewTabPageMostVisitedTest', () => {
source: 0,
titleSource: 0,
isQueryTile: false,
dataGenerationTime: {internalValue: 0},
dataGenerationTime: {internalValue: BigInt(0)},
});
assertDeepEquals(tiles[1], {
title: 'b',
Expand All @@ -837,7 +837,7 @@ suite('NewTabPageMostVisitedTest', () => {
source: 1,
titleSource: 1,
isQueryTile: false,
dataGenerationTime: {internalValue: 0},
dataGenerationTime: {internalValue: BigInt(0)},
});
});

Expand All @@ -862,7 +862,7 @@ suite('NewTabPageMostVisitedTest', () => {
source: 0,
titleSource: 0,
isQueryTile: false,
dataGenerationTime: {internalValue: 0},
dataGenerationTime: {internalValue: BigInt(0)},
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,12 @@ export class ChromeHelper {
});

const idleManager = blink.mojom.IdleManager.getRemote();
// Set a large threshold since we don't care about user idle.
const threshold = {microseconds: 86400000000};
// Set a large threshold since we don't care about user idle. Note that
// ESLint does not yet seem to know about BigInt, so it complains about an
// uppercase "function" being used as something other than a constructor,
// and about BigInt not existing.
// eslint-disable-next-line new-cap, no-undef
const threshold = {microseconds: BigInt(86400000000)};
const {state} = await idleManager.addMonitor(
threshold, monitorCallbackRouter.$.bindNewPipeAndPassRemote());
callback(state.screen === blink.mojom.ScreenIdleState.kLocked);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ class DiagnosticsProxy {
(message);
this.assertNumberIsPositive(request.lengthSeconds);
return await getOrCreateDiagnosticsService().runPrimeSearchRoutine(
request.lengthSeconds, request.maximumNumber);
request.lengthSeconds, BigInt(request.maximumNumber));
};

/**
Expand Down Expand Up @@ -813,9 +813,10 @@ class TelemetryProxy {
// At this point, closure compiler knows that the input is {!Object}.

// Rule #2: convert objects like { value: X } to X, where X is either a
// number or a boolean.
// number, a bigint, or a boolean.
if (Object.entries(input).length === 1 &&
(typeof input['value'] === 'number' ||
typeof input['value'] === typeof BigInt(0) ||
typeof input['value'] === 'boolean')) {
return input['value'];
}
Expand Down
6 changes: 4 additions & 2 deletions chromeos/components/telemetry_extension_ui/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ js_library("untrusted_browsertest") {
"untrusted_test_handlers.js",
]
deps = [ "//chromeos/components/telemetry_extension_ui/resources:untrusted" ]
externs_list =
[ "//chromeos/components/web_applications/js2gtest_support.externs.js" ]
externs_list = [
"$externs_path/pending.js",
"//chromeos/components/web_applications/js2gtest_support.externs.js",
]
}
Loading

0 comments on commit 0092862

Please sign in to comment.