Bug 86762

Summary: Enable web intents flag for chromium build
Product: WebKit Reporter: Greg Billock <gbillock>
Component: New BugsAssignee: Greg Billock <gbillock>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 75123    
Attachments:
Description Flags
Patch none

Description Greg Billock 2012-05-17 11:48:04 PDT
Enable web intents flag for chromium build
Comment 1 Greg Billock 2012-05-17 11:48:55 PDT
Created attachment 142517 [details]
Patch
Comment 2 Adam Barth 2012-05-17 12:06:19 PDT
Can we unskip some tests now that this feature is enabled during testing?
Comment 3 Greg Billock 2012-05-17 12:47:05 PDT
(In reply to comment #2)
> Can we unskip some tests now that this feature is enabled during testing?

They're not skipped in chromium now, but the tests should now actually be running the full test in the suite instead of skipping the good stuff. (The intent tag test is still skipped.)
Comment 4 Adam Barth 2012-05-17 12:49:18 PDT
> They're not skipped in chromium now, but the tests should now actually be running the full test in the suite instead of skipping the good stuff. (The intent tag test is still skipped.)

What does that mean?  Do we need to update any -expected.txt files?  I'm confused.
Comment 5 Greg Billock 2012-05-17 14:46:13 PDT
(In reply to comment #4)
> > They're not skipped in chromium now, but the tests should now actually be running the full test in the suite instead of skipping the good stuff. (The intent tag test is still skipped.)
> 
> What does that mean?  Do we need to update any -expected.txt files?  I'm confused.

I don't think so (the bot passes fine). My local webkit checkout builds and runs the tests. Am I doing what I think I'm doing with this flag? My expectation was that the webkit chromium builds use it, but the flags can be (and are)set differently in the chromium-initiated builds, from the chromium gypi files. (And that this change merely aligns them.) Is that not the right understanding? The tests were definitely running before this cl (hence the red tests in cr-linux on the delivery change).

IOW, my impression is that the chromium code would no longer need to pass in enable_web_intents=1, but that it in fact does pass that in right now. So they're now more aligned, but there's still a shut-off valve on both sides. Is that right? Or does this change also (inadvertently) disable the enable_web_intents shut-off valve on the Chromium side?
Comment 6 Adam Barth 2012-05-17 15:09:22 PDT
There shouldn't be any misalignment.  We use the same build flags for both.  It sounds like the next step after this patch is to delete the enable_web_intents variable.
Comment 7 Greg Billock 2012-05-17 15:19:38 PDT
(In reply to comment #6)
> There shouldn't be any misalignment.  We use the same build flags for both.  It sounds like the next step after this patch is to delete the enable_web_intents variable.

Right. That sounds good to me.
Comment 8 WebKit Review Bot 2012-05-18 13:32:57 PDT
Comment on attachment 142517 [details]
Patch

Clearing flags on attachment: 142517

Committed r117615: <http://trac.webkit.org/changeset/117615>
Comment 9 WebKit Review Bot 2012-05-18 13:33:02 PDT
All reviewed patches have been landed.  Closing bug.