Bug 82677 – [Chromium] Delete WebKit/chromium/bridge

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

[Chromium] Delete WebKit/chromium/bridge

[Adam Barth]

Created attachment 134695 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/Platform/ChangeLog', u'Source/Platf..." exit_code: 1
Source/WebCore/platform/chromium/support/WebMediaStreamSourcesRequest.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamSourcesRequest.cpp:38:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaHints.cpp:37:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebICEOptions.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamComponent.cpp:37:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebICECandidateDescriptor.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamSource.cpp:37:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamDescriptor.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/chromium/support/WebMediaStreamDescriptor.cpp:38:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 9 in 52 files


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

[Tommy Widenflycht]

I like this patch a lot  and it can be further simplified by deleting the *Internal classes (to my eternal joy and happiness). I'll be happy to do this in a follow-up patch.

[Tommy Widenflycht]

There seems to be a major problem with the patch in that the files going to Source/Platform/chromium/public are missing. 

You could try generating the patch yourself with 
git diff --no-renames --no-prefix master..HEAD >some_filename
and uploading it manually. If you are using git that is.

[James Robinson]

From the actual patch:

Index: Source/Platform/chromium/public/WebICECandidateDescriptor.h
===================================================================
--- Source/Platform/chromium/public/WebICECandidateDescriptor.h	(revision 112571)	(from Source/WebKit/chromium/public/platform/WebICECandidateDescriptor.h:112571)
+++ Source/Platform/chromium/public/WebICECandidateDescriptor.h	(working copy)
Index: Source/Platform/chromium/public/WebICEOptions.h
===================================================================
--- Source/Platform/chromium/public/WebICEOptions.h	(revision 112571)	(from Source/WebKit/chromium/public/platform/WebICEOptions.h:112571)
+++ Source/Platform/chromium/public/WebICEOptions.h	(working copy)


that looks fine to me. I'll patch this in locally to review

[James Robinson]

Comment on attachment 134695 [details]
Patch

Looks great! R=me. You've got merge conflicts in the .gypis, but that's not a huge surprise.

[Adam Barth]

Unfortunately, some of the earlier patches were rolled back, so I'll need to figure them out first,

[Adam Barth]

Committed r112784: <http://trac.webkit.org/changeset/112784>

[Adam Barth]

> Unfortunately, some of the earlier patches were rolled back, so I'll need to figure them out first,

I managed to beat the Windows component build into shape and was able to re-land the earlier patches.

> I like this patch a lot  and it can be further simplified by deleting the *Internal classes (to my eternal joy and happiness). I'll be happy to do this in a follow-up patch.

Yay!  That's the goal.  Getting rid of the "Internal" classes would be quite welcome.  Depending on how that works, we might need to do some more build system work.  Let me know if you run into any difficulties.