Bug 177766 – [Settings] Convert more settings to be generated

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

[Settings] Convert more settings to be generated

[Sam Weinig]

Created attachment 322414 [details]
Patch

[Sam Weinig]

Created attachment 322489 [details]
Patch

[Fujii Hironori]

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

[Sam Weinig]

(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.

[Fujii Hironori]

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"

[Sam Weinig]

Created attachment 324165 [details]
Patch

[Sam Weinig]

Created attachment 324168 [details]
Patch

[Sam Weinig]

Created attachment 324190 [details]
Patch

[Sam Weinig]

Created attachment 324191 [details]
Patch

[Dean Jackson]

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

[WebKit Commit Bot]

Comment on attachment 324191 [details]
Patch

Clearing flags on attachment: 324191

Committed r223653: <https://trac.webkit.org/changeset/223653>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35067339>

[Ryan Haddad]

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]

[Sam Weinig]

(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.

[Sam Weinig]

Build fix in r223665.