[webkit-dev] Feature Announcement: Adding <iframe seamless>

[Adam Barth]

On Wed, May 2, 2012 at 12:11 PM, Maciej Stachowiak <mjs at apple.com> wrote:
> On May 2, 2012, at 11:48 AM, Jarred Nicholls <jarred at webkit.org> wrote:
> On Wed, May 2, 2012 at 2:03 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>> On May 2, 2012, at 6:14 AM, Jarred Nicholls <jarred at webkit.org> wrote:
>> On Tue, May 1, 2012 at 7:39 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>>> I'm not too picky about how it's done, but I'd feel more comfortable with
>>> #ifdef protecting the code changes rather than if(). If the changes are so
>>> entangled that it's not easy to put only the changes in an ifdef, then I
>>> would be skeptical that they actually have no possible effect on the
>>> non-seamless case.
>>
>> Do we not have sufficient tests to lend us more confidence in this area?
>>
>> There's no amount of automated testing that would make me comfortable with
>> landing a major feature today and shipping it to customers tomorrow
>> (exaggerated case, but this is the kind of thing we're talking about).
>
>
> I wholeheartedly agree, and agree #ifdefs provide safety in this regard.
>
> I was speaking more towards the skepticism that the runtime conditional
> checks were not adversely affecting the non-seamless case.  I would hope to
> think that our automated tests were (or will be) abundant and thorough
> enough to prove with some level of confidence that what Adam suggests would
> work.  If that isn't the outcome, then one could argue tests are worthless
> in all situations.
>
>
> I'm not necessarily automatically skeptical. I see two possible cases:
>
> 1) Code relevant to the feature is entangled with many parts of existing
> code ==> greater probability of error ==> more value to #ifdefs
> 2) Code relevant to the feature well isolated ==> #ifdefs should be few in
> number and easy to place ==> lower cost to #ifdefs
>
> I don't know what the case against thoroughly deploying #ifdefs is, so I'm
> not sure which of these applies. And maybe I made a mistake and these
> possibilities are not exhaustive.
>
> #ifdefs are valuable and necessary for the reasons you stated, particularly
> security and bugs in new exposed features.  These things ought to be
> gradually exposed, starting with explicit opt-ins.  But, aside from that and
> as a separate issue really, I would hope to think that our tests properly
> mitigate concern for regressions on code that is being modified.
>
>
> I have already explained why I think automated regression tests are not
> sufficient risk mitigation for nontrivial new features by themselves,
> namely, they have not been sufficient in the past. See for example the HTML5
> parser, which was awesome but needed significant bake time to flush out the
> site compatibility regressions and other bugs.
>
> But I would look at it another way: #ifdef is our usual approach to making
> it possible to turn off new code that needs bake time. Folks are welcome to
> propose something other than the usual in specific cases. But there should
> be a positive reason given why a different approach is better. Do you know
> of a specific reason why #ifdefs are not appropriate in this case?

One example from this case is seamless navigation.  I implemented
seamless navigation in two steps:

1) Refactoring the existing codepaths to go through a common function.
2) Teaching the common function how to redirect navigation for
seamless iframes (e.g., hyperlinks inside a seamless iframe navigate
the parent frame).

Step (1) is hard to protect with ifdefs whereas step (2) is easy.
Fortunately, in this specific case, we landed step (1) a while ago, so
it's had some time to bake already.

Adam


> P.S. I have in the past had the personal joy of adding back #ifdefs around a
> feature that had been prematurely enabled unconditionally because it was
> insufficient quality to ship. This is part of why I'm touchy about the idea
> of landing significant new features with no ifdefs. I think we should try to
> avoid that sort of thing and make it feasible to ship something close to
> WebKit trunk at any given time, without need for lengthy branching or
> extensive modifications.
>