Bug 228809

Summary:

Deduplicate logging channel algorithms

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

New Bugs

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, aboxhall, andresg_22, annulen, apinheiro, benjamin, cdumez, cfleizach, cmarcelo, commit-queue, ddkilzer, dmazzoni, ews-watchlist, fujii.hironori, gyuyoung.kim, hi, jcraig, jdiggs, joepeck, ryuan.choi, samuel_white, sergio, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

228897

Bug Blocks:

228768, 228899

Attachments:

Description	Flags
Patch	fujii.hironori: review+, ews-feeder: commit-queue-
Patch for committing	ews-feeder: commit-queue-
Patch for committing	none

Myles C. Maxfield

Reported 2021-08-04 19:29:08 PDT

Deduplicate logging channel algorithms

Attachments
Patch (63.86 KB, patch) 2021-08-05 19:57 PDT, Myles C. Maxfield	fujii.hironori: review+ ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch for committing (64.38 KB, patch) 2021-08-06 20:05 PDT, Myles C. Maxfield	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch for committing (64.98 KB, patch) 2021-08-06 23:40 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Myles C. Maxfield

Comment 1 2021-08-05 19:57:12 PDT

Created attachment 435051 [details] Patch

Fujii Hironori

Comment 2 2021-08-06 14:40:34 PDT

Comment on attachment 435051 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435051&action=review > Source/WTF/wtf/LogChannels.cpp:37 > + WTFLogChannel* channel = WTFLogChannelByName(m_logChannels.data(), m_logChannels.size(), name.utf8().data()); You can use getLogChannel(name) here. > Source/WTF/wtf/LogChannels.cpp:45 > + WTFLogChannel* channel = WTFLogChannelByName(m_logChannels.data(), m_logChannels.size(), name.utf8().data()); Ditto.

Fujii Hironori

Comment 3 2021-08-06 14:44:47 PDT

Comment on attachment 435051 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435051&action=review > Source/WebCore/platform/LogInitialization.cpp:32 > +#include <wtf/text/CString.h> Do you really use <wtf/text/CString.h> in this file? > Source/WebCore/platform/LogInitialization.cpp:36 > +#if PLATFORM(COCOA) Do you really need Cocoa specific headers in this file?

Fujii Hironori

Comment 4 2021-08-06 14:51:17 PDT

They have some #undef for the problem. https://github.com/WebKit/WebKit/blob/4c96573f0f4dde291b8ce4a23dfd54d4b9fd7d5e/Source/WebCore/platform/graphics/GLContext.h#L51 https://github.com/WebKit/WebKit/blob/4c96573f0f4dde291b8ce4a23dfd54d4b9fd7d5e/Source/WebCore/platform/graphics/GraphicsContext.h#L73

Fujii Hironori

Comment 5 2021-08-06 14:52:45 PDT

Comment on attachment 435051 [details] Patch Nice refactoring. LGTM. But, cq- for the GTK build problem.

Myles C. Maxfield

Comment 6 2021-08-06 20:05:34 PDT

Created attachment 435110 [details] Patch for committing

Myles C. Maxfield

Comment 7 2021-08-06 23:40:08 PDT

Created attachment 435119 [details] Patch for committing

Myles C. Maxfield

Comment 8 2021-08-07 00:55:03 PDT

Committed r280756 (240341@main): <https://commits.webkit.org/240341@main>

Radar WebKit Bug Importer

Comment 9 2021-08-07 00:56:17 PDT

<rdar://problem/81647472>

Aakash Jain

Comment 10 2021-08-07 09:26:09 PDT

(In reply to Myles C. Maxfield from comment #8) > Committed r280756 (240341@main): <https://commits.webkit.org/240341@main> This seems to have broken GTK build. e.g.: https://build.webkit.org/#/builders/41/builds/4806 passed with 240340@main https://build.webkit.org/#/builders/41/builds/4807 failed with 240341@main

WebKit Commit Bot

Comment 11 2021-08-07 09:27:49 PDT

Re-opened since this is blocked by bug 228897

Myles C. Maxfield

Comment 12 2021-08-07 11:15:58 PDT

Oh, somehow the patch I committed didn't include the fix for GTK! The Patch for committing above includes a change to Threading.h but r280756 doesn't. Let's try this again, with the fully updated patch this time.

EWS

Comment 13 2021-08-07 11:50:23 PDT

Committed r280758 (240343@main): <https://commits.webkit.org/240343@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435119 [details].

Myles C. Maxfield

Comment 14 2021-08-07 11:55:35 PDT

Watching the GTK bot now...

David Kilzer (:ddkilzer)

Comment 15 2021-09-12 12:45:14 PDT

(In reply to EWS from comment #13) > Committed r280758 (240343@main): <https://commits.webkit.org/240343@main> > > All reviewed patches have been landed. Closing bug and clearing flags on > attachment 435119 [details]. Follow-up fix to the WebKit Xcode project: REGRESSION (r280758): WebKit project won't open in Xcode 11.4 <https://trac.webkit.org/changeset/282317/webkit>

Note You need to log in before you can comment on or make changes to this bug.