[webkit-reviews] review denied: [Bug 158121] Implement W3C Secure Contexts Draft Specification : [Attachment 312784] Part 4: Add runtime enabled feature flag for isSecureContext

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 158121: Implement W3C Secure Contexts Draft Specification
https://bugs.webkit.org/show_bug.cgi?id=158121

Attachment 312784: Part 4: Add runtime enabled feature flag for isSecureContext

https://bugs.webkit.org/attachment.cgi?id=312784&action=review




--- Comment #55 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 312784
  --> https://bugs.webkit.org/attachment.cgi?id=312784
Part 4: Add runtime enabled feature flag for isSecureContext

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

This looks great, except the fragility of COM Interfaces means that you have to
make a new IWebPreferencesPrivate5 to hold the new method. Otherwise shipping
this code will break existing COM clients that don't expect those new methods
in the interface.

> Source/WebKit/win/Interfaces/IWebPreferencesPrivate.idl:205
> +    HRESULT setIsSecureContextAttributeEnabled([in] BOOL enabled);

These two new values must be moved to a new IWebPreferencesPrivate5 that
inherits from IWebPreferencesPrivate4, or else you will break shipping WebKit
clients.

> Source/WebKit/win/WebView.cpp:5288
> +    hr = prefsPrivate->isSecureContextAttributeEnabled(&enabled);

You must construct an IWebPreferencesPrivate5 now to access this new method. Or
use one of the QueryInterface cast functions to get the proper handle.