Bug 204739 - [GTK] Fix some warnings
Summary: [GTK] Fix some warnings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-02 01:59 PST by Rob Buis
Modified: 2019-12-02 07:43 PST (History)
10 users (show)

See Also:


Attachments
Patch (4.29 KB, patch)
2019-12-02 02:01 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (2.86 KB, patch)
2019-12-02 04:40 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (2.76 KB, patch)
2019-12-02 06:44 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (2.77 KB, patch)
2019-12-02 07:37 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (2.77 KB, patch)
2019-12-02 07:40 PST, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2019-12-02 01:59:19 PST
I see this in a recent build:

In file included from DerivedSources/WebCore/unified-sources/UnifiedSource-4babe430-6.cpp:4:
../../Source/WebCore/Modules/indexeddb/IDBDatabase.cpp: In member function ‘void WebCore::IDBDatabase::connectionToServerLost(const WebCore::IDBError&)’:
../../Source/WebCore/Modules/indexeddb/IDBDatabase.cpp:281:15: warning: unused variable ‘context’ [-Wunused-variable]
     if (auto* context = scriptExecutionContext())
               ^~~~~~~
../../Source/WebCore/Modules/indexeddb/IDBDatabase.cpp:287:15: warning: unused variable ‘context’ [-Wunused-variable]
     if (auto* context = scriptExecutionContext())
               ^~~~~~~
[2894/3859] Building CXX object Source/WebCore/CMakeFiles/WebCore.dir/__/__/DerivedSources/WebCore/unified-sources/UnifiedSource-207b877e-4.cpp.o
In file included from DerivedSources/WebCore/unified-sources/UnifiedSource-207b877e-4.cpp:4:
../../Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp:267:17: warning: multi-line comment [-Wcomment]
                 //              \
Comment 1 Rob Buis 2019-12-02 02:01:42 PST
Created attachment 384601 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2019-12-02 02:54:40 PST
Comment on attachment 384601 [details]
Patch

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

> Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp:-267
> -                //              \

I can't see anything on https://webkit.org/code-style-guidelines/ saying we shouldn't use "C style" but maybe an alternative is to change this '\'. For example use '/' instead and move inline-block to the right.
Comment 3 Rob Buis 2019-12-02 02:58:16 PST
(In reply to Frédéric Wang (:fredw) from comment #2)
> Comment on attachment 384601 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=384601&action=review
> 
> > Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp:-267
> > -                //              \
> 
> I can't see anything on https://webkit.org/code-style-guidelines/ saying we
> shouldn't use "C style" but maybe an alternative is to change this '\'. For
> example use '/' instead and move inline-block to the right.

I have a hard time understanding that part of the ASCII art. I think I'll try your suggestion and CC zalan/antii whether they like it.
Comment 4 Rob Buis 2019-12-02 04:40:37 PST
Created attachment 384609 [details]
Patch
Comment 5 Rob Buis 2019-12-02 06:36:03 PST
@zalan, antti, is the new ASCII art acceptable? :) It fixes the warning.
Comment 6 zalan 2019-12-02 06:41:30 PST
(In reply to Rob Buis from comment #5)
> @zalan, antti, is the new ASCII art acceptable? :) It fixes the warning.

Let's just remove those 2 lines:
+++ b/Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp
@@ -264,8 +264,6 @@ void LineBuilder::alignContentVertically(RunList& runList)
                 ASSERT(!formattingState.lineBoxes().isEmpty());
                 auto inlineBlockBaselineOffset = formattingState.lineBoxes().last().baselineOffset();
                 // The inline-block's baseline offset is relative to its content box. Let's convert it relative to the margin box.
-                //   inline-block
-                //              \
                 //           _______________ <- margin box
                 //          |
                 //          |  ____________  <- border box
Comment 7 Rob Buis 2019-12-02 06:44:05 PST
Created attachment 384615 [details]
Patch
Comment 8 WebKit Commit Bot 2019-12-02 06:46:17 PST
Comment on attachment 384615 [details]
Patch

Rejecting attachment 384615 [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', 'validate-changelog', '--check-oops', '--non-interactive', 384615, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/13284915
Comment 9 WebKit Commit Bot 2019-12-02 07:36:16 PST
Comment on attachment 384615 [details]
Patch

Clearing flags on attachment: 384615

Committed r252982: <https://trac.webkit.org/changeset/252982>
Comment 10 WebKit Commit Bot 2019-12-02 07:36:17 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Rob Buis 2019-12-02 07:37:42 PST
Reopening to attach new patch.
Comment 12 Rob Buis 2019-12-02 07:37:44 PST
Created attachment 384617 [details]
Patch
Comment 13 Radar WebKit Bug Importer 2019-12-02 07:37:58 PST
<rdar://problem/57557807>
Comment 14 Radar WebKit Bug Importer 2019-12-02 07:37:58 PST
<rdar://problem/57557808>
Comment 15 WebKit Commit Bot 2019-12-02 07:39:47 PST
Comment on attachment 384617 [details]
Patch

Rejecting attachment 384617 [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-03', 'apply-attachment', '--no-update', '--non-interactive', 384617, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

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

Parsed 3 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/Modules/indexeddb/IDBDatabase.cpp
Hunk #1 FAILED at 278.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/Modules/indexeddb/IDBDatabase.cpp.rej
patching file Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp
Hunk #1 FAILED at 263.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp.rej

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

Full output: https://webkit-queues.webkit.org/results/13284944
Comment 16 Rob Buis 2019-12-02 07:40:40 PST
Created attachment 384618 [details]
Patch