Bug 26952 - WebCore Support for the Haiku WebKit port
: WebCore Support for the Haiku WebKit port
Status: RESOLVED FIXED
: WebKit
WebKit Misc.
: 528+ (Nightly build)
: All Other
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-07-03 02:44 PST by
Modified: 2009-07-28 12:00 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-07-03 02:44:41 PST
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 From 2009-07-03 02:54:15 PST -------
Created an attachment (id=32229) [details]
Patch to add ChromeClient for Haiku WebCore support.
------- Comment #2 From 2009-07-03 03:07:51 PST -------
Created an attachment (id=32230) [details]
Patch to add ContextMenuClient for Haiku WebCore support.
------- Comment #3 From 2009-07-03 03:16:12 PST -------
Created an attachment (id=32231) [details]
Patch to add DragClient for Haiku WebCore support.
------- Comment #4 From 2009-07-03 03:28:53 PST -------
Created an attachment (id=32232) [details]
Patch to add EditorClient for Haiku WebCore support.
------- Comment #5 From 2009-07-03 03:38:03 PST -------
Created an attachment (id=32233) [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
------- Comment #6 From 2009-07-03 03:43:27 PST -------
Created an attachment (id=32234) [details]
Patch to add InspectorClient for Haiku WebCore support.
------- Comment #7 From 2009-07-07 00:21:05 PST -------
(From update of attachment 32231 [details])
Seems like you probably wanted DragDestinationActionNone, but OK.

Looks OK.
------- Comment #8 From 2009-07-07 00:21:53 PST -------
(From update of attachment 32234 [details])
Looks OK.  We've often used TemporaryLinkStubs for such things, but this is also fine.
------- Comment #9 From 2009-07-16 00:09:36 PST -------
(From update of attachment 32229 [details])
addMessageToConsole and canRunBeforeUnloadConfirmPanel definitions are not correctly indented
------- Comment #10 From 2009-07-16 00:15:14 PST -------
(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.
------- Comment #11 From 2009-07-16 04:44:49 PST -------
Created an attachment (id=32858) [details]
Patch to add ChromeClient for Haiku WebCore support.
------- Comment #12 From 2009-07-16 04:58:09 PST -------
Created an attachment (id=32860) [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
------- Comment #13 From 2009-07-16 05:02:30 PST -------
(In reply to comment #10)
> (From update of attachment 32233 [details] [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 From 2009-07-16 23:02:28 PST -------
(From update of attachment 32230 [details])
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 From 2009-07-16 23:02:43 PST -------
(From update of attachment 32231 [details])
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 From 2009-07-16 23:02:59 PST -------
(From update of attachment 32232 [details])
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 From 2009-07-16 23:03:15 PST -------
(From update of attachment 32234 [details])
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 From 2009-07-16 23:03:19 PST -------
All reviewed patches landed, closing.
------- Comment #19 From 2009-07-16 23:03:52 PST -------
Again, bugzilla-tool is too eager about closing bugs.  :)
------- Comment #20 From 2009-07-18 17:14:53 PST -------
(From update of attachment 32858 [details])
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 From 2009-07-19 01:21:47 PST -------
Created an attachment (id=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 From 2009-07-28 02:44:59 PST -------
(From update of attachment 32860 [details])
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 From 2009-07-28 02:50:01 PST -------
(From update of attachment 33037 [details])
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 From 2009-07-28 03:03:06 PST -------
Created an attachment (id=33607) [details]
Patch to add ChromeClient for Haiku WebCore support.
------- Comment #25 From 2009-07-28 03:22:33 PST -------
Created an attachment (id=33608) [details]
Patch to add FrameLoaderClient for Haiku WebCore support.
------- Comment #26 From 2009-07-28 11:29:45 PST -------
(From update of attachment 33607 [details])
> +
> +        #if ENABLE(OFFLINE_WEB_APPLICATIONS)
> +        virtual void reachedMaxAppCacheSize(int64_t spaceNeeded);
> +        #endif

The #if (and #endif) shouldn't be indented.
------- Comment #27 From 2009-07-28 12:00:18 PST -------
Commited as http://trac.webkit.org/changeset/46493
and http://trac.webkit.org/changeset/46494