Bug 26952 - WebCore Support for the Haiku WebKit port
: WebCore Support for the Haiku WebKit port
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebKit Misc.
: 528+ (Nightly build)
: All Other
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-03 02:44 PDT by Maxime Simon
Modified: 2009-07-28 12:00 PDT (History)
3 users (show)

See Also:


Attachments
Patch to add ChromeClient for Haiku WebCore support. (17.75 KB, patch)
2009-07-03 02:54 PDT, Maxime Simon
oliver: review-
Details | Formatted Diff | Diff
Patch to add ContextMenuClient for Haiku WebCore support. (7.00 KB, patch)
2009-07-03 03:07 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add DragClient for Haiku WebCore support. (6.81 KB, patch)
2009-07-03 03:16 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add EditorClient for Haiku WebCore support. (23.20 KB, patch)
2009-07-03 03:28 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add FrameLoaderClient for Haiku WebCore support. (41.68 KB, patch)
2009-07-03 03:38 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add InspectorClient for Haiku WebCore support. (8.44 KB, patch)
2009-07-03 03:43 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add ChromeClient for Haiku WebCore support. (18.14 KB, patch)
2009-07-16 04:44 PDT, Maxime Simon
no flags Details | Formatted Diff | Diff
Patch to add FrameLoaderClient for Haiku WebCore support. (43.49 KB, patch)
2009-07-16 04:58 PDT, Maxime Simon
levin: review-
Details | Formatted Diff | Diff
Patch to add ChromeClient for Haiku WebCore support. (18.18 KB, patch)
2009-07-19 01:21 PDT, Maxime Simon
levin: review-
Details | Formatted Diff | Diff
Patch to add ChromeClient for Haiku WebCore support. (18.51 KB, patch)
2009-07-28 03:03 PDT, Maxime Simon
levin: review+
Details | Formatted Diff | Diff
Patch to add FrameLoaderClient for Haiku WebCore support. (43.92 KB, patch)
2009-07-28 03:22 PDT, Maxime Simon
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maxime Simon 2009-07-03 02:44:41 PDT
As the previous bug contains too much patches,
( https://bugs.webkit.org/show_bug.cgi?id=26620 )
I fill a new bug with only patches related to WebCore Support.
( located in WebKit/haiku/WebCoreSupport )
Comment 1 Maxime Simon 2009-07-03 02:54:15 PDT
Created attachment 32229 [details]
Patch to add ChromeClient for Haiku WebCore support.
Comment 2 Maxime Simon 2009-07-03 03:07:51 PDT
Created attachment 32230 [details]
Patch to add ContextMenuClient for Haiku WebCore support.
Comment 3 Maxime Simon 2009-07-03 03:16:12 PDT
Created attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.
Comment 4 Maxime Simon 2009-07-03 03:28:53 PDT
Created attachment 32232 [details]
Patch to add EditorClient for Haiku WebCore support.
Comment 5 Maxime Simon 2009-07-03 03:38:03 PDT
Created attachment 32233 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
Comment 6 Maxime Simon 2009-07-03 03:43:27 PDT
Created attachment 32234 [details]
Patch to add InspectorClient for Haiku WebCore support.
Comment 7 Eric Seidel 2009-07-07 00:21:05 PDT
Comment on attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.

Seems like you probably wanted DragDestinationActionNone, but OK.

Looks OK.
Comment 8 Eric Seidel 2009-07-07 00:21:53 PDT
Comment on attachment 32234 [details]
Patch to add InspectorClient for Haiku WebCore support.

Looks OK.  We've often used TemporaryLinkStubs for such things, but this is also fine.
Comment 9 Oliver Hunt 2009-07-16 00:09:36 PDT
Comment on attachment 32229 [details]
Patch to add ChromeClient for Haiku WebCore support.

addMessageToConsole and canRunBeforeUnloadConfirmPanel definitions are not correctly indented
Comment 10 Oliver Hunt 2009-07-16 00:15:14 PDT
Comment on attachment 32233 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.

FrameLoaderClientHaiku::saveDocumentViewToCachedPage shouldn't be present if its commented out.  But aside from that i'd prefer it if brady or antti reviewed this patch.
Comment 11 Maxime Simon 2009-07-16 04:44:49 PDT
Created attachment 32858 [details]
Patch to add ChromeClient for Haiku WebCore support.
Comment 12 Maxime Simon 2009-07-16 04:58:09 PDT
Created attachment 32860 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
Comment 13 Maxime Simon 2009-07-16 05:02:30 PDT
(In reply to comment #10)
> (From update of attachment 32233 [details])
> FrameLoaderClientHaiku::saveDocumentViewToCachedPage shouldn't be present if
> its commented out.  But aside from that i'd prefer it if brady or antti
> reviewed this patch.

Since I commited the old patch I made some changes.
So I commited a new patch. There is no more commented out method.
Comment 14 Adam Barth 2009-07-16 23:02:28 PDT
Comment on attachment 32230 [details]
Patch to add ContextMenuClient for Haiku WebCore support.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/ChangeLog
	A	WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.h
Committed r46013
	A	WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.h
	A	WebKit/haiku/WebCoreSupport/ContextMenuClientHaiku.cpp
	M	WebKit/ChangeLog
r46013 = c289b08492c0915116b42e31e5a6a33201fabcd2 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46013
Comment 15 Adam Barth 2009-07-16 23:02:43 PDT
Comment on attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/ChangeLog
	A	WebKit/haiku/WebCoreSupport/DragClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/DragClientHaiku.h
Committed r46014
	A	WebKit/haiku/WebCoreSupport/DragClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/DragClientHaiku.h
	M	WebKit/ChangeLog
r46014 = 1f86110d811f8df279aa86457ba1ae333c89d41e (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46014
Comment 16 Adam Barth 2009-07-16 23:02:59 PDT
Comment on attachment 32232 [details]
Patch to add EditorClient for Haiku WebCore support.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/ChangeLog
	A	WebKit/haiku/WebCoreSupport/EditorClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/EditorClientHaiku.h
Committed r46015
	A	WebKit/haiku/WebCoreSupport/EditorClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/EditorClientHaiku.h
	M	WebKit/ChangeLog
r46015 = 38130bab20f4c45ac67252185d757cf5bfc243b3 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46015
Comment 17 Adam Barth 2009-07-16 23:03:15 PDT
Comment on attachment 32234 [details]
Patch to add InspectorClient for Haiku WebCore support.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/ChangeLog
	A	WebKit/haiku/WebCoreSupport/InspectorClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/InspectorClientHaiku.h
Committed r46016
	A	WebKit/haiku/WebCoreSupport/InspectorClientHaiku.cpp
	A	WebKit/haiku/WebCoreSupport/InspectorClientHaiku.h
	M	WebKit/ChangeLog
r46016 = adf9bbe9de1e7ae895f888409449f552d061b9d8 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46016
Comment 18 Adam Barth 2009-07-16 23:03:19 PDT
All reviewed patches landed, closing.
Comment 19 Adam Barth 2009-07-16 23:03:52 PDT
Again, bugzilla-tool is too eager about closing bugs.  :)
Comment 20 Oliver Hunt 2009-07-18 17:14:53 PDT
Comment on attachment 32858 [details]
Patch to add ChromeClient for Haiku WebCore support.

Is it a deliberate choice to leave the printf's in the ChromeClientHaiku::runJavaScript* methods?  Other than those this patch seems good but i'd like to confirm that those are/are not deliberate
Comment 21 Maxime Simon 2009-07-19 01:21:47 PDT
Created attachment 33037 [details]
Patch to add ChromeClient for Haiku WebCore support.

Indeed, it wasn't my intention to leave the printf's in the runJavaScript[...] methods.
Comment 22 David Levin 2009-07-28 02:44:59 PDT
Comment on attachment 32860 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.

Just a few issues to address.


> diff --git a/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp b/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp
> +/*
> + * Copyright (C) 2006 Don Gibson <dgibson77@gmail.com>
> + * Copyright (C) 2006 Zack Rusin <zack@kde.org>
> + * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2007 Trolltech ASA
> + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com>
> + *
> + * All rights reserved.

This is an odd location for "All rights reserved."  It typically follows the copyright lines.

> +#include "config.h"
> +#include "FrameLoaderClientHaiku.h"
> +
> +#include "DocumentLoader.h"
> +#include "FrameLoader.h"
> +#include "FrameTree.h"
> +#include "Frame.h"
> +#include "FrameView.h"
> +#include "HTMLFrameOwnerElement.h"
> +#include "Page.h"
> +#include "PlatformString.h"
> +#include "ResourceRequest.h"
> +
> +#include "NotImplemented.h"
This appears out of sort order.

> +void FrameLoaderClientHaiku::setWebView(WebView* webview)
webView

> +bool FrameLoaderClientHaiku::hasWebView() const
> +{
> +    return m_webView != NULL;
Avoid NULL (use 0), but also avoid comparisons to 0/NULL.

    return m_webView;

> +void FrameLoaderClientHaiku::dispatchWillPerformClientRedirect(const KURL&,
> +                                                               double interval,
> +                                                               double fireDate)
Feel free to unwrap these parameters into one line.


> +void FrameLoaderClientHaiku::didChangeTitle(DocumentLoader *docLoader)

"*" in wrong location. (DocumentLoader* docLoader)



> +    if (m_firstData) {
> +        FrameLoader *frameLoader = loader->frameLoader();

"*" in wrong location.

> +{
> +	// FIXME:

Indentation is off.


> diff --git a/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.h b/WebKit/haiku/WebCoreSupport/FrameLoaderClientHaiku.h
> @@ -0,0 +1,248 @@
> +/*
> + * Copyright (C) 2006 Zack Rusin <zack@kde.org>
> + * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com>
> + *
> + * All rights reserved.

Same comment as before.

> +#ifndef FrameLoaderClientHaiku_H

Use lowercase h like this:
FrameLoaderClientHaiku_h

> +#include "FrameLoaderClient.h"
> +#include "FrameLoader.h"

Out of sort order.

> +    class NavigationAction;
> +    class String;
> +    class ResourceLoader;

Out of sort order.

> +        void setFrame(Frame *frame);

Remove parameter names if they don't add any information.
This should be 
  "void setFrame(Frame*);"

> +        void setWebView(WebView *webview);

Remove parameter names if they don't add any information.


> +        virtual void dispatchDecidePolicyForMIMEType(FramePolicyFunction function,

Remove parameter names if they don't add any information.

> +        virtual void dispatchDecidePolicyForNewWindowAction(FramePolicyFunction function,

Remove parameter names if they don't add any information.

> +        virtual void dispatchDecidePolicyForNavigationAction(FramePolicyFunction function,

Remove parameter names if they don't add any information.


> +        // FIXME: This should probably not be here, but it's needed for the tests currently
Add "."


> +        virtual PassRefPtr<Frame> createFrame(const KURL& url, const String& name,
> +                                              HTMLFrameOwnerElement* ownerElement,

Remove parameter names if they don't add any information: ownerElement.


> +        virtual PassRefPtr<Widget> createPlugin(const IntSize&, HTMLPlugInElement*, const KURL&,
> +                                     const Vector<String>&, const Vector<String>&, const String&,

indentation is off.

> +                                     bool loadManually) ;

> +
> +        virtual PassRefPtr<Widget> createJavaAppletWidget(const IntSize&, HTMLAppletElement*,
> +                                               const KURL& baseURL, const Vector<String>& paramNames,

indentation is off.

> +                                               const Vector<String>& paramValues);

> +        Frame *m_frame;

"*" in wrong location.

> +        WebView *m_webView;

"*" in wrong location.
Comment 23 David Levin 2009-07-28 02:50:01 PDT
Comment on attachment 33037 [details]
Patch to add ChromeClient for Haiku WebCore support.

Just a few comments to address and this will be ready to land.

> diff --git a/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp b/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.cpp
> @@ -0,0 +1,360 @@
> +/*
> + * Copyright (C) 2006 Zack Rusin <zack@kde.org>
> + * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com>
> + *
> + * All rights reserved.

Looks odd as before.
> +
> +#include "Frame.h"
> +#include "FrameView.h"
> +#include "FrameLoadRequest.h"
> +#include "PlatformString.h"
> +#include "HitTestResult.h"
> +
> +#include "NotImplemented.h"

Sort all of these.


> diff --git a/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.h b/WebKit/haiku/WebCoreSupport/ChromeClientHaiku.h
> + * Copyright (C) 2006 Zack Rusin <zack@kde.org>
> + * Copyright (C) 2007 Ryan Leavengood <leavengood@gmail.com>
> + *
> + * All rights reserved.

Ditto :)

> +#ifndef ChromeClientHaiku_H

Use "_h" instead of "_H"
Comment 24 Maxime Simon 2009-07-28 03:03:06 PDT
Created attachment 33607 [details]
Patch to add ChromeClient for Haiku WebCore support.
Comment 25 Maxime Simon 2009-07-28 03:22:33 PDT
Created attachment 33608 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
Comment 26 David Levin 2009-07-28 11:29:45 PDT
Comment on attachment 33607 [details]
Patch to add ChromeClient for Haiku WebCore support.

> +
> +        #if ENABLE(OFFLINE_WEB_APPLICATIONS)
> +        virtual void reachedMaxAppCacheSize(int64_t spaceNeeded);
> +        #endif

The #if (and #endif) shouldn't be indented.