Bug 55212 - Bridge.h should not include BridgeJSC.h
Summary: Bridge.h should not include BridgeJSC.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Steve Block
URL:
Keywords:
Depends on:
Blocks: 55383 55387
  Show dependency treegraph
 
Reported: 2011-02-25 04:49 PST by Steve Block
Modified: 2011-03-01 06:10 PST (History)
6 users (show)

See Also:


Attachments
Patch (20.02 KB, patch)
2011-02-25 04:56 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch (20.06 KB, patch)
2011-02-25 06:26 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch (22.94 KB, patch)
2011-02-25 08:40 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch (22.95 KB, patch)
2011-02-28 16:19 PST, Steve Block
no flags Details | Formatted Diff | Diff
Patch (22.99 KB, patch)
2011-03-01 01:17 PST, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2011-02-25 04:49:02 PST
Bridge.h should not include BridgeJSC.h
Comment 1 Steve Block 2011-02-25 04:54:01 PST
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.
Comment 2 Steve Block 2011-02-25 04:56:09 PST
Created attachment 83796 [details]
Patch
Comment 3 WebKit Review Bot 2011-02-25 04:58:15 PST
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.
Comment 4 Steve Block 2011-02-25 05:01:12 PST
> If any of these errors are false positives, please file a bug against check-webkit-style.
See Bug 33891
Comment 5 Build Bot 2011-02-25 06:23:52 PST
Attachment 83796 [details] did not build on win:
Build output: http://queues.webkit.org/results/8019245
Comment 6 Steve Block 2011-02-25 06:26:41 PST
Created attachment 83800 [details]
Patch
Comment 7 WebKit Review Bot 2011-02-25 06:28:46 PST
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.
Comment 8 Build Bot 2011-02-25 06:36:35 PST
Attachment 83796 [details] did not build on win:
Build output: http://queues.webkit.org/results/8052005
Comment 9 WebKit Review Bot 2011-02-25 08:15:27 PST
Attachment 83800 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8034269
Comment 10 Steve Block 2011-02-25 08:28:38 PST
Comment on attachment 83800 [details]
Patch

Looking into Mac break ...
Comment 11 Steve Block 2011-02-25 08:40:34 PST
Created attachment 83818 [details]
Patch
Comment 12 WebKit Review Bot 2011-02-25 08:43:23 PST
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.
Comment 13 Jeremy Orlow 2011-02-28 15:26:01 PST
Comment on attachment 83818 [details]
Patch

r=me
Comment 14 WebKit Commit Bot 2011-02-28 15:34:57 PST
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
Comment 15 Steve Block 2011-02-28 16:19:44 PST
Created attachment 84147 [details]
Patch
Comment 16 WebKit Review Bot 2011-02-28 16:21:11 PST
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.
Comment 17 Steve Block 2011-02-28 16:24:17 PST
Comment on attachment 84147 [details]
Patch

Rebased only
Comment 18 David Levin 2011-02-28 17:28:48 PST
(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).
Comment 19 David Levin 2011-02-28 17:29:26 PST
Comment on attachment 84147 [details]
Patch

clearing r? as this has already been reviewed and already has the reviewer in the ChangeLog even.
Comment 20 Steve Block 2011-03-01 00:53:52 PST
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.
Comment 21 WebKit Commit Bot 2011-03-01 00:56:24 PST
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
Comment 22 Steve Block 2011-03-01 01:17:34 PST
Created attachment 84200 [details]
Patch
Comment 23 Steve Block 2011-03-01 01:20:15 PST
Comment on attachment 84200 [details]
Patch

Rebased only
Comment 24 WebKit Review Bot 2011-03-01 01:20:18 PST
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.
Comment 25 WebKit Commit Bot 2011-03-01 03:52:06 PST
Comment on attachment 84200 [details]
Patch

Clearing flags on attachment: 84200

Committed r79988: <http://trac.webkit.org/changeset/79988>
Comment 26 WebKit Commit Bot 2011-03-01 03:52:11 PST
All reviewed patches have been landed.  Closing bug.
Comment 27 WebKit Commit Bot 2011-03-01 05:05:45 PST
The commit-queue encountered the following flaky tests while processing attachment 84200 [details]:

media/invalid-media-url-crash.html bug 51138 (authors: inferno@chromium.org and jamesr@chromium.org)
The commit-queue is continuing to process your patch.
Comment 28 David Levin 2011-03-01 06:10:48 PST
(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.)