Created attachment 105355 [details] Patch Building WebKit for Android requires the toolchain from Android's NDK to be used. Furthermore, the project files need to be taught that OS=android and the DEPS file should be able to have custom dependencies for Android. This patch introduces the --chromium-android command line switch for update-webkit, update-webkit-chromium and build-webkit. It's a bit rough right now, and I'm mostly curious to your opinions about the environment variables being set in webkitdirs.pm. Is this the right approach?
Comment on attachment 105355 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=105355&action=review > Tools/Scripts/update-webkit:103 > + push @chromiumUpdateArgs, "--chromium-android" if isChromiumAndroid(); Thus flag can probably just be --android b/c update-webkit-chromium already implies Chromium. > Tools/Scripts/update-webkit-chromium:37 > +determineIsChromiumAndroid(); I see, but you're just sharing code. Yeah, your way is better. > Tools/Scripts/update-webkit-chromium:63 > + # TODO(beverloo) Add code to automatically download the NDK TODO(beverloo) => FIXME ^^^ WebKit's style is to use FIXME rather than TODO. > Tools/Scripts/update-webkit-chromium:79 > +push @gclientArgs, "--deps=unix,android" if isChromiumAndroid(); We grab the unix deps, even on Mac? > Tools/Scripts/webkitdirs.pm:1912 > + my $ndkBaseDir = getcwd() . "/Source/WebKit/chromium/third_party/android-ndk-r6"; You sure getcwd() is right here? I would have expected sourceDir(). Please make sure this works when run from outside the root before landing.
Thank you for the review! (In reply to comment #1) > (From update of attachment 105355 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105355&action=review > > > Tools/Scripts/update-webkit-chromium:63 > > + # TODO(beverloo) Add code to automatically download the NDK > > TODO(beverloo) => FIXME > ^^^ WebKit's style is to use FIXME rather than TODO. Done. > > Tools/Scripts/update-webkit-chromium:79 > > +push @gclientArgs, "--deps=unix,android" if isChromiumAndroid(); > > We grab the unix deps, even on Mac? Yes, we're building for Android, which is based on Linux (gclient uses "unix" instead of "linux"). > > Tools/Scripts/webkitdirs.pm:1912 > > + my $ndkBaseDir = getcwd() . "/Source/WebKit/chromium/third_party/android-ndk-r6"; > > You sure getcwd() is right here? I would have expected sourceDir(). Please make sure this works when run from outside the root before landing. build-webkit (just like update-webkit and update-webkit-chrmoium) chdir as one of the first steps in their execution. Since sourceDir() ensures this, however, I've updated the line.
Created attachment 105440 [details] Patch Updated patch
Comment on attachment 105440 [details] Patch Clearing flags on attachment: 105440 Committed r93939: <http://trac.webkit.org/changeset/93939>
All reviewed patches have been landed. Closing bug.
Now that update-webkit and build-webkit work for chromium-android, are there plans to update the setup and build instructions for the Chromium Port to cover building on Android? For example, we should probably update http://trac.webkit.org/wiki/Chromium and http://www.webkit.org/building/tools.html. Let me know if I should file a bug.
I think it's better to wait until we have DRT building and linking without local changes, as that's still not the case. Having unusable instructions wouldn't serve a lot of purpose.
Addendum: filing a bug to make sure we don't forget it is fine, of course. Thanks.
Filed Bug 69837. Let's continue discussion there.