Bug 38989 - [Qt] Remove the unneeded check for QWidgetPageClient @QGraphicsWebView::detachCurrentPage
Summary: [Qt] Remove the unneeded check for QWidgetPageClient @QGraphicsWebView::detac...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Antonio Gomes
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-05-12 06:52 PDT by Antonio Gomes
Modified: 2010-05-12 08:08 PDT (History)
3 users (show)

See Also:


Attachments
patch v1 (2.47 KB, patch)
2010-05-12 06:54 PDT, Antonio Gomes
kenneth: review-
Details | Formatted Diff | Diff
patch v2 (2.61 KB, patch)
2010-05-12 07:21 PDT, Antonio Gomes
kenneth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2010-05-12 06:52:22 PDT
Bug title says it all.

Reasons:
- There is no need to check for a QWidgetPageClient-based in QGraphicsWebView.
- The client has always to be deleted anyways.

patch coming ...
Comment 1 Antonio Gomes 2010-05-12 06:54:57 PDT
Created attachment 55840 [details]
patch v1
Comment 2 Jesus Sanchez-Palencia 2010-05-12 07:04:40 PDT
(In reply to comment #1)
> Created an attachment (id=55840) [details]
> patch v1

I missed that on my PageClient patches. =/
Thanks! LGTM, just remember to add the bug number to the Changelog. :)
Comment 3 Jesus Sanchez-Palencia 2010-05-12 07:09:59 PDT
If we want this to go into the 2.0 release we would need to cherry-pick the PageClient refactor first.
https://bugs.webkit.org/show_bug.cgi?id=37856
Comment 4 Antonio Gomes 2010-05-12 07:11:55 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=55840) [details] [details]
> > patch v1
> 
> I missed that on my PageClient patches. =/
> Thanks! LGTM, just remember to add the bug number to the Changelog. :)

I can fix that while landing (it is done locally already). Thank you.

(In reply to comment #3)
> If we want this to go into the 2.0 release we would need to cherry-pick the PageClient refactor first.
> https://bugs.webkit.org/show_bug.cgi?id=37856

Sure, removing the block for now.
Comment 5 Kenneth Rohde Christiansen 2010-05-12 07:17:09 PDT
Comment on attachment 55840 [details]
patch v1

WebKit/qt/Api/qgraphicswebview.cpp:451
 +          delete page->d->client;
No need to use an if, delete'ing a null pointer does nothing.
Comment 6 Antonio Gomes 2010-05-12 07:21:26 PDT
Created attachment 55843 [details]
patch v2

as requested:

- Adds the bugzilla entry to the ChangeLog
- Removes the "if" wrapping the delete
Comment 7 Antonio Gomes 2010-05-12 08:08:35 PDT
Committed r59234: <http://trac.webkit.org/changeset/59234>