Bug 106213 – Make the IFRAME_SEAMLESS flag runtime-enabled.

• Component: New Bugs
• Status: RESOLVED FIXED

[Mike West]

Make the IFRAME_SEAMLESS flag runtime-enabled.

[Mike West]

Created attachment 181486 [details]
Patch

[Mike West]

Hi Eric! We talked last week about hiding IFRAME_SEAMLESS behind a runtime flag to ease the burden on Chromium's release team. This patch takes care of the WebKit side of that.

+hyatt, as this might interest you as well.

[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.

[Mike West]

Hrm. I was a bit too quick to upload a patch. I'm cargo-culting my way to (hopefully) successfully enabling the flag for the various ports' DRT; new patch shortly.

[Mike West]

Created attachment 181495 [details]
Cargo culting.

[Mike West]

I'd love someone to take a look at this, especially the mac bits. I'm totally guessing. :)

[Eric Seidel (no email)]

What's the difference between a runtime feature and a Setting?  Settings are per page? Features are global?

[Mike West]

(In reply to comment #7)
> What's the difference between a runtime feature and a Setting?  Settings are per page? Features are global?

That's my vague understanding, yes. Adam pointed me to RuntimeEnabledFeatures for our work on CSP 1.1; it seems applicable here as well. :)

[Ojan Vafai]

Comment on attachment 181495 [details]
Cargo culting.

We have too many ways of doing runtime feature enabling. That's not this patch's fault though.

[Adam Barth]

> We have too many ways of doing runtime feature enabling.

Yeah.

> What's the difference between a runtime feature and a Setting?  Settings are per page? Features are global?

Pretty much.  Also, Settings can be changed over the lifetime of the process and often over the lifetime of a given Page.  Runtime features are supposed to be initialized at startup and then never changed again.

The original reason we added the global runtime features is that we wanted to hide JavaScript API completely at runtime.  In order to do that, we needed to know the value of the setting when creating the ObjectTemplates, which, once created, cannot be changed.

[Adam Barth]

Comment on attachment 181495 [details]
Cargo culting.

API change LGTM

[WebKit Review Bot]

Comment on attachment 181495 [details]
Cargo culting.

Clearing flags on attachment: 181495

Committed r138983: <http://trac.webkit.org/changeset/138983>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #12)
> (From update of attachment 181495 [details])
> Clearing flags on attachment: 181495
> 
> Committed r138983: <http://trac.webkit.org/changeset/138983>

It broke the !ENABLE_IFRAME_SEAMLESS builds:

/ramdisk/qt-linux-release-minimal/build/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp: In static member function 'static void DumpRenderTreeSupportQt::setSeamlessIFramesEnabled(bool)':
/ramdisk/qt-linux-release-minimal/build/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:1055: error: 'setSeamlessIFramesEnabled' is not a member of 'WebCore::RuntimeEnabledFeatures'

[Mike West]

(In reply to comment #14)
> (In reply to comment #12)
> > (From update of attachment 181495 [details] [details])
> > Clearing flags on attachment: 181495
> > 
> > Committed r138983: <http://trac.webkit.org/changeset/138983>
> 
> It broke the !ENABLE_IFRAME_SEAMLESS builds:
> 
> /ramdisk/qt-linux-release-minimal/build/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp: In static member function 'static void DumpRenderTreeSupportQt::setSeamlessIFramesEnabled(bool)':
> /ramdisk/qt-linux-release-minimal/build/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:1055: error: 'setSeamlessIFramesEnabled' is not a member of 'WebCore::RuntimeEnabledFeatures'

Hrm. It passed both the qt and qt-wk2 EWS bots. :(

I think the fix should be straightforward though. I'll have a patch up shortly.

[Antoine Quint]

Reopening to attach new patch.

[Antoine Quint]

Created attachment 181562 [details]
Patch

[Antoine Quint]

A mishapp with webkit-patch caused me to upload a patch on this bug meant for another bug, sorry.

[Mike West]

(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #12)
> > > (From update of attachment 181495 [details] [details] [details])
> > > Clearing flags on attachment: 181495
> > > 
> > > Committed r138983: <http://trac.webkit.org/changeset/138983>
> > 
> > It broke the !ENABLE_IFRAME_SEAMLESS builds:
> > 
> > /ramdisk/qt-linux-release-minimal/build/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp: In static member function 'static void DumpRenderTreeSupportQt::setSeamlessIFramesEnabled(bool)':
> > /ramdisk/qt-linux-release-minimal/build/Source/WebKit/qt/WebCoreSupport/DumpRenderTreeSupportQt.cpp:1055: error: 'setSeamlessIFramesEnabled' is not a member of 'WebCore::RuntimeEnabledFeatures'
> 
> Hrm. It passed both the qt and qt-wk2 EWS bots. :(
> 
> I think the fix should be straightforward though. I'll have a patch up shortly.

https://bugs.webkit.org/show_bug.cgi?id=106252

[Roger Fong]

Is this feature supported on Windows?
I'm seeing 27 seamless frame tests failing on the Apple Win 7test bots.
http://build.webkit.org/results/Apple%20Win%207%20Release%20(Tests)/r138986%20(31310)/results.html

[Eric Seidel (no email)]

There is nothing windows-specific about this (9mo) feature. :)  It's just a question of if it was ever enabled in the vsproj or not.

[Roger Fong]

(In reply to comment #21)
> There is nothing windows-specific about this (9mo) feature. :)  It's just a question of if it was ever enabled in the vsproj or not.

The feature flag has been enabled but it looks like none of the webpreferences have been so Runtime enabled features flag will be false,
which would cause Document::shouldDisplaySeamlesslyWithParent to return false.

I'll go ahead and add the extra layer.