Bug 26952 – WebCore Support for the Haiku WebKit port

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Maxime Simon]

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 )

[Maxime Simon]

Created attachment 32229 [details]
Patch to add ChromeClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 32230 [details]
Patch to add ContextMenuClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 32232 [details]
Patch to add EditorClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 32233 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 32234 [details]
Patch to add InspectorClient for Haiku WebCore support.

[Eric Seidel (no email)]

Comment on attachment 32231 [details]
Patch to add DragClient for Haiku WebCore support.

Seems like you probably wanted DragDestinationActionNone, but OK.

Looks OK.

[Eric Seidel (no email)]

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.

[Oliver Hunt]

Comment on attachment 32229 [details]
Patch to add ChromeClient for Haiku WebCore support.

addMessageToConsole and canRunBeforeUnloadConfirmPanel definitions are not correctly indented

[Oliver Hunt]

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.

[Maxime Simon]

Created attachment 32858 [details]
Patch to add ChromeClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 32860 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.

[Maxime Simon]

(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.

[Adam Barth]

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

[Adam Barth]

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

[Adam Barth]

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

[Adam Barth]

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

[Adam Barth]

All reviewed patches landed, closing.

[Adam Barth]

Again, bugzilla-tool is too eager about closing bugs.  :)

[Oliver Hunt]

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

[Maxime Simon]

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.

[David Levin]

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 <[e-mail]>
> + * Copyright (C) 2006 Zack Rusin <[e-mail]>
> + * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2007 Trolltech ASA
> + * Copyright (C) 2007 Ryan Leavengood <[e-mail]>
> + *
> + * 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 <[e-mail]>
> + * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2007 Ryan Leavengood <[e-mail]>
> + *
> + * 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.

[David Levin]

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 <[e-mail]>
> + * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
> + * Copyright (C) 2007 Ryan Leavengood <[e-mail]>
> + *
> + * 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 <[e-mail]>
> + * Copyright (C) 2007 Ryan Leavengood <[e-mail]>
> + *
> + * All rights reserved.

Ditto :)

> +#ifndef ChromeClientHaiku_H

Use "_h" instead of "_H"

[Maxime Simon]

Created attachment 33607 [details]
Patch to add ChromeClient for Haiku WebCore support.

[Maxime Simon]

Created attachment 33608 [details]
Patch to add FrameLoaderClient for Haiku WebCore support.

[David Levin]

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.

[David Levin]

Commited as http://trac.webkit.org/changeset/46493
and http://trac.webkit.org/changeset/46494