Bug 47278 – Add WebThemeEngine api for chromium/linux

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Dave Moore]

Currently all scrollbar drawing for chromium / linux is done within platform/chromium/ScrollbarThemeChromiumLinux.cpp. This prevents us from customizing it from the chromium project. We need to add new api to support this.

[Dave Moore]

Created attachment 69958 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 3 in 4 files


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

[Dave Moore]

Created attachment 69960 [details]
Patch

[Tony Chang]

Comment on attachment 69960 [details]
Patch

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

Can you also update WebKit.gyp to include the new files (maybe remove the old file?) and update WebKit/chromium/src/ChromiumBridge.cpp to include win/WebThemeEngine.h?

> ChangeLog:11
> +        * ../../WebKit/chromium/public/WebThemeEngine.h:

This path is incorrect.  Maybe webkit-patch is confused about being an svn checkout in a chromium git checkout?  This path should be relative to the ChangeLog file.

[Dave Moore]

Created attachment 69967 [details]
Patch

[Dave Moore]

Comment on attachment 69967 [details]
Patch

I can't remove the old WebThemeEngine.h without breaking the chromium build. I'll do it once I've added support for the new api there, and changed its includes.

[Tony Chang]

Comment on attachment 69967 [details]
Patch

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

> I can't remove the old WebThemeEngine.h without breaking the chromium build. I'll do it once I've added support for the new api there, and changed its includes.

Right, I just mean to remove it from the gyp file (I think that's harmless, it just changes what file the IDE tries to open).  You can still change the include in WebKit/chromium/src/ChromiumBridge.cpp, right?

r- for ChangeLog

> ChangeLog:1
> [phone]  Dave Moore  <[e-mail]>

Actually, this is the wrong ChangeLog file.  You want to edit WebKit/ChangeLog not the root ChangeLog.  I think if you're going to use webkit-patch, you need to have a full WebKit checkout.  Alternately, you can probably use the svn based scripts from your WebKit/WebKit/ directory (i.e., manually run prepare-ChangeLog and svn-create-diff).

[Dave Moore]

Created attachment 69969 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 6 files


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

[Dave Moore]

Created attachment 69971 [details]
Patch

[Tony Chang]

Comment on attachment 69971 [details]
Patch

LGTM.  Darin, can you do a quick review too?  (Not setting review flag so Darin can review and so cr-linux can run.)

[Darin Fisher (:fishd, Google)]

Comment on attachment 69971 [details]
Patch

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

> WebKit/chromium/public/WebThemeEngine.h:42
> +// This file has been moved to the win subdirectory as it's entirely windows

nit: please prefix this with "FIXME:" for easy grepping later

> WebKit/chromium/public/linux/WebThemeEngine.h:67
> +        // The bounds of the entire track, as opposed to the part being painted.

can you use WebRect here?

   WebRect trackBounds

[Dave Moore]

Created attachment 70013 [details]
Patch

[Kent Tamura]

Comment on attachment 70013 [details]
Patch

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

> ChangeLog:1
> [phone]  Dave Moore  <[e-mail]>

The patch still update a wrong ChangeLog.  It should be WebKit/chromium/ChangeLog, not the root ChangeLog.

[Dave Moore]

Created attachment 70090 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 70090 [details]
Patch

Clearing flags on attachment: 70090

Committed r69311: <http://trac.webkit.org/changeset/69311>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.