Bug 55212 – Bridge.h should not include BridgeJSC.h

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

[Steve Block]

Bridge.h should not include BridgeJSC.h

[Steve Block]

Instead, BridgeJSC.h should include Bridge.h and code should include         BridgeJSC.h as appropriate.

This prevents ports that use V8 from having to include JSC-specific files, even if the contents of those files are guarded.

[Steve Block]

Created attachment 83796 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/bridge/testqtbindings.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bridge/testbindings.cpp:23:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 38 files


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

[Steve Block]

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

[Build Bot]

Attachment 83796 [details] did not build on win:
Build output: http://queues.webkit.org/results/8019245

[Steve Block]

Created attachment 83800 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/bridge/testqtbindings.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bridge/testbindings.cpp:23:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 39 files


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

[Build Bot]

Attachment 83796 [details] did not build on win:
Build output: http://queues.webkit.org/results/8052005

[WebKit Review Bot]

Attachment 83800 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8034269

[Steve Block]

Comment on attachment 83800 [details]
Patch

Looking into Mac break ...

[Steve Block]

Created attachment 83818 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/bridge/testqtbindings.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bridge/testbindings.cpp:23:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 44 files


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

[Jeremy Orlow]

Comment on attachment 83818 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 83818 [details]
Patch

Rejecting attachment 83818 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'apply-..." exit_code: 2

Last 500 characters of output:
s/win/PluginViewWin.cpp
patching file Source/WebKit/mac/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/mac/Plugins/Hosted/ProxyInstance.h
patching file Source/WebKit/mac/Plugins/Hosted/WebHostedNetscapePluginView.mm
patching file Source/WebKit/qt/Api/qwebframe.cpp
patching file Source/WebKit/qt/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Jeremy Orlow', u'--for..." exit_code: 1

Full output: http://queues.webkit.org/results/8077179

[Steve Block]

Created attachment 84147 [details]
Patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/bridge/testqtbindings.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bridge/testbindings.cpp:23:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 44 files


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

[Steve Block]

Comment on attachment 84147 [details]
Patch

Rebased only

[David Levin]

(In reply to comment #17)
> (From update of attachment 84147 [details])
> Rebased only

Don't set r? if you don't want a review :).

You can use the commit queue by setting cq+ and leaving r blank if that is what you are looking to do (as long as the reviewer is filled in which it looks like it is).

[David Levin]

Comment on attachment 84147 [details]
Patch

clearing r? as this has already been reviewed and already has the reviewer in the ChangeLog even.

[Steve Block]

Comment on attachment 84147 [details]
Patch

> Don't set r? if you don't want a review :).
> You can use the commit queue by setting cq+ and leaving r blank if that is what you are looking to do (as long as the reviewer is filled in which it looks like it is).
Apologies, I thought that the commit queue wouldn't accept it without this patchset being r+'ed, and that setting r+ myself would confuse it as to the real reviewer.

[WebKit Commit Bot]

Comment on attachment 84147 [details]
Patch

Rejecting attachment 84147 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'ap..." exit_code: 2

Last 500 characters of output:
pp
patching file Source/WebCore/plugins/win/PluginViewWin.cpp
patching file Source/WebKit/mac/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebKit/mac/Plugins/Hosted/ProxyInstance.h
patching file Source/WebKit/mac/Plugins/Hosted/WebHostedNetscapePluginView.mm
patching file Source/WebKit/qt/Api/qwebframe.cpp
patching file Source/WebKit/qt/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.

Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8073589

[Steve Block]

Created attachment 84200 [details]
Patch

[Steve Block]

Comment on attachment 84200 [details]
Patch

Rebased only

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/bridge/testqtbindings.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bridge/testbindings.cpp:23:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 2 in 44 files


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

[WebKit Commit Bot]

Comment on attachment 84200 [details]
Patch

Clearing flags on attachment: 84200

Committed r79988: <http://trac.webkit.org/changeset/79988>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 84200 [details]:

media/invalid-media-url-crash.html bug 51138 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[David Levin]

(In reply to comment #20)
> (From update of attachment 84147 [details])
> > Don't set r? if you don't want a review :).
> > You can use the commit queue by setting cq+ and leaving r blank if that is what you are looking to do (as long as the reviewer is filled in which it looks like it is).
> Apologies, I thought that the commit queue wouldn't accept it without this patchset being r+'ed, and that setting r+ myself would confuse it as to the real reviewer.

My tone came across wrong. I was simply trying to inform. (I wasn't upset at all.)