Bug 52184 - Work towards having the WebPageProxy decide when to create the DrawingAreaProxy
Summary: Work towards having the WebPageProxy decide when to create the DrawingAreaProxy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
: 52183 (view as bug list)
Depends on: 52183
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-10 17:59 PST by Anders Carlsson
Modified: 2011-01-11 09:26 PST (History)
4 users (show)

See Also:


Attachments
Patch (9.04 KB, patch)
2011-01-10 18:04 PST, Anders Carlsson
sam: review+
Details | Formatted Diff | Diff
patch with qt parts (14.49 KB, patch)
2011-01-11 07:33 PST, Balazs Kelemen
no flags Details | Formatted Diff | Diff
Patch (13.08 KB, patch)
2011-01-11 08:38 PST, 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 Anders Carlsson 2011-01-10 17:59:14 PST
Work towards having the WebPageProxy decide when to create the DrawingAreaProxy
Comment 1 Anders Carlsson 2011-01-10 18:04:48 PST
Created attachment 78485 [details]
Patch
Comment 2 WebKit Review Bot 2011-01-10 18:07:24 PST
Attachment 78485 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit2/ChangeLog', u'WebKit2/UIProcess/AP..." exit_code: 1
WebKit2/UIProcess/API/mac/WKViewInternal.h:31:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Anders Carlsson 2011-01-10 18:12:12 PST
Committed r75459: <http://trac.webkit.org/changeset/75459>
Comment 4 Csaba Osztrogonác 2011-01-10 19:56:10 PST
(In reply to comment #3)
> Committed r75459: <http://trac.webkit.org/changeset/75459>

It broke Qt build:
../../../WebKit2/UIProcess/API/qt/qwkpage_p.h:46: error: ‘DrawingAreaProxy’ was not declared in this scope
../../../WebKit2/UIProcess/API/qt/qwkpage_p.h:46: error: template argument 1 is invalid
../../../WebKit2/UIProcess/API/qt/qwkpage_p.h:46: error: conflicting return type specified for ‘virtual int QWKPagePrivate::createDrawingAreaProxy()’
../../../WebKit2/UIProcess/PageClient.h:53: error:   overriding ‘virtual WTF::PassOwnPtr<WebKit::DrawingAreaProxy> WebKit::PageClient::createDrawingAreaProxy()’

Could you guys fix the build or roll it out?
Comment 5 Csaba Osztrogonác 2011-01-10 20:30:11 PST
(In reply to comment #3)
> Committed r75459: <http://trac.webkit.org/changeset/75459>

Rolled-out: http://trac.webkit.org/changeset/75473
Comment 6 Csaba Osztrogonác 2011-01-11 03:30:57 PST
Comment on attachment 78485 [details]
Patch

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

> WebKit2/UIProcess/API/qt/qwkpage_p.h:46
> +    virtual PassOwnPtr<DrawingAreaProxy> createDrawingAreaProxy();

WebKit::DrawingAreaProxy instead of DrawingAreaProxy will make Qt build happier.
Comment 7 Csaba Osztrogonác 2011-01-11 03:41:39 PST
Comment on attachment 78485 [details]
Patch

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

> WebKit2/UIProcess/API/qt/qwkpage.cpp:126
> +PassOwnPtr<DrawingAreaProxy> QWKPagePrivate::createDrawingAreaProxy()
> +{
> +    // FIXME: Implement. See https://bugs.webkit.org/show_bug.cgi?id=52183.
> +}

After fix in Comment #6 I got the following build error:

In file included from ../../../WebKit2/UIProcess/API/qt/qwkpage.cpp:742:
../../../WebKit2/UIProcess/API/qt/qwkpage.cpp: In member function 'virtual WTF::PassOwnPtr<WebKit::DrawingAreaProxy> QWKPagePrivate::createDrawingAreaProxy()':
../../../WebKit2/UIProcess/API/qt/qwkpage.cpp:126: error: control reaches end of non-void function
Comment 8 Balazs Kelemen 2011-01-11 07:33:45 PST
Created attachment 78528 [details]
patch with qt parts

Updated patch to TOT with real (non-stub) implementation of qt part.
Comment 9 WebKit Review Bot 2011-01-11 07:35:10 PST
Attachment 78528 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit2/ChangeLog', u'WebKit2/UIProcess/AP..." exit_code: 1
WebKit2/UIProcess/API/mac/WKViewInternal.h:31:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Andreas Kling 2011-01-11 07:47:52 PST
Comment on attachment 78528 [details]
patch with qt parts

You should get the QGraphicsWKView* via QWKPagePrivate::item instead.
Comment 11 Andreas Kling 2011-01-11 08:01:22 PST
(In reply to comment #10)
> (From update of attachment 78528 [details])
> You should get the QGraphicsWKView* via QWKPagePrivate::item instead.

Ehm. I just learned that the variable I'm referring to has not hit trunk yet (branches.. grmbl..)

We don't want to tie the QWKPage and QGraphicsWKView together like that on the API level though. I'd rather we add a QGraphicsItem* member to QWKPagePrivate and stash the QGraphicsWKView* in there during QWKPagePrivate::init().
Comment 12 Balazs Kelemen 2011-01-11 08:38:16 PST
Created attachment 78533 [details]
Patch
Comment 13 WebKit Review Bot 2011-01-11 08:39:45 PST
Attachment 78533 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebKit2/ChangeLog', u'WebKit2/UIProcess/AP..." exit_code: 1
WebKit2/UIProcess/API/mac/WKViewInternal.h:31:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Andreas Kling 2011-01-11 08:48:40 PST
Comment on attachment 78533 [details]
Patch

Better, thanks. r=me
Comment 15 Balazs Kelemen 2011-01-11 09:22:35 PST
Comment on attachment 78533 [details]
Patch

Clearing flags on attachment: 78533

Committed r75506: <http://trac.webkit.org/changeset/75506>
Comment 16 Balazs Kelemen 2011-01-11 09:22:46 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Balazs Kelemen 2011-01-11 09:26:14 PST
*** Bug 52183 has been marked as a duplicate of this bug. ***