Bug 237047 - Refactor DOMWindow calls generating console warnings into a single DOMWindow::printWarningMessage method
Summary: Refactor DOMWindow calls generating console warnings into a single DOMWindow:...
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Darin Adler
URL:
Keywords: InRadar
Depends on:
Blocks: 237046
  Show dependency treegraph
 
Reported: 2022-02-22 11:42 PST by Tim Nguyen (:ntim)
Modified: 2022-05-11 09:42 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.12 KB, patch)
2022-02-22 13:08 PST, Tim Nguyen (:ntim)
darin: review+
Details | Formatted Diff | Diff
[fast-cq] Patch (5.14 KB, patch)
2022-02-22 21:58 PST, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Nguyen (:ntim) 2022-02-22 11:42:01 PST
See https://bugs.webkit.org/show_bug.cgi?id=237046#c5
Comment 1 Tim Nguyen (:ntim) 2022-02-22 13:08:36 PST
Created attachment 452896 [details]
Patch
Comment 2 Darin Adler 2022-02-22 15:59:16 PST
Comment on attachment 452896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452896&action=review

> Source/WebCore/page/DOMWindow.h:273
> +    void printConsoleMessage(const String&, MessageLevel) const;
>      void printErrorMessage(const String&) const;
> +    void printWarningMessage(const String&) const;

Can these all be private? Is there any caller using them outside the class?
Comment 3 Tim Nguyen (:ntim) 2022-02-22 21:58:40 PST
Created attachment 452937 [details]
[fast-cq] Patch
Comment 4 EWS 2022-02-22 22:02:27 PST
Committed r290348 (247666@main): <https://commits.webkit.org/247666@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452937 [details].
Comment 5 Radar WebKit Bug Importer 2022-02-22 22:03:17 PST
<rdar://problem/89336944>
Comment 6 Robert Jenner 2022-02-23 16:39:40 PST
Reverted r290348 for reason:

Broke a test, slowing down EWS.

Committed r290399 (247711@trunk): <https://commits.webkit.org/247711@trunk>
Comment 7 Dawn Morningstar 2022-02-23 16:49:35 PST
Robert reverted r290348 for me as I do not have committer status yet. 
https://bugs.webkit.org/show_bug.cgi?id=237116
Change seems to have broken "lastConsoleMessage" As it no longer sees the warning.
Comment 8 Darin Adler 2022-02-23 17:56:45 PST
Turns out there is a debugging feature, Document::setConsoleMessageListener, used for regression tests, that only works with messages sent to the console client through the document. And there was a test depending on using this mechanism to see the console messages from startListeningForDeviceOrientationIfNecessary and startListeningForDeviceMotionIfNecessary.

Outside of the tests, there was nothing wrong with this, but it broke the tests.

Sadly this means that we have to replace Document::setConsoleMessageListener with a better testing mechanism or keep these messages flowing through the document.

My fault for suggesting that we didn’t need to send the messages through the document. Turns out not to be true for this exotic test-only reason.
Comment 9 Darin Adler 2022-02-24 09:12:25 PST
I’ll do this.