Bug 177766

Summary: [Settings] Convert more settings to be generated
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, Hironori.Fujii, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Sam Weinig 2017-10-02 12:55:31 PDT
[Settings] Convert more settings to be generated
Comment 1 Sam Weinig 2017-10-02 13:10:34 PDT
Created attachment 322414 [details]
Patch
Comment 2 Sam Weinig 2017-10-02 20:44:47 PDT
Created attachment 322489 [details]
Patch
Comment 3 Fujii Hironori 2017-10-03 01:32:42 PDT
GTK port EWS also failed even though it is green.
I tested with GTK port and reproduced the same error.

By removing the following outdated file, it succeeds to build:

WebKitBuild/Release/DerivedSources/ForwardingHeaders/WebCore/Settings.h
Comment 4 Sam Weinig 2017-10-03 07:00:34 PDT
(In reply to Fujii Hironori from comment #3)
> GTK port EWS also failed even though it is green.
> I tested with GTK port and reproduced the same error.
> 
> By removing the following outdated file, it succeeds to build:
> 
> WebKitBuild/Release/DerivedSources/ForwardingHeaders/WebCore/Settings.h

Any idea how to fix this? I'm not sure how to make the new generated Settings.h available as a forwarding header.
Comment 5 Fujii Hironori 2017-10-03 19:45:50 PDT
Generally speaking, it's impossible to remove old generating files.
For example, Foo.idl generated Foo.cpp, and Bar.cpp generated Bar.o.
Then, the source files Foo.idl and Bar.cpp are removed.
The old generated files Foo.cpp and Bar.o exist in the build directory.
It's impossible to remove these generated files because the new build script has no information about the files.

Old generated file is WebKitBuild/Release/DerivedSources/ForwardingHeaders/WebCore/Settings.h.
New generated file is WebKitBuild/Release/DerivedSources/WebCore/Settings.h

-IDerivedSources/ForwardingHeaders is given before -IDerivedSources.
Then, #include <WebCore/Settings.h> refers old one.

One solution is reordering the include directories.
But, I don't think this is a good idea.
Becuase opposite problems can happen, and this is only for the incremental build failure.

Another solution is using a different name, for example SettingsGenerated.h.
And, Preserve Source/WebCore/page/Settings.h with just two following lines:

> #pragma once
> #include "SettingsGenerated.h"
Comment 6 Sam Weinig 2017-10-18 14:16:06 PDT Comment hidden (obsolete)
Comment 7 Sam Weinig 2017-10-18 14:43:41 PDT Comment hidden (obsolete)
Comment 8 Sam Weinig 2017-10-18 18:11:06 PDT Comment hidden (obsolete)
Comment 9 Sam Weinig 2017-10-18 18:18:03 PDT
Created attachment 324191 [details]
Patch
Comment 10 Dean Jackson 2017-10-18 20:41:25 PDT
Comment on attachment 324191 [details]
Patch

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

> Source/WebCore/page/Settings.in:328
> +loadsImagesAutomatically initial=false, onChange=imagesEnabledChanged
> +imagesEnabled initial=true, getter=areImagesEnabled, onChange=imagesEnabledChanged

Should these have the same onChange?

Answer: yes, even though it looks like a typo.

> Source/WebCore/page/SettingsBase.cpp:478
>  
> +
> +

Lots of spaces
Comment 11 WebKit Commit Bot 2017-10-18 21:08:42 PDT
Comment on attachment 324191 [details]
Patch

Clearing flags on attachment: 324191

Committed r223653: <https://trac.webkit.org/changeset/223653>
Comment 12 WebKit Commit Bot 2017-10-18 21:08:44 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2017-10-18 21:09:26 PDT
<rdar://problem/35067339>
Comment 14 Ryan Haddad 2017-10-18 21:36:55 PDT
This change broke the Windows build:
C:\cygwin\home\buildbot\WebKit\Source\WebCore\page\SettingsBase.cpp(562): error C2039: 'setResourceUsageOverlayVisible': is not a member of 'WebCore::Page' [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]
Comment 15 Sam Weinig 2017-10-18 21:40:20 PDT
(In reply to Ryan Haddad from comment #14)
> This change broke the Windows build:
> C:\cygwin\home\buildbot\WebKit\Source\WebCore\page\SettingsBase.cpp(562):
> error C2039: 'setResourceUsageOverlayVisible': is not a member of
> 'WebCore::Page'
> [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.
> vcxproj]

Working on a fix.
Comment 16 Sam Weinig 2017-10-18 21:42:39 PDT
Build fix in r223665.