Bug 26952

Summary: WebCore Support for the Haiku WebKit port
Product: WebKit Reporter: Maxime Simon <simon.maxime>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, leavengood, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: Other   
Attachments:
Description Flags
Patch to add ChromeClient for Haiku WebCore support.
oliver: review-
Patch to add ContextMenuClient for Haiku WebCore support.
none
Patch to add DragClient for Haiku WebCore support.
none
Patch to add EditorClient for Haiku WebCore support.
none
Patch to add FrameLoaderClient for Haiku WebCore support.
none
Patch to add InspectorClient for Haiku WebCore support.
none
Patch to add ChromeClient for Haiku WebCore support.
none
Patch to add FrameLoaderClient for Haiku WebCore support.
levin: review-
Patch to add ChromeClient for Haiku WebCore support.
levin: review-
Patch to add ChromeClient for Haiku WebCore support.
levin: review+
Patch to add FrameLoaderClient for Haiku WebCore support. levin: review+

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 (no email) 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 (no email) 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.