Bug 87045 – [chromium] Build content_shell from within WebKit

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

[chromium] Build content_shell from within WebKit

Created attachment 143098 [details]
Patch

this is WIP patch for people that want to experiment with this.

I tested this on Linux only

The required chromium side is here: https://chromiumcodereview.appspot.com/10388218

[Levi Weintraub]

(In reply to comment #2)
> The required chromium side is here: https://chromiumcodereview.appspot.com/10388218

Is there anything stopping us from putting in the Chromium side? It doesn't actually change anything, no?

[Levi Weintraub]

(In reply to comment #3)
> Is there anything stopping us from putting in the Chromium side? It doesn't actually change anything, no?

(I mean in the standard build case)

(In reply to comment #4)
> (In reply to comment #3)
> > Is there anything stopping us from putting in the Chromium side? It doesn't actually change anything, no?
> 
> (I mean in the standard build case)

I just uploaded the CL, gimme a sec!

[Levi Weintraub]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Is there anything stopping us from putting in the Chromium side? It doesn't actually change anything, no?
> > 
> > (I mean in the standard build case)
> 
> I just uploaded the CL, gimme a sec!

Sorry, I wasn't trying to pressure you, just validating my understanding of that patch.

here's the steps to build content_shell right now:

1) update-webkit --chromium
2) webkit-patch apply-attachment --no-clean --no-update 143098
3) cd Source/WebKit/chromium; gclient sync
4) curl -o - https://chromiumcodereview.appspot.com/download/issue10388218_4002.diff | patch -p1
5) python ./gyp_webkit
6) webkit-build --chromium --debug

this should give you out/Debug/content_shell

run with --dump-render-tree for a DRT like interface, or with --remote-debugging-port=9222 for devtools

once the chromium side has landed, steps 4 and 5 are not required

[WebKit Review Bot]

Comment on attachment 143098 [details]
Patch

Attachment 143098 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12740556

[Darin Fisher (:fishd, Google)]

Comment on attachment 143098 [details]
Patch

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

> Source/WebKit/chromium/DEPS:142
> +  'third_party/flac':

don't you think this file is getting out of control?  the webkit repository
shouldn't encode all of these details about what packages are required to
build chromium.  somehow we should extract that information from a file 
maintained in the chromium repository.  webkit repository should just
indicate the version of chromium and the location of the repository.

problem:  if the set of things chromium depends on changes, then you have
to modify this file.  the file listing the things chromium depends on
should live in the chromium repository so that when those changes are
made, they can be made as part of the same CL.

It is already out of control...

I think the right solution is that the.individual modules in chromium like webkit support or content should specify their own DEPS files instead of having this huge single DEPS in src/

[Dirk Pranke]

Comment on attachment 143098 [details]
Patch

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

>> Source/WebKit/chromium/DEPS:142
>> +  'third_party/flac':
> 
> don't you think this file is getting out of control?  the webkit repository
> shouldn't encode all of these details about what packages are required to
> build chromium.  somehow we should extract that information from a file 
> maintained in the chromium repository.  webkit repository should just
> indicate the version of chromium and the location of the repository.
> 
> problem:  if the set of things chromium depends on changes, then you have
> to modify this file.  the file listing the things chromium depends on
> should live in the chromium repository so that when those changes are
> made, they can be made as part of the same CL.

as an alternative to multiple DEPS files in chrome, maybe we need to add some sort of wildcard or dependency or cascade feature into gclient, so you we could say something like 'content_deps': and have that get expanded to some list on the chromium-side ...

[Darin Fisher (:fishd, Google)]

Yes, defining a variable like 'content_deps' on the chromium side that WebKit could use seems good.  Jochen, the multiple DEPS file things suffers from diamond dependency problems.  What happens if two modules don't agree on the version of 'net' or 'base' that they require?  A single master list is simpler.

Created attachment 153768 [details]
Patch

[Peter Beverloo]

Created attachment 154053 [details]
Changes for Android

Hi Jochen, I've attached the changes (to your patch) required to make this work for Android.

At this moment we're not yet able to build content_shell itself. Instead, the content_shell_apk target will be sufficient for now, which right now wraps the Java files and creates an .apk. As work continues in bringing content_shell up on the Chromium side, dependencies will be added to the content_shell_apk target (i.e. one on content_shell itself).

Concretely, this means that it won't work at all for Android :-). As development still is actively ongoing, I think it's best *not* to add a dependency on either content_shell or content_shell_apk for now when building for Android, mostly to avoid random build breakages after Chromium DEPS rolls. Once the work stabilizes and is closer to being finished, we can enable the dependency and make steps towards switching over. Adding the missing dependencies now should be fine, however.

Thank you for the feedback.

Right now, this bug is just for people that want to experiment with content shell in WebKit. I don't think it'll ever land, as webkit should not depend on content

The plan is (roughly) to add methods to webkit_support for creating a webview that will either be created using TestShell or ContentShell

[Mike West]

Created attachment 171464 [details]
Adding Mac DEPS.