Upstream about: feature in WebKit/blackberry/WebCoreSupport/

• Component: Platform
• Status: RESOLVED FIXED

[Leo Yang]

This is the BlackBerry platform specific about data.

[Leo Yang]

Created attachment 117545 [details]
Patch

[Rob Buis]

This looks good. But maybe it can be cleaned up some more, some features got removed, see for instance:

https://bugs.webkit.org/show_bug.cgi?id=68018 (ENABLE_SVG_FOREIGN_OBJECT)

[Eric Seidel (no email)]

Comment on attachment 117545 [details]
Patch

Do other ports do this?  I'm not sure this information really belongs in WebCore.  It seems like embedder-level functionality.

[Leo Yang]

Let me move to WebKit/blackberry/WebCoreSupport/

[Leo Yang]

(In reply to comment #2)
> This looks good. But maybe it can be cleaned up some more, some features got removed, see for instance:
> 
> https://bugs.webkit.org/show_bug.cgi?id=68018 (ENABLE_SVG_FOREIGN_OBJECT)

Thanks. But can I do this in a separated bug? After they are in, I'll file a bug to track up-to-date issue.

[Leo Yang]

Created attachment 117820 [details]
Patch v2

[Antonio Gomes]

(In reply to comment #5)
> (In reply to comment #2)
> > This looks good. But maybe it can be cleaned up some more, some features got removed, see for instance:
> > 
> > https://bugs.webkit.org/show_bug.cgi?id=68018 (ENABLE_SVG_FOREIGN_OBJECT)
> 
> Thanks. But can I do this in a separated bug? After they are in, I'll file a bug to track up-to-date issue.

It is hard to say, Leo. I know our last rebase is behind this macro removals/clean up, but reintroducing them does not sounds right to me.

[Leo Yang]

Created attachment 117849 [details]
Patch v3

OK. Let me up it to date.

[Rob Buis]

Comment on attachment 117849 [details]
Patch v3

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

I think another round is needed, to cleanup the code some more. Good job on figuring out what Features got dropped and added.

> Source/WebKit/blackberry/WebCoreSupport/AboutData.cpp:2126
> +    "YARR_JIT_DEBUG");

The features seem to match now AFAICS. But looking at this repeated structure, this seems to cry out for a macro :)

> Source/WebKit/blackberry/WebCoreSupport/AboutData.cpp:2143
> +static const int s_kilobyte = 1024;

Should use DEFINE_STATIC_LOCAL within cacheTypeStatisticToHTMLTr

> Source/WebKit/blackberry/WebCoreSupport/AboutData.cpp:2153
> +    return tr;

Maybe does not need the temp var.

[Leo Yang]

(In reply to comment #9)
> (From update of attachment 117849 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117849&action=review
> 
> I think another round is needed, to cleanup the code some more. Good job on figuring out what Features got dropped and added.
> 
> > Source/WebKit/blackberry/WebCoreSupport/AboutData.cpp:2126
> > +    "YARR_JIT_DEBUG");
> 
> The features seem to match now AFAICS. But looking at this repeated structure, this seems to cry out for a macro :)
> 
C++ doesn't support pre-processing instructions in a macro. I'll use a perl script to generate code fragments.
 
> > Source/WebKit/blackberry/WebCoreSupport/AboutData.cpp:2143
> > +static const int s_kilobyte = 1024;
> 
> Should use DEFINE_STATIC_LOCAL within cacheTypeStatisticToHTMLTr
Actually static is not needed. I'll change to const int s_kiloByte = 1024 and put it in the cacheTypeStatisticToHTMLTr.

> 
> > Source/WebKit/blackberry/WebCoreSupport/AboutData.cpp:2153
> > +    return tr;
> 
> Maybe does not need the temp var.
OK.

[Leo Yang]

Created attachment 118003 [details]
Patch v4

Using perl script to generate code fragments.

[Leo Yang]

Committed r102207: <http://trac.webkit.org/changeset/102207>