Bug 112162 – [BlackBerry] Implement platform strategies

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

BlackBerry is one of the few ports not using platform strategies yet.

[Carlos Garcia Campos]

Created attachment 192757 [details]
Patch

[WebKit Review Bot]

Attachment 192757 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Platform.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/PlatformBlackBerry.cmake', u'Source/WebCore/loader/CookieJar.cpp', u'Source/WebCore/loader/blackberry/CookieJarBlackBerry.cpp', u'Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp', u'Source/WebCore/plugins/blackberry/PluginDataBlackBerry.cpp', u'Source/WebKit/ChangeLog', u'Source/WebKit/PlatformBlackBerry.cmake', u'Source/WebKit/blackberry/Api/BlackBerryGlobal.cpp', u'Source/WebKit/blackberry/ChangeLog', u'Source/WebKit/blackberry/WebCoreSupport/PlatformStrategiesBlackBerry.cpp', u'Source/WebKit/blackberry/WebCoreSupport/PlatformStrategiesBlackBerry.h']" exit_code: 1
Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:21:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Comment on attachment 192757 [details]
Patch

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

>> Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:21
>> +
> 
> Alphabetical sorting problem.  [build/include_order] [4]

I think this is correct, this file is the implementation of PlatformCookieJar.h

[Carlos Garcia Campos]

Created attachment 200661 [details]
Updated patch

Patch updated to current git master

[WebKit Commit Bot]

Attachment 200661 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Platform.h', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/PlatformBlackBerry.cmake', u'Source/WebCore/PlatformEfl.cmake', u'Source/WebCore/PlatformWinCE.cmake', u'Source/WebCore/loader/CookieJar.cpp', u'Source/WebCore/loader/blackberry/CookieJarBlackBerry.cpp', u'Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp', u'Source/WebCore/plugins/blackberry/PluginDataBlackBerry.cpp', u'Source/WebKit/ChangeLog', u'Source/WebKit/PlatformBlackBerry.cmake', u'Source/WebKit/blackberry/Api/BlackBerryGlobal.cpp', u'Source/WebKit/blackberry/ChangeLog', u'Source/WebKit/blackberry/WebCoreSupport/PlatformStrategiesBlackBerry.cpp', u'Source/WebKit/blackberry/WebCoreSupport/PlatformStrategiesBlackBerry.h']" exit_code: 1
Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:21:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rob Buis]

Comment on attachment 200661 [details]
Updated patch

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

>> Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:21
>> +
> 
> Alphabetical sorting problem.  [build/include_order] [4]

This could be bogus, since we do not have CookieJarBlackBerry.h.

[Rob Buis]

Comment on attachment 200661 [details]
Updated patch

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

> Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:45
> +    return !cookieManager().cookieJar().isEmpty();

otcheung says: for us, cookieManager().cookieJar() holds the file name of the cookie database file. It's always init to some string object so it'll always return true in this case. since we don't have access to the websettings object through document anymore, we may want to have ftns that toggles the availability of cookies in cookiemanager, and update those values with the toggles in WebPageClientImpl. However, that seems quite unclean because we'll be storing the cookieenabled bool in multiple places.

[Carlos Garcia Campos]

(In reply to comment #6)
> (From update of attachment 200661 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=200661&action=review
> 
> >> Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:21
> >> +
> > 
> > Alphabetical sorting problem.  [build/include_order] [4]
> 
> This could be bogus, since we do not have CookieJarBlackBerry.h.

Yes, this is the implementation of PlatformCookieJar.h

[Carlos Garcia Campos]

(In reply to comment #7)
> (From update of attachment 200661 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=200661&action=review
> 
> > Source/WebCore/platform/network/blackberry/CookieJarBlackBerry.cpp:45
> > +    return !cookieManager().cookieJar().isEmpty();
> 
> otcheung says: for us, cookieManager().cookieJar() holds the file name of the cookie database file. It's always init to some string object so it'll always return true in this case. since we don't have access to the websettings object through document anymore, we may want to have ftns that toggles the availability of cookies in cookiemanager, and update those values with the toggles in WebPageClientImpl. However, that seems quite unclean because we'll be storing the cookieenabled bool in multiple places.

This cookiesEnabled() means whether cookies are available from a platform point of view. See the Navigator::cookieEnabled() implementation for example, it first checks whether the cookies are enabled for the page settings and then it calls cookiesEnabled(). See also Document::cookie() and Document::setCookie(), platform cookies methods are only called if the cookies are enabled in the page settings (and if the security origin allows to access cookies). In the soup implementation, for example, we check if the soup cookie jar feature is available in the soup session.

[Rob Buis]

Comment on attachment 200661 [details]
Updated patch

Ok.

[WebKit Commit Bot]

Comment on attachment 200661 [details]
Updated patch

Clearing flags on attachment: 200661

Committed r150062: <http://trac.webkit.org/changeset/150062>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.