Bug 71279 - [Qt] Create infrastructure for Qt's builtin bundle in web process.
Summary: [Qt] Create infrastructure for Qt's builtin bundle in web process.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Marcelo de Oliveira Filho
URL:
Keywords:
Depends on:
Blocks: 70545
  Show dependency treegraph
 
Reported: 2011-11-01 03:56 PDT by Caio Marcelo de Oliveira Filho
Modified: 2011-11-01 06:34 PDT (History)
3 users (show)

See Also:


Attachments
Patch (16.68 KB, patch)
2011-11-01 03:57 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Patch (14.87 KB, patch)
2011-11-01 06:26 PDT, Caio Marcelo de Oliveira Filho
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Marcelo de Oliveira Filho 2011-11-01 03:56:47 PDT
[Qt] Create infrastructure for Qt's builtin bundle in web process.
Comment 1 Caio Marcelo de Oliveira Filho 2011-11-01 03:57:32 PDT
Created attachment 113156 [details]
Patch
Comment 2 Simon Hausmann 2011-11-01 05:17:20 PDT
Comment on attachment 113156 [details]
Patch

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

I suggest renaming QtBundle to QtBuiltinBundle

> Source/WebKit2/WebProcess/qt/QtBundle.cpp:47
> +void QtBundle::initialize(WKBundleRef bundle, WKTypeRef initializationUserData)

initializationUserData seems to be an unused parameter.

> Source/WebKit2/WebProcess/qt/QtBundle.cpp:79
> +    size_t size = m_pages.size();
> +    for (size_t i = 0; i < size; ++i) {

I'd prefer to use of m_page.size() instead of the local size variable. It's not that calling m_pages.size() is expensive ;)

> Source/WebKit2/WebProcess/qt/QtBundle.cpp:90
> +    size_t size = m_pages.size();
> +    for (size_t i = 0; i < size; ++i) {

Same here.

> Source/WebKit2/WebProcess/qt/WebProcessQt.cpp:99
> +        m_injectedBundle->loadWithFunction(qt_wk_WKBundleInitialize);

Why add loadWithFunction to the generic InjectedBundle when you could simply call QtBundle::shared().initialize(toAPI(m_injectedBundle), 0); right there?
Comment 3 Caio Marcelo de Oliveira Filho 2011-11-01 06:26:28 PDT
Created attachment 113165 [details]
Patch
Comment 4 Simon Hausmann 2011-11-01 06:29:22 PDT
Comment on attachment 113165 [details]
Patch

r=me
Comment 5 Caio Marcelo de Oliveira Filho 2011-11-01 06:34:55 PDT
Committed r98957: <http://trac.webkit.org/changeset/98957>