[Webkit-unassigned] [Bug 73215] Split QWebPermissionRequest into QWebSecurityOrigin

[bugzilla-daemon at webkit.org]

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


Rafael Brandao <rafael.lobo at openbossa.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #122049|review-                     |review?
               Flag|                            |




--- Comment #5 from Rafael Brandao <rafael.lobo at openbossa.org>  2012-01-11 12:21:56 PST ---
(From update of attachment 122049)
View in context: https://bugs.webkit.org/attachment.cgi?id=122049&action=review

Do you have any use case in mind for this API? Maybe adding a test to show how it could be used would be very nice. :-)

> Source/WebKit2/ChangeLog:16
> +

I fail to see how this change log is related to the rest of the patch. You've added QtWebSecurityOrigin, but did not mention it here... neither the new files are listed there.

> Source/WebKit2/UIProcess/API/qt/qtwebsecurityorigin.cpp:65
> +bool QtWebSecurityOrigin::isLocalScheme(const QString &scheme)

Style (const QString&)

> Source/WebKit2/UIProcess/API/qt/qtwebsecurityorigin.cpp:67
> +    // FIXME: how to say if a scheme is local or not?

Maybe you could use SchemeRegistry::shouldTreatURLSchemeAsLocal, not sure if this alone is enough.

> Source/WebKit2/UIProcess/API/qt/qtwebsecurityorigin.cpp:77
> +        s_localSchemes << QLatin1String("file") << QLatin1String("qrc");

I believe you don't need to do this... we already have code inside WebCore to deal with local schemes.

> Source/WebKit2/UIProcess/API/qt/qtwebsecurityorigin_p.h:32
> +class QWEBKIT_EXPORT QtWebSecurityOrigin : public QObject {

I believe the naming is incorrect here, the others start with Q prefix instead of Qt, but I see that we already have QWebSecurityOrigin. Maybe someone else has a suggestion how to fix this, I don't know.

> Source/WebKit2/UIProcess/API/qt/qtwebsecurityorigin_p.h:67
> +    bool isLocalScheme(const QString &);

Style (use const QString&, no whitespace)

> Source/WebKit2/UIProcess/API/qt/qwebpermissionrequest_p.h:53
> +    QtWebSecurityOrigin &securityOrigin();

Style (use QWebSecurityOrigin& instead)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.