Bug 112162

Summary: [BlackBerry] Implement platform strategies
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit BlackBerryAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, gyuyoung.kim, japhet, mifenton, ojan.autocc, otcheung, rakuco, rwlbuis, staikos, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 114431    
Attachments:
Description Flags
Patch
none
Updated patch none

Description Carlos Garcia Campos 2013-03-12 10:05:44 PDT
BlackBerry is one of the few ports not using platform strategies yet.
Comment 1 Carlos Garcia Campos 2013-03-12 10:13:09 PDT
Created attachment 192757 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-12 10:16:23 PDT
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.
Comment 3 Carlos Garcia Campos 2013-03-12 11:02:21 PDT
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
Comment 4 Carlos Garcia Campos 2013-05-06 05:37:51 PDT
Created attachment 200661 [details]
Updated patch

Patch updated to current git master
Comment 5 WebKit Commit Bot 2013-05-06 05:40:47 PDT
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.
Comment 6 Rob Buis 2013-05-06 11:33:06 PDT
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.
Comment 7 Rob Buis 2013-05-06 11:56:33 PDT
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.
Comment 8 Carlos Garcia Campos 2013-05-07 00:39:17 PDT
(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
Comment 9 Carlos Garcia Campos 2013-05-07 00:45:37 PDT
(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.
Comment 10 Rob Buis 2013-05-13 09:10:09 PDT
Comment on attachment 200661 [details]
Updated patch

Ok.
Comment 11 WebKit Commit Bot 2013-05-14 03:11:58 PDT
Comment on attachment 200661 [details]
Updated patch

Clearing flags on attachment: 200661

Committed r150062: <http://trac.webkit.org/changeset/150062>
Comment 12 WebKit Commit Bot 2013-05-14 03:12:03 PDT
All reviewed patches have been landed.  Closing bug.