Bug 46646 – [chromium] merge WebScreenInfoFactory into a single header

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

[Tony Chang]

[chromium] merge WebScreenInfoFactory into a single header

[Tony Chang]

Created attachment 68948 [details]
Patch

[Tony Chang]

This is a 3 sided change.

1) add WebScreenInfoFactory.h in the new location
2) Roll DEPS to include this change and fix #includes in chromium code
3) Remove the old WebScreenInfoFactory.h files.

There are style errors because these files all have the same name and webkit style is to only include the filename in the include guards.  Once the files are removed (step 3), this won't be a problem.

[WebKit Review Bot]

Attachment 68948 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/public/win/WebScreenInfoFactory.h:31:  #ifndef header guard has wrong style, please use: WebScreenInfoFactory_h  [build/header_guard] [5]
WebKit/chromium/public/mac/WebScreenInfoFactory.h:31:  #ifndef header guard has wrong style, please use: WebScreenInfoFactory_h  [build/header_guard] [5]
WebKit/chromium/public/x11/WebScreenInfoFactory.h:31:  #ifndef header guard has wrong style, please use: WebScreenInfoFactory_h  [build/header_guard] [5]
Total errors found: 3 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Fisher (:fishd, Google)]

What is the reason for this change?  Do you prefer #ifdef soup?

[Tony Chang]

(In reply to comment #4)
> What is the reason for this change?  Do you prefer #ifdef soup?

Yes, I prefer #ifdef soup. :)

It seems more consistent with the rest of WebCore/WebKit and it makes it less confusing when trying to find WebScreenInfoFactory.h when it is included in WebKit.

[Darin Fisher (:fishd, Google)]

(In reply to comment #5)
> (In reply to comment #4)
> > What is the reason for this change?  Do you prefer #ifdef soup?
> 
> Yes, I prefer #ifdef soup. :)
> 
> It seems more consistent with the rest of WebCore/WebKit and it makes it less confusing when trying to find WebScreenInfoFactory.h when it is included in WebKit.

The point of the subdirectories under public/ was to avoid #ifdef soup.  The intent was that platform-specific interfaces live in platform-specific directories.  That way when you look at one of those headers, you know that you only need to worry about said platform.  The only thing common to all platforms is the name of the interface.  That way we don't need an #ifdef in public/ when specifying the interface.

[Tony Chang]

(In reply to comment #6)
> The point of the subdirectories under public/ was to avoid #ifdef soup.  The intent was that platform-specific interfaces live in platform-specific directories.  That way when you look at one of those headers, you know that you only need to worry about said platform.  The only thing common to all platforms is the name of the interface.  That way we don't need an #ifdef in public/ when specifying the interface.

I see.  I guess it feels like the same interface to me (you pass in a window and it gets screen info about the screen that contains the window).  Same with WebInputEventFactory.h.  But I don't feel strongly about this, so I don't mind marking the bug as WONTFIX.