Many warnings are came from the result when RELEASE_LOG is disabled.
Created attachment 426490 [details] PATCH
Created attachment 426494 [details] PATCH
Created attachment 426495 [details] PATCH
Comment on attachment 426495 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=426495&action=review > Source/WebCore/loader/SubresourceLoader.cpp:311 > + UNUSED_VARIABLE(this); This isn’t the best fix. The correct fix is to not capture this, since we’re not using it. Not sure we should be capturing protectedThis either. > Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:540 > unsigned numberOfBatches = std::ceil(domains.size() / static_cast<float>(maxNumberOfDomainsInOneLogStatement)); > + UNUSED_VARIABLE(numberOfBatches); Better fix is to wrap this line in: #if !RELEASE_LOG_DISABLED #endif > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:359 > +#endif I think this #endif should be up above, not wrapped around the entire block. We don’t want or need to nest the #if > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:160 > RELEASE_LOG(ServiceWorker, "Created service worker %" PRIu64 " in process PID %i", serviceWorkerIdentifier.toUInt64(), getCurrentProcessID()); > + UNUSED_VARIABLE(serviceWorkerIdentifier); Better fix is to use data.serviceWorkerIdentifier in the RELEASE_LOG line and get rid of the local variable. > Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:55 > +static UNUSED_FUNCTION uint64_t pageIDFromWebFrame(const RefPtr<WebFrame>& frame) Better fix is to put these functions inside #if !RELEASE_LOG_DISABLED.
Comment on attachment 426495 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=426495&action=review >> Source/WebCore/loader/SubresourceLoader.cpp:311 >> + UNUSED_VARIABLE(this); > > This isn’t the best fix. The correct fix is to not capture this, since we’re not using it. Not sure we should be capturing protectedThis either. But `this` is uses in RELEASE_LOG_IF_ALLOWED macro. It will be warned if RELEASE_LOG_DISABLED. >> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:540 >> + UNUSED_VARIABLE(numberOfBatches); > > Better fix is to wrap this line in: > > #if !RELEASE_LOG_DISABLED > #endif Right. >> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:359 >> +#endif > > I think this #endif should be up above, not wrapped around the entire block. We don’t want or need to nest the #if Ah, okay. Make sense. >> Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:160 >> + UNUSED_VARIABLE(serviceWorkerIdentifier); > > Better fix is to use data.serviceWorkerIdentifier in the RELEASE_LOG line and get rid of the local variable. Right. >> Source/WebKit/WebProcess/WebPage/WebURLSchemeTaskProxy.cpp:55 >> +static UNUSED_FUNCTION uint64_t pageIDFromWebFrame(const RefPtr<WebFrame>& frame) > > Better fix is to put these functions inside #if !RELEASE_LOG_DISABLED. Right.
Comment on attachment 426495 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=426495&action=review >>> Source/WebCore/loader/SubresourceLoader.cpp:311 >>> + UNUSED_VARIABLE(this); >> >> This isn’t the best fix. The correct fix is to not capture this, since we’re not using it. Not sure we should be capturing protectedThis either. > > But `this` is uses in RELEASE_LOG_IF_ALLOWED macro. It will be warned if RELEASE_LOG_DISABLED. I will add RELEASE_LOG_IF_ALLOWED for RELEASE_LOG_DISABLED case. >>> Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:160 >>> + UNUSED_VARIABLE(serviceWorkerIdentifier); >> >> Better fix is to use data.serviceWorkerIdentifier in the RELEASE_LOG line and get rid of the local variable. > > Right. Ah, no. it is passed with WTFMove(data). We shouldn't use that. Wrap with #!RELEASE_LOG_DISABLED.
Created attachment 426504 [details] PATCH
Comment on attachment 426504 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=426504&action=review > Source/WebKit/WebProcess/WebProcess.cpp:1469 > + UNUSED_VARIABLE(this); Don’t need this any more since you put a fix into RELEASE_LOG_IF_ALLOWED
Comment on attachment 426504 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=426504&action=review > Source/WebKit/NetworkProcess/NetworkProcess.cpp:2214 > + UNUSED_VARIABLE(this); Can we do something for this? >> Source/WebKit/WebProcess/WebProcess.cpp:1469 >> + UNUSED_VARIABLE(this); > > Don’t need this any more since you put a fix into RELEASE_LOG_IF_ALLOWED That was defined in SubresourceLoader.cpp, so I have to define that here for this file. Okay.
Created attachment 426518 [details] PATCH
Committed r276306 (236788@main): <https://commits.webkit.org/236788@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426518 [details].
rdar://76900340