Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Web Inspector: Console: timestamps are always wrong
https://bugs.webkit.org/show_bug.cgi?id=255033

Reviewed by Patrick Angle.

Use `WallTime` instead of `Monotonic` time since this is a timestamp we expect to display rather than something for comparison (i.e. "was this before that?").

* Source/JavaScriptCore/inspector/ConsoleMessage.h:
* Source/JavaScriptCore/inspector/ConsoleMessage.cpp:
(Inspector::ConsoleMessage::ConsoleMessage):
(Inspector::ConsoleMessage::addToFrontend):
(Inspector::ConsoleMessage::updateRepeatCountInConsole):
* Source/WebCore/page/PageConsoleClient.cpp:
(WebCore::PageConsoleClient::screenshot):

* LayoutTests/inspector/console/timestamp.html: Added.
* LayoutTests/inspector/console/timestamp-expected.txt: Added.

Canonical link: https://commits.webkit.org/263261@main
  • Loading branch information
dcrousso committed Apr 21, 2023
1 parent a9b2452 commit 1eb0aa3
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 24 deletions.
9 changes: 9 additions & 0 deletions LayoutTests/inspector/console/timestamp-expected.txt
@@ -0,0 +1,9 @@
CONSOLE MESSAGE: 42
Test for the ConsoleMessage.timestamp event.


== Running test suite: ConsoleMessage.Timestamp
-- Running test case: ConsoleMessage.Timestamp.Basic
PASS: Should be after pre-captured timestamp.
PASS: Should be before post-captured timestamp.

34 changes: 34 additions & 0 deletions LayoutTests/inspector/console/timestamp.html
@@ -0,0 +1,34 @@
<!DOCTYPE html>
<html>
<head>
<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
<script>
function test()
{
let suite = InspectorTest.createAsyncSuite("ConsoleMessage.Timestamp");

suite.addTestCase({
name: "ConsoleMessage.Timestamp.Basic",
async test() {
let before = Date.now();
let [event] = await Promise.all([
WI.consoleManager.awaitEvent(WI.ConsoleManager.Event.MessageAdded),
InspectorTest.evaluateInPage(`console.log(42)`),
]);
let after = Date.now();

let {message} = event.data;

InspectorTest.expectGreaterThanOrEqual(message.timestamp * 1000, before, "Should be after pre-captured timestamp.");
InspectorTest.expectLessThanOrEqual(message.timestamp * 1000, after, "Should be before post-captured timestamp.");
},
});

suite.runTestCasesAndFinish();
}
</script>
</head>
<body onload="runTest()">
<p>Test for the ConsoleMessage.timestamp event.</p>
</body>
</html>
30 changes: 15 additions & 15 deletions Source/JavaScriptCore/inspector/ConsoleMessage.cpp
Expand Up @@ -42,18 +42,18 @@

namespace Inspector {

ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, unsigned long requestIdentifier, Seconds timestamp)
ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, unsigned long requestIdentifier, WallTime timestamp)
: m_source(source)
, m_type(type)
, m_level(level)
, m_message(message)
, m_url()
, m_requestId(IdentifiersFactory::requestId(requestIdentifier))
{
m_timestamp = timestamp ? timestamp : Seconds(MonotonicTime::now().secondsSinceEpoch());
m_timestamp = timestamp ? timestamp : WallTime::now();
}

ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, const String& url, unsigned line, unsigned column, JSC::JSGlobalObject* globalObject, unsigned long requestIdentifier, Seconds timestamp)
ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, const String& url, unsigned line, unsigned column, JSC::JSGlobalObject* globalObject, unsigned long requestIdentifier, WallTime timestamp)
: m_source(source)
, m_type(type)
, m_level(level)
Expand All @@ -63,11 +63,11 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
, m_column(column)
, m_requestId(IdentifiersFactory::requestId(requestIdentifier))
{
m_timestamp = timestamp ? timestamp : Seconds(MonotonicTime::now().secondsSinceEpoch());
m_timestamp = timestamp ? timestamp : WallTime::now();
autogenerateMetadata(globalObject);
}

ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, Ref<ScriptCallStack>&& callStack, unsigned long requestIdentifier, Seconds timestamp)
ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, Ref<ScriptCallStack>&& callStack, unsigned long requestIdentifier, WallTime timestamp)
: m_source(source)
, m_type(type)
, m_level(level)
Expand All @@ -76,7 +76,7 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
, m_url()
, m_requestId(IdentifiersFactory::requestId(requestIdentifier))
{
m_timestamp = timestamp ? timestamp : Seconds(MonotonicTime::now().secondsSinceEpoch());
m_timestamp = timestamp ? timestamp : WallTime::now();
const ScriptCallFrame* frame = m_callStack ? m_callStack->firstNonNativeCallFrame() : nullptr;
if (frame) {
m_url = frame->sourceURL();
Expand All @@ -85,7 +85,7 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
}
}

ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, Ref<ScriptArguments>&& arguments, Ref<ScriptCallStack>&& callStack, unsigned long requestIdentifier, Seconds timestamp)
ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, Ref<ScriptArguments>&& arguments, Ref<ScriptCallStack>&& callStack, unsigned long requestIdentifier, WallTime timestamp)
: m_source(source)
, m_type(type)
, m_level(level)
Expand All @@ -95,7 +95,7 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
, m_url()
, m_requestId(IdentifiersFactory::requestId(requestIdentifier))
{
m_timestamp = timestamp ? timestamp : Seconds(MonotonicTime::now().secondsSinceEpoch());
m_timestamp = timestamp ? timestamp : WallTime::now();
const ScriptCallFrame* frame = m_callStack ? m_callStack->firstNonNativeCallFrame() : nullptr;
if (frame) {
m_url = frame->sourceURL();
Expand All @@ -104,7 +104,7 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
}
}

ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, Ref<ScriptArguments>&& arguments, JSC::JSGlobalObject* globalObject, unsigned long requestIdentifier, Seconds timestamp)
ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, const String& message, Ref<ScriptArguments>&& arguments, JSC::JSGlobalObject* globalObject, unsigned long requestIdentifier, WallTime timestamp)
: m_source(source)
, m_type(type)
, m_level(level)
Expand All @@ -113,18 +113,18 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
, m_url()
, m_requestId(IdentifiersFactory::requestId(requestIdentifier))
{
m_timestamp = timestamp ? timestamp : Seconds(MonotonicTime::now().secondsSinceEpoch());
m_timestamp = timestamp ? timestamp : WallTime::now();
autogenerateMetadata(globalObject);
}

ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, Vector<JSONLogValue>&& messages, JSC::JSGlobalObject* globalObject, unsigned long requestIdentifier, Seconds timestamp)
ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLevel level, Vector<JSONLogValue>&& messages, JSC::JSGlobalObject* globalObject, unsigned long requestIdentifier, WallTime timestamp)
: m_source(source)
, m_type(type)
, m_level(level)
, m_url()
, m_requestId(IdentifiersFactory::requestId(requestIdentifier))
{
m_timestamp = timestamp ? timestamp : Seconds(MonotonicTime::now().secondsSinceEpoch());
m_timestamp = timestamp ? timestamp : WallTime::now();
if (globalObject)
m_globalObject = { globalObject->vm(), globalObject };

Expand Down Expand Up @@ -256,7 +256,7 @@ void ConsoleMessage::addToFrontend(ConsoleFrontendDispatcher& consoleFrontendDis
messageObject->setNetworkRequestId(m_requestId);

if (m_timestamp)
messageObject->setTimestamp(m_timestamp.seconds());
messageObject->setTimestamp(m_timestamp.secondsSinceEpoch().value());

if ((m_arguments && m_arguments->argumentCount()) || m_jsonLogValues.size()) {
InjectedScript injectedScript = injectedScriptManager.injectedScriptFor(globalObject());
Expand Down Expand Up @@ -333,8 +333,8 @@ String ConsoleMessage::toString() const

void ConsoleMessage::updateRepeatCountInConsole(ConsoleFrontendDispatcher& consoleFrontendDispatcher)
{
Seconds timestamp = MonotonicTime::now().secondsSinceEpoch();
consoleFrontendDispatcher.messageRepeatCountUpdated(m_repeatCount, timestamp.seconds());
auto timestamp = WallTime::now();
consoleFrontendDispatcher.messageRepeatCountUpdated(m_repeatCount, timestamp.secondsSinceEpoch().value());
}

bool ConsoleMessage::isEqual(ConsoleMessage* msg) const
Expand Down
16 changes: 8 additions & 8 deletions Source/JavaScriptCore/inspector/ConsoleMessage.h
Expand Up @@ -56,12 +56,12 @@ class JS_EXPORT_PRIVATE ConsoleMessage {
WTF_MAKE_NONCOPYABLE(ConsoleMessage);
WTF_MAKE_FAST_ALLOCATED;
public:
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, unsigned long requestIdentifier = 0, Seconds timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, const String& url, unsigned line, unsigned column, JSC::JSGlobalObject* = nullptr, unsigned long requestIdentifier = 0, Seconds timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, Ref<ScriptCallStack>&&, unsigned long requestIdentifier = 0, Seconds timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, Ref<ScriptArguments>&&, Ref<ScriptCallStack>&&, unsigned long requestIdentifier = 0, Seconds timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, Ref<ScriptArguments>&&, JSC::JSGlobalObject* = nullptr, unsigned long requestIdentifier = 0, Seconds timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, Vector<JSONLogValue>&&, JSC::JSGlobalObject*, unsigned long requestIdentifier = 0, Seconds timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, unsigned long requestIdentifier = 0, WallTime timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, const String& url, unsigned line, unsigned column, JSC::JSGlobalObject* = nullptr, unsigned long requestIdentifier = 0, WallTime timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, Ref<ScriptCallStack>&&, unsigned long requestIdentifier = 0, WallTime timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, Ref<ScriptArguments>&&, Ref<ScriptCallStack>&&, unsigned long requestIdentifier = 0, WallTime timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, const String& message, Ref<ScriptArguments>&&, JSC::JSGlobalObject* = nullptr, unsigned long requestIdentifier = 0, WallTime timestamp = { });
ConsoleMessage(MessageSource, MessageType, MessageLevel, Vector<JSONLogValue>&&, JSC::JSGlobalObject*, unsigned long requestIdentifier = 0, WallTime timestamp = { });
~ConsoleMessage();

void addToFrontend(ConsoleFrontendDispatcher&, InjectedScriptManager&, bool generatePreview);
Expand All @@ -74,7 +74,7 @@ class JS_EXPORT_PRIVATE ConsoleMessage {
const String& url() const { return m_url; }
unsigned line() const { return m_line; }
unsigned column() const { return m_column; }
Seconds timestamp() const { return m_timestamp; }
WallTime timestamp() const { return m_timestamp; }

JSC::JSGlobalObject* globalObject() const;

Expand Down Expand Up @@ -105,7 +105,7 @@ class JS_EXPORT_PRIVATE ConsoleMessage {
unsigned m_column { 0 };
unsigned m_repeatCount { 1 };
String m_requestId;
Seconds m_timestamp;
WallTime m_timestamp;
};

} // namespace Inspector
2 changes: 1 addition & 1 deletion Source/WebCore/page/PageConsoleClient.cpp
Expand Up @@ -318,7 +318,7 @@ void PageConsoleClient::screenshot(JSC::JSGlobalObject* lexicalGlobalObject, Ref
String dataURL;
JSC::JSValue target;

auto timestamp = m_page.inspectorController().executionStopwatch().elapsedTime();
auto timestamp = WallTime::now();

if (arguments->argumentCount()) {
auto possibleTarget = arguments->argumentAt(0);
Expand Down

0 comments on commit 1eb0aa3

Please sign in to comment.