From 126d4779256509a492671bba8bdc440916a9f89a Mon Sep 17 00:00:00 2001 From: Maksim Sadym Date: Mon, 11 Jul 2022 12:14:46 +0000 Subject: [PATCH] Fix Date BiDi format 1. Add `toISOString` to `v8::Date`. 2. Switch serialization to `ISOString`. Bug: v8:13043 Change-Id: I8a852f4a4a46bb3b8e5d52ef3cdffde7a408b403 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3749203 Auto-Submit: Maksim Sadym Reviewed-by: Toon Verwaest Commit-Queue: Toon Verwaest Cr-Commit-Position: refs/heads/main@{#81647} --- include/v8-date.h | 5 +++ src/api/api.cc | 14 +++++++ src/builtins/builtins-date.cc | 20 ++------- src/date/date.cc | 18 ++++++-- src/date/date.h | 1 + src/inspector/v8-webdriver-serializer.cc | 8 ++-- .../generate-web-driver-value-expected.txt | 41 +++++++++++++++---- .../runtime/generate-web-driver-value.js | 26 ++---------- 8 files changed, 78 insertions(+), 55 deletions(-) diff --git a/include/v8-date.h b/include/v8-date.h index e7a01f29b2..8d82ccc9ea 100644 --- a/include/v8-date.h +++ b/include/v8-date.h @@ -27,6 +27,11 @@ class V8_EXPORT Date : public Object { */ double ValueOf() const; + /** + * Generates ISO string representation. + */ + v8::Local ToISOString() const; + V8_INLINE static Date* Cast(Value* value) { #ifdef V8_ENABLE_CHECKS CheckCast(value); diff --git a/src/api/api.cc b/src/api/api.cc index bc70d48781..40dd9a0d9e 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -7280,6 +7280,20 @@ double v8::Date::ValueOf() const { return jsdate->value().Number(); } +v8::Local v8::Date::ToISOString() const { + i::Handle obj = Utils::OpenHandle(this); + i::Handle jsdate = i::Handle::cast(obj); + i::Isolate* i_isolate = jsdate->GetIsolate(); + API_RCS_SCOPE(i_isolate, Date, NumberValue); + i::DateBuffer buffer = + i::ToDateString(jsdate->value().Number(), i_isolate->date_cache(), + i::ToDateStringMode::kISODateAndTime); + i::Handle str = i_isolate->factory() + ->NewStringFromUtf8(base::VectorOf(buffer)) + .ToHandleChecked(); + return Utils::ToLocal(str); +} + // Assert that the static TimeZoneDetection cast in // DateTimeConfigurationChangeNotification is valid. #define TIME_ZONE_DETECTION_ASSERT_EQ(value) \ diff --git a/src/builtins/builtins-date.cc b/src/builtins/builtins-date.cc index 3961442ca6..a07ce60d9f 100644 --- a/src/builtins/builtins-date.cc +++ b/src/builtins/builtins-date.cc @@ -684,22 +684,10 @@ BUILTIN(DatePrototypeToISOString) { THROW_NEW_ERROR_RETURN_FAILURE( isolate, NewRangeError(MessageTemplate::kInvalidTimeValue)); } - int64_t const time_ms = static_cast(time_val); - int year, month, day, weekday, hour, min, sec, ms; - isolate->date_cache()->BreakDownTime(time_ms, &year, &month, &day, &weekday, - &hour, &min, &sec, &ms); - char buffer[128]; - if (year >= 0 && year <= 9999) { - SNPrintF(base::ArrayVector(buffer), "%04d-%02d-%02dT%02d:%02d:%02d.%03dZ", - year, month + 1, day, hour, min, sec, ms); - } else if (year < 0) { - SNPrintF(base::ArrayVector(buffer), "-%06d-%02d-%02dT%02d:%02d:%02d.%03dZ", - -year, month + 1, day, hour, min, sec, ms); - } else { - SNPrintF(base::ArrayVector(buffer), "+%06d-%02d-%02dT%02d:%02d:%02d.%03dZ", - year, month + 1, day, hour, min, sec, ms); - } - return *isolate->factory()->NewStringFromAsciiChecked(buffer); + DateBuffer buffer = ToDateString(time_val, isolate->date_cache(), + ToDateStringMode::kISODateAndTime); + RETURN_RESULT_OR_FAILURE( + isolate, isolate->factory()->NewStringFromUtf8(base::VectorOf(buffer))); } // ES6 section 20.3.4.41 Date.prototype.toString ( ) diff --git a/src/date/date.cc b/src/date/date.cc index 5749c9833e..c4742aab30 100644 --- a/src/date/date.cc +++ b/src/date/date.cc @@ -559,9 +559,10 @@ DateBuffer ToDateString(double time_val, DateCache* date_cache, return FormatDate("Invalid Date"); } int64_t time_ms = static_cast(time_val); - int64_t local_time_ms = mode != ToDateStringMode::kUTCDateAndTime - ? date_cache->ToLocal(time_ms) - : time_ms; + int64_t local_time_ms = (mode == ToDateStringMode::kUTCDateAndTime || + mode == ToDateStringMode::kISODateAndTime) + ? time_ms + : date_cache->ToLocal(time_ms); int year, month, day, weekday, hour, min, sec, ms; date_cache->BreakDownTime(local_time_ms, &year, &month, &day, &weekday, &hour, &min, &sec, &ms); @@ -590,6 +591,17 @@ DateBuffer ToDateString(double time_val, DateCache* date_cache, : "%s, %02d %s %04d %02d:%02d:%02d GMT", kShortWeekDays[weekday], day, kShortMonths[month], year, hour, min, sec); + case ToDateStringMode::kISODateAndTime: + if (year >= 0 && year <= 9999) { + return FormatDate("%04d-%02d-%02dT%02d:%02d:%02d.%03dZ", year, + month + 1, day, hour, min, sec, ms); + } else if (year < 0) { + return FormatDate("-%06d-%02d-%02dT%02d:%02d:%02d.%03dZ", -year, + month + 1, day, hour, min, sec, ms); + } else { + return FormatDate("+%06d-%02d-%02dT%02d:%02d:%02d.%03dZ", year, + month + 1, day, hour, min, sec, ms); + } } UNREACHABLE(); } diff --git a/src/date/date.h b/src/date/date.h index 703cd45e15..467ba8802d 100644 --- a/src/date/date.h +++ b/src/date/date.h @@ -255,6 +255,7 @@ enum class ToDateStringMode { kLocalTime, kLocalDateAndTime, kUTCDateAndTime, + kISODateAndTime }; // ES6 section 20.3.4.41.1 ToDateString(tv) diff --git a/src/inspector/v8-webdriver-serializer.cc b/src/inspector/v8-webdriver-serializer.cc index 292c405d81..f29c56f2aa 100644 --- a/src/inspector/v8-webdriver-serializer.cc +++ b/src/inspector/v8-webdriver-serializer.cc @@ -40,12 +40,10 @@ std::unique_ptr DescriptionForDate( v8::Local context, v8::Local date) { v8::Isolate* isolate = context->GetIsolate(); v8::TryCatch tryCatch(isolate); - v8::Local description; - bool success = date->ToString(context).ToLocal(&description); - DCHECK(success); - USE(success); - return protocol::StringValue::create(toProtocolString(isolate, description)); + v8::Local dateISOString = date->ToISOString(); + return protocol::StringValue::create( + toProtocolString(isolate, dateISOString)); } String16 _descriptionForRegExpFlags(v8::Local value) { diff --git a/test/inspector/runtime/generate-web-driver-value-expected.txt b/test/inspector/runtime/generate-web-driver-value-expected.txt index c83a872fce..7706ebf198 100644 --- a/test/inspector/runtime/generate-web-driver-value-expected.txt +++ b/test/inspector/runtime/generate-web-driver-value-expected.txt @@ -306,14 +306,39 @@ Runtime.callFunctionOn } Running test: Date -testing date: Thu Apr 07 2022 16:16:25 GMT+1100 -Expected date in GMT: Thu, 07 Apr 2022 05:16:25 GMT -Date type as expected: true -Date value as expected: true -testing date: Thu Apr 07 2022 16:16:25 GMT-1100 -Expected date in GMT: Fri, 08 Apr 2022 03:16:25 GMT -Date type as expected: true -Date value as expected: true +testing expression: new Date('Thu Apr 07 2022 16:17:18 GMT') +Runtime.evaluate +{ + type : date + value : 2022-04-07T16:17:18.000Z +} +Runtime.callFunctionOn +{ + type : date + value : 2022-04-07T16:17:18.000Z +} +testing expression: new Date('Thu Apr 07 2022 16:17:18 GMT+1100') +Runtime.evaluate +{ + type : date + value : 2022-04-07T05:17:18.000Z +} +Runtime.callFunctionOn +{ + type : date + value : 2022-04-07T05:17:18.000Z +} +testing expression: new Date('Thu Apr 07 2022 16:17:18 GMT-1100') +Runtime.evaluate +{ + type : date + value : 2022-04-08T03:17:18.000Z +} +Runtime.callFunctionOn +{ + type : date + value : 2022-04-08T03:17:18.000Z +} Running test: Error testing expression: [new Error(), new Error('qwe')] diff --git a/test/inspector/runtime/generate-web-driver-value.js b/test/inspector/runtime/generate-web-driver-value.js index 1a9277f2b4..c8831c867d 100644 --- a/test/inspector/runtime/generate-web-driver-value.js +++ b/test/inspector/runtime/generate-web-driver-value.js @@ -35,9 +35,9 @@ InspectorTest.runAsyncTestSuite([ await testExpression("[new RegExp('ab+c'), new RegExp('ab+c', 'ig')]"); }, async function Date() { - // Serialization depends on the timezone, so0 manual vreification is needed. - await testDate("Thu Apr 07 2022 16:16:25 GMT+1100"); - await testDate("Thu Apr 07 2022 16:16:25 GMT-1100"); + await testExpression("new Date('Thu Apr 07 2022 16:17:18 GMT')"); + await testExpression("new Date('Thu Apr 07 2022 16:17:18 GMT+1100')"); + await testExpression("new Date('Thu Apr 07 2022 16:17:18 GMT-1100')"); }, async function Error() { await testExpression("[new Error(), new Error('qwe')]"); @@ -73,26 +73,6 @@ InspectorTest.runAsyncTestSuite([ await testExpression("{key_level_1: {key_level_2: {key_level_3: 'value_level_3'}}}"); }]); -async function testDate(dateStr) { - // TODO(sadym): make the test timezone-agnostic. Current approach is not 100% valid, as it relies on the `date.ToString` implementation. - InspectorTest.logMessage("testing date: " + dateStr); - const serializedDate = (await serializeViaEvaluate("new Date('" + dateStr + "')")).result.result.webDriverValue; - // Expected format: { - // type: "date" - // value: "Fri Apr 08 2022 03:16:25 GMT+0000 (Coordinated Universal Time)" - // } - const expectedDateStr = new Date(dateStr).toString(); - - InspectorTest.logMessage("Expected date in GMT: " + (new Date(dateStr).toGMTString())); - InspectorTest.logMessage("Date type as expected: " + (serializedDate.type === "date")); - if (serializedDate.value === expectedDateStr) { - InspectorTest.logMessage("Date value as expected: " + (serializedDate.value === expectedDateStr)); - } else { - InspectorTest.logMessage("Error. Eexpected " + expectedDateStr + ", but was " + serializedDate.value); - - } -} - async function serializeViaEvaluate(expression) { return await Protocol.Runtime.evaluate({ expression: "("+expression+")",