[webkit-reviews] review denied: [Bug 91541] [BlackBerry] Move about: URL handling out of WebCore : [Attachment 153828] fix style

Mon Jul 23 12:37:52 PDT 2012

Rob Buis <rwlbuis at gmail.com> has denied Yong Li <yoli at rim.com>'s request for
review:
Bug 91541: [BlackBerry] Move about: URL handling out of WebCore
https://bugs.webkit.org/show_bug.cgi?id=91541

Attachment 153828: fix style
https://bugs.webkit.org/attachment.cgi?id=153828&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=153828&action=review

Looks good, still some issues.

> Source/WebKit/blackberry/ChangeLog:11
> +	   Other changes: Remove about:version which makes little sense. Make
about:memory partially visible.

Bu about:version is still in use.

> Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:397
> +	       WTF::RefPtr<SharedBuffer> buffer =
SharedBuffer::create(source.is8Bit() ? reinterpret_cast<const
char*>(source.characters8()) : source.latin1().data(), source.length());

No WTF:: prefix needed.

> Source/WebKit/blackberry/WebKitSupport/AboutData.cpp:328
> +    } else if (equalIgnoringCase(aboutWhat, "cache")) {

We seem to be dealing with cache a lot, how about an if block for when
aboutWhat starts with "cache" and then do the subtests?

> Source/WebKit/blackberry/WebKitSupport/AboutData.cpp:344
> +	   result.append(String("<html><head><title>BlackBerry Browser Disk
Cache</title></head><body>Http disk cache is enabled.</body></html>"));

These two blocks are so similar that maybe a helper function can be added for
them.