Bug 48327 - [Qt] Assert due to multiple initialization of WebPlatformStrategies
Summary: [Qt] Assert due to multiple initialization of WebPlatformStrategies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: QtWebKit Unassigned
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2010-10-26 07:55 PDT by zalan
Modified: 2010-10-27 08:24 PDT (History)
4 users (show)

See Also:


Attachments
remove multiple init (1.25 KB, patch)
2010-10-27 00:02 PDT, zalan
no flags Details | Formatted Diff | Diff
second attempt to fix the original issue (1.73 KB, patch)
2010-10-27 06:17 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2010-10-26 07:55:27 PDT
https://bugs.webkit.org/show_bug.cgi?id=46078 introduced WebPlatformStrategies::initialize() in QWKPagePrivate::QWKPagePrivate() to initialize the platform strategies in the UI process. Initialization should be done only once, and not per page. Multiple initialization triggers assert in WebCore/PlatformStrategies. The code should mimic the web process side instead. (init is part of the webprocess constuct).

I'll upload a patch soon, which removes the initialization, as it is not needed at all atm.
Comment 1 zalan 2010-10-27 00:02:30 PDT
Created attachment 71990 [details]
remove multiple init
Comment 2 WebKit Commit Bot 2010-10-27 01:19:23 PDT
Comment on attachment 71990 [details]
remove multiple init

Clearing flags on attachment: 71990

Committed r70620: <http://trac.webkit.org/changeset/70620>
Comment 3 WebKit Commit Bot 2010-10-27 01:19:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Balazs Kelemen 2010-10-27 06:03:35 PDT
This makes Minibrowser crashing immediately.
Comment 5 Balazs Kelemen 2010-10-27 06:17:46 PDT
Created attachment 72024 [details]
second attempt to fix the original issue
Comment 6 Andreas Kling 2010-10-27 06:57:10 PDT
Comment on attachment 72024 [details]
second attempt to fix the original issue

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

r=me, one comment:

> WebKit2/UIProcess/API/qt/qwkpage.cpp:55
> +    if (!initialized) {

Use early return.
Comment 7 zalan 2010-10-27 07:06:44 PDT
oh, sorry. i was about to post the fix on that.
Comment 8 Balazs Kelemen 2010-10-27 07:15:20 PDT
(In reply to comment #6)
> (From update of attachment 72024 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72024&action=review
> 
> r=me, one comment:
> 
> > WebKit2/UIProcess/API/qt/qwkpage.cpp:55
> > +    if (!initialized) {
> 
> Use early return.

I will. I know that this is the preferred style, but personally I hate that.
There are a lot situations where the early return makes the code harder to read.
Comment 9 Balazs Kelemen 2010-10-27 08:24:16 PDT
Committed in http://trac.webkit.org/changeset/70645