Bug 194920 – Fix WTFLogVerbose variadic parameters forwarding

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Xabier Rodríguez Calvar]

Fix WTFLogVerbose variadic parameters forwarding

[Xabier Rodríguez Calvar]

Created attachment 362647 [details]
Patch

[EWS Watchlist]

Comment on attachment 362647 [details]
Patch

Attachment 362647 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11238881

New failing tests:
storage/indexeddb/intversion-open-in-upgradeneeded.html
storage/indexeddb/intversion-open-in-upgradeneeded-private.html
storage/indexeddb/modern/blocked-open-db-requests-private.html
storage/indexeddb/odd-strings-private.html

[EWS Watchlist]

Created attachment 362675 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Alex Christensen]

Comment on attachment 362647 [details]
Patch

r=me
I'm curious about what configuration led you to need to fix this bug.

[WebKit Commit Bot]

Comment on attachment 362647 [details]
Patch

Rejecting attachment 362647 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 362647, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=362647&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=194920&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 362647 from bug 194920.
Fetching: https://bugs.webkit.org/attachment.cgi?id=362647
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 2 diffs from patch file(s).
patching file Source/WTF/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WTF/wtf/Assertions.cpp
Hunk #1 FAILED at 380.
Hunk #2 succeeded at 454 (offset 50 lines).
Hunk #3 succeeded at 473 (offset 50 lines).
1 out of 3 hunks FAILED -- saving rejects to file Source/WTF/wtf/Assertions.cpp.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/11274586

[Xabier Rodríguez Calvar]

(In reply to Alex Christensen from comment #4)
> Comment on attachment 362647 [details]
> Patch
> 
> r=me
> I'm curious about what configuration led you to need to fix this bug.

Nothing too fancy, just:

@@ -458,7 +458,7 @@ WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithSecurityImplication(v
 #if LOG_DISABLED
 #define LOG(channel, ...) ((void)0)
 #else
-#define LOG(channel, ...) WTFLog(&LOG_CHANNEL(channel), __VA_ARGS__)
+#define LOG(channel, ...) WTFLogVerbose(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, &LOG_CHANNEL(channel), __VA_ARGS__)
 #endif
 
 /* LOG_VERBOSE */

For what I'm debugging, it is easier for me to know where the prints are done. I thought of adding a compile switch to force this. Do you think it could be interesting?

PS: Thx for the review, will land tomorrow morning CET.

[Alex Christensen]

I don't think a compile switch to turn on all the logs would be good.  There are ways to enable and disable each logging channel at runtime, which can accomplish the same thing.

[Xabier Rodríguez Calvar]

(In reply to Alex Christensen from comment #7)
> I don't think a compile switch to turn on all the logs would be good.  There
> are ways to enable and disable each logging channel at runtime, which can
> accomplish the same thing.

No, I'm talking about a compile switch to do what I do in the diff I added here

[Xabier Rodríguez Calvar]

Created attachment 362962 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 362962 [details]
Patch for landing

Clearing flags on attachment: 362962

Committed r242075: <https://trac.webkit.org/changeset/242075>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48391464>

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/242075/webkit in conjunction with the rollout in https://trac.webkit.org/changeset/242189/webkit has caused several storage/indexeddb/ tests to crash on Mac Debug WK1 and are slowing down EWS. Marking the tests as skip causes other tests to crash.  

The details of this were tracked in 
https://bugs.webkit.org/show_bug.cgi?id=195210 and https://bugs.webkit.org/show_bug.cgi?id=195141

Can you update your change to reflect the rollout? otherwise we will need to roll r242075 out as well to fix testing.

[Xabier Rodríguez Calvar]

(In reply to Truitt Savell from comment #13)
> Can you update your change to reflect the rollout? otherwise we will need to
> roll r242075 out as well to fix testing.

On it.

[Xabier Rodríguez Calvar]

Anyway, I don't follow why I need to fix this myself or my patch will be rolled out. The original implementation is wrong and it should be fixed as well. We can't just roll out a patch and leave things wrong, right?

[Xabier Rodríguez Calvar]

(In reply to Truitt Savell from comment #13)
> Can you update your change to reflect the rollout? otherwise we will need to
> roll r242075 out as well to fix testing.

As I commented on bug 195210, it looks like bots are green now thanks to a fix by Darin, who landed r242308.