Bug 77033 – Implement overridePreference for boolean preferences in WebKitTestRunner

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

[Caio Marcelo de Oliveira Filho]

Implement overridePreference for boolean preferences in WebKitTestRunner

[Caio Marcelo de Oliveira Filho]

Created attachment 123982 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 123982 [details]
Patch

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

What resets the preferences back for the next test? Please make sure that this is well understood before landing.

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:67
>  WK_EXPORT void WKBundleOverrideXSSAuditorEnabledForTestRunner(WKBundleRef bundle, WKBundlePageGroupRef pageGroup, bool enabled);

Can this be removed now?

[Caio Marcelo de Oliveira Filho]

Created attachment 124020 [details]
Patch

[Caio Marcelo de Oliveira Filho]

(In reply to comment #2)
> (From update of attachment 123982 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123982&action=review
> 
> What resets the preferences back for the next test? Please make sure that this is well understood before landing.

Thanks for this question! I was overlooking the fact that: since properties were not changing, we were not updating the WebPreferencesStore. And worst, even if I forced an update, it would decode() before the the remove function was called.

I made a small change to the initial patch: instead of overwriting the preferences, the getter now asks the override first. Also made an explicit function that WTR should call to force the update.

> > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:67
> >  WK_EXPORT void WKBundleOverrideXSSAuditorEnabledForTestRunner(WKBundleRef bundle, WKBundlePageGroupRef pageGroup, bool enabled);
> 
> Can this be removed now?

Removed, using WKBundleOverrideBoolPreferenceForTestRunner() instead.

Could you take another look at it?

[Alexey Proskuryakov]

Comment on attachment 124020 [details]
Patch

> I made a small change to the initial patch: instead of overwriting the preferences, the getter now asks the override first.

Thanks! I'd appreciate a detailed explanation for posterity - I think that I understand what you're saying, but only barely, and it will be more difficult in a few months. I.e., what function gets called between tests, what preferences it resets, and how that filters down to each page.

[Caio Marcelo de Oliveira Filho]

Created attachment 124118 [details]
Patch for landing

[Caio Marcelo de Oliveira Filho]

In the patch for landing I've improved the ChangeLog to explain in more detail how the preference override work.

[WebKit Review Bot]

Comment on attachment 124118 [details]
Patch for landing

Clearing flags on attachment: 124118

Committed r106005: <http://trac.webkit.org/changeset/106005>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

> This clearing could be improved by creating a proper message instead of hooking
> at WebPage::preferencesDidChange().

Sounds like a good idea.