Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed missing headers from Webcore for Webkit2 #8

Merged
merged 17 commits into from May 6, 2019

Conversation

@RAJAGOPALAN-GANGADHARAN
Copy link

commented Feb 10, 2019

Fixed some of the missing headers for the day
Next target is Webkit itself

along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
* Copyright (C) 2016 Haiku, Inc. All rights reserved.

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 10, 2019

Member

I'm curious about this file, where does it come from? Why is the copyright year 2016?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 11, 2019

Author

am sorry @pulkomandy i was trying to fix workqueue initially it had not implemented functions. But when i saw about workqueue i found it should be under web template framework as it is the one which handles threads and stuff. so got confused little bit 😅

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 11, 2019

Author

i will restore this part right now!

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 11, 2019

Member

Ah, yes, this was moved from WebKit to WTF. So this file can be deleted, then?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 11, 2019

Author

yeah i guess workqueue is handled by wtf so no need in webkit right?

@@ -131,3 +131,14 @@ add_definitions(
-DNETWORKPROCESSNAME=\"NetworkProcess\"
)

set(WebKit_FORWARDING_HEADERS_DIRECTORIES
shared/API/c

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 10, 2019

Member

Please don't leave empty lines here

@@ -41,7 +41,8 @@
#endif

#if PLATFORM(HAIKU)
#include "graphics/haiku/StillImageHaiku.h"
//#include "graphics/haiku/StillImageHaiku.h"

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 10, 2019

Member

You can remove this commented line

@stippi
Copy link

left a comment

The work queue implementation seems to be sound.

@@ -26,6 +26,7 @@

#pragma once

#include "AffineTransform.h"

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 15, 2019

Member

Usually I try to get this working without modifying the WebKit and WebCore headers.
My solution is to move WebKit and webcore includes to local includes in the CMakeLists (you can see how this is done in PlatformHaiku.cmake for WebKitLegacy).
This way, the include with quotes will include WebKit header, and the include with brackets will include Haiku headers

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 15, 2019

Author

@pulkomandy ur saying about local include directories?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 15, 2019

Author

@pulkomandy i did this because to avoid the forward declaration problem that was stopping the build!
If i understood correctly affinetransform.h is referring to haiku header? but i could only find a common header 😅

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 15, 2019

Member

In this case it is referring to the WebCore header, but if you do nothing special, #include "AffineTransform.h" will include the Haiku header instead, and the build will be confused.

@@ -71,6 +72,10 @@ class AffineTransform {
AffineTransform(const D2D1_MATRIX_3X2_F&);
#endif

#if PLATFORM(HAIKU)
AffineTransform(const BAffineTransform&);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 15, 2019

Member

I would prefer this was not here, or at least marked explicit to avoid accidentally converting between the two (it could be a performance problem). There are already functions to convert between the two types where needed.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 15, 2019

Author

@pulkomandy so including webkit headers would have BAffineTransform within?

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 15, 2019

Member

No, if you do this:

#include "AffineTransform.h" // get the WebCore header
#include <AffineTransform.h> // get the Haiku header

There are several other cases of similar header clashes in the codebase, which is the reason for introducing LOCAL_INCLUDE_DIRECTORIES in the CMakeLists. The same change can be applied in the CMakeLists for WebKit(2)

@@ -0,0 +1,86 @@
/*

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 15, 2019

Member

Why is this new file needed? Wasn't it moved elsewhere?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 15, 2019

Author

But this file is under performance tests but building required this file . So i moved it here.
🤔

@pulkomandy pulkomandy force-pushed the haiku:webkit2 branch from d7b0fc8 to 01e8fac Feb 19, 2019

@pulkomandy

This comment has been minimized.

Copy link
Member

commented Feb 19, 2019

Hi,

I have rebased your changes and pushed them to the webkit2 branch, with some fixes. I stopped at an issue I don't know how to solve, so your turn to investigate!

The changes I made should give you an idea how various problems can be fixed.

I think the CMakeLists.txt in WebKit/ is out of date, so maybe compare it with the current version in github.com/webkit/webkit and decide which changes are relevant, and which should be dropped. This also applies potentially to all files in the WebKit directory, if in doubt, make sure they are in sync with the versions in the upstream webkit repository.

I have included some extra notes about what I did in the commit message. Don't hesitate to rework your commits (using git rebase -i for example), so that the overall history makes sense. It is easier to follow if a commits does not reverse what was done in the previous one :)

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:webkit2 branch from 1da548e to 39d4719 Feb 23, 2019

@@ -31,6 +31,8 @@

#if defined(WIN32) || defined(_WIN32)
typedef int WKProcessID;
#elif PLATFORM(HAIKU)
typedef int32_t WKProcessID;

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 24, 2019

Member

This is not right, pid_t should be used for WKProcessID. Adjust the places where it is encoded/decoded if needed.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 27, 2019

Author

As far as i know WTF::ProcessID should be same as WKProcessID . We gave WTF::ProcessID as int32_t explicitly so there is ambuiguity rised in function definitions because WKProcessID is of pid_t.
So u think setting WKPRocessID to int32_t explicitly like we did for WTF::processid is viable solution?

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 27, 2019

Member

I think it would be better to keep pid_t and use the toPid/toProcessId functions I added to convert between the two types?

But if that does not work, yes, we will need this change. In this case I wonder why the code does not use WTF::ProcessID here and defines a different type...

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 27, 2019

Author

I was wondering the same 😕 . We need not use converting right as having int32_t is simple and viable solution will not introduce much merge conflicts what say?

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 27, 2019

Member

In this case we have found a real problem in WebKit code, and we will send them the change. So we can make more modifications, and once they have accepted the patch, there will be no merge conflict.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 27, 2019

Author

So can i right away explicitly convert to int32_t the WKProcessID?
I asked the webkit-dev still now there was no response so when they suggest something better we better adapt that

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 27, 2019

Member

They already replied to my mail previously and you asked the same question again. Surely they will find this boring and will not reply a second time.

As I said, I added the toPid/toProcessId functions so we can use them to convert between WKProcessID (pid_t) and WTF::ProcessID (int32_t) and back

@@ -31,6 +31,8 @@

#if defined(WIN32) || defined(_WIN32)
typedef int WKProcessID;
#elif PLATFORM(HAIKU)
typedef int32_t WKProcessID;

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 24, 2019

Member

same here (and why is this enum repeated anyways?)

@@ -2077,7 +2077,7 @@ WebsiteDataStoreParameters WebsiteDataStore::parameters()
SandboxExtension::createHandleForReadWriteDirectory(parameters.serviceWorkerRegistrationDirectory, parameters.serviceWorkerRegistrationDirectoryExtensionHandle);
#endif

platformSetNetworkParameters(parameters);
//platformSetNetworkParameters(parameters);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 24, 2019

Member

Please do not edit generic WebKit headers, unless they are out of sync with upstream webkit code because of an earlier merge problem.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 24, 2019

Author

@pulkomandy i was trying to debug thats why commented it . iforgot to revert it back 😕 sorry.

@@ -36,7 +36,9 @@
namespace WTF {

#if OS(WINDOWS)
using ProcessID = int;
using ProcessID = int;
#elif PLATFORM(HAIKU)

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 24, 2019

Author

@pulkomandy this is also not allowed? should i use pid_t and cast it to int32_t?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 24, 2019

Author

@pulkomandy alright it is also un conventional so let me fix by casting itself 😄

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Feb 24, 2019

Author

@pulkomandy am i allowed to add casting in generic file if so where can i add?

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 24, 2019

Member

In this case, we are forced to change generic files anyways. (ProcessID.h is also a generic file :))
So, let's add the cast, or maybe add specific calls to the decoders/encoders to encode/decode a pid_t. I don't know which solution is preferred, I will ask webkit-dev.

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Feb 24, 2019

Member

I have pushed a work in progress implementation which does what I think the WebKit developers want (but I'm not sure). I did not get the "decode" part working yet. I think you can work from that (unless they changed their mind while I was sleeping).

@pulkomandy pulkomandy force-pushed the haiku:webkit2 branch from 5916107 to 82a8d11 Feb 25, 2019

@pulkomandy

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

Hi,

Today I cleaned up your commits to not have one undoing another, removed some changes I think are incorrect (to AffineTransform, and introduction of CurrentTime.h in wtf which doesn't exist in upstream webkitm for example).

I have changed the ProcessID implementation again, I think this is what Sam Weining meant on the webkit mailing list, however the conversion macros are not used because implicit conversion will work.

Currently the build fails with:

../../Source/WebKit/NetworkProcess/CustomProtocols/LegacyCustomProtocolManager.h:92:32: error: 'CustomProtocol' has not been declared
uint64_t addCustomProtocol(CustomProtocol&&);

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:webkit2 branch from 6bb54d7 to d883420 Mar 3, 2019

@@ -0,0 +1,21 @@
/*
license

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Mar 7, 2019

Member

there is a standard license for webkit, please use it in all files you create:

https://trac.webkit.org/export/242591/webkit/trunk/Source/WebCore/LICENSE-APPLE

of course replace copyright years and assign copyright either to yourself or to Haiku, inc, not to Apple.

#include <pal/crypto/gcrypt/Initialization.h>
#endif
#endif*/

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 9, 2019

Author

This is the error i told that day the compiler couldnt identify USE maybe its because the source is happening in a seperate process?

@@ -227,6 +230,12 @@ bool WebFrameLoaderClient::shouldUseCredentialStorage(DocumentLoader*, unsigned
return webPage->injectedBundleResourceLoadClient().shouldUseCredentialStorage(*webPage, *m_frame, identifier);
}

bool WebFrameLoaderClient::dispatchDidReceiveInvalidCertificate(WebCore::DocumentLoader*, const WebCore::CertificateInfo&, const char* message)

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 9, 2019

Author

should i also add if directive to check if its haiku?

notImplemented();
return nullptr;
}

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 9, 2019

Author

I deleted it accidentally and moved it in back but its showing deleted 😞 .

{
notImplemented();
return false;
}

void ArgumentCoder<ResourceResponse>::encodePlatformData(ArgumentEncoder& encoder, const ResourceResponse& resourceResponse)
/*void ArgumentCoder<ResourceResponse>::encodePlatformData(Encoder& encoder, const ResourceResponse& resourceResponse)

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 9, 2019

Author

this one doesnt have a matching template in webcoderarguement. so shall i add a template for resourceresponse ? i suppose other platforms dont use resourceresponse so i commented it .

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 10, 2019

Author

There is a resourceError template i think it covers response? what say

{
//encoder << static_cast<uint32_t>(resourceResponse.soupMessageFlags());
}
bool ArgumentCoder<ResourceResponse>::decodePlatformData(ArgumentDecoder& decoder, ResourceResponse& resourceResponse)
bool ArgumentCoder<ResourceResponse>::decodePlatformData(Decoder& decoder, ResourceResponse& resourceResponse)
{

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN
static NeverDestroyed<NetworkProcess> networkProcess(WTFMove(parameters));
}

int NetworkProcessMainUnix(int argc, char** argv)

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Mar 13, 2019

Member

Why not use the existing NetworkProcessMainUnix? When the UNIX code works; there is no need to copy it, and copying it and changing copyright on it is not right (you should keep the original copyright when you make changes and add the haiku copyright as well)

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 13, 2019

Author

Only declaration is provided by Unix we have to define it for our own platform hence this

@@ -1,5 +1,5 @@
/*
* Copyright (C) 2014 Haiku, inc.
* Copyright 2019 Haiku, Inc.

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Mar 13, 2019

Member

2014-2019

@@ -32,8 +32,8 @@
#include "MIMETypeRegistry.h"
#include "NotImplemented.h"
#include "StillImageHaiku.h"
#include "JavaScriptCore/GenericTypedArrayViewInlines.h"
#include "JavaScriptCore/JSGenericTypedArrayView.h"
#include "JavaScriptCore/JSCInlines.h"

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Mar 13, 2019

Member

Have you checked that the changes in WebCore do not build the WebKitLegacy build?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 13, 2019

Author

Changing webcore doesnt build webkitlegacy? i thought all the linked files would be rebuilt again not sure let me build again and let you know 😄

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Mar 13, 2019

Member

you can build DumpRenderTree or HaikuLauncher (ninja HaikuLauncher) to test building just the webkitlegacy parts. Since WebKit2 build is broken the build may stop before or after building webkit...

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Mar 13, 2019

Author

This change is absolutely safe Pulkomandy 😄

@@ -1,5 +1,5 @@
/*
* Copyright (C) 2013 Intel Corporation. All rights reserved.

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Mar 13, 2019

Member

Previous copyright should be kept if the file was initially a copy of the work done for another platform.

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Author

commented on 4a7a146 Mar 17, 2019

Mini browser is again platform specific implementation it is just to have common name to test webkit (similar to Haiku launcher) so we will follow the convention I might as well use ui parts of Haiku launcher.

This comment has been minimized.

Copy link
Member

replied Mar 17, 2019

Yes, copy what you need (not that there is much to the ui anyways, it's very simple)

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Owner Author

commented on Tools/MiniBrowser/haiku/App.h in c9f0fd0 Apr 5, 2019

This is just done for testing purpose later once everything is working i will wrap them up in a usable class

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Owner Author

commented on Tools/MiniBrowser/haiku/App.cpp in c9f0fd0 Apr 5, 2019

later these will be wrapped in a usuable class

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Owner Author

commented on Tools/MiniBrowser/haiku/App.h in c9f0fd0 Apr 5, 2019

using retain pointers and page references for testing purpose later will configure page proxy's instead of using the reatin pointers (helps in the long run)

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Owner Author

commented on Tools/MiniBrowser/haiku/App.cpp in c9f0fd0 Apr 5, 2019

his is just done for testing purpose later once everything is working i will wrap them up in a usable class

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Owner Author

commented on Source/WebKit/UIProcess/WebPageProxy.cpp in c9f0fd0 Apr 5, 2019

added it for debugging will remove it later 😄

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

Copy link
Owner Author

commented on Source/WebKit/UIProcess/WebProcessPool.cpp in c9f0fd0 Apr 5, 2019

I will restore all the changes made to generic its only made for debugging purpose

@pulkomandy

This comment has been minimized.

Copy link
Member

commented on Tools/MiniBrowser/haiku/App.cpp in c9f0fd0 Apr 5, 2019

please use webkit style for headers in webkit, not haiku style.

This comment has been minimized.

Copy link
Author

replied Apr 5, 2019

Ok got it 👍

@pulkomandy

This comment has been minimized.

Copy link
Member

commented on Tools/MiniBrowser/haiku/BrowserWindow.cpp in c9f0fd0 Apr 5, 2019

?

This comment has been minimized.

Copy link
Author

replied Apr 5, 2019

This one will be done once after basic implementation is working 😅 (currently im working on a prototype of minibrowser trying to take one step at a time)

@pulkomandy

This comment has been minimized.

Copy link
Member

commented on c9f0fd0 Apr 5, 2019

Beware of identation, it is a bit mixed in our files but try to follow what WebKit does elsewhere. I think it should be 4 space or 1 tab per level, but I'm not sure. At least try to stick to one style per file or function.

@@ -100,7 +100,7 @@ void RunLoop::run()
looper->LockLooper();
looper->AddHandler(current().m_handler);
looper->UnlockLooper();

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

i will get the spacing and weird debug messages out as now my only aim is to get it rendering 😄

@@ -115,7 +115,8 @@ void RunLoop::stop()

void RunLoop::wakeUp()
{
m_handler->Looper()->PostMessage('loop', m_handler);
RunLoop::current().performWork();

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

Uh, this look completely wrong!
RunLoop::wakeUp is called from other threads to wake up the loop.
performWork should be run inside the main loop thread.

There is nothing to win in taking such shortcuts. You will only get things to crash in very strange ways.

Also, this probably breaks existing use of runloop in WebKitLegacy. It would be great if I could merge your changes in the main branch, but with changes like this, it's not going to work.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

Sorry just was curious what happens beyond this point 😢 but i knew it is going to break things i will find howto make it work.

@@ -96,7 +96,7 @@ list(APPEND WebCore_SOURCES
platform/graphics/texmap/GraphicsLayerTextureMapper.cpp
platform/graphics/texmap/TextureMapperImageBuffer.cpp

platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp
#platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

What's the problem with this file?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

This file does not do anything everything is wrapped under USE(COORDINATEDGRAPHICS) while we have disabled it so thought it would be a good clean up 😅😅

@@ -117,7 +117,7 @@ WTF::Optional<size_t> BFormDataIO::readFromFile(const FormDataElement::EncodedFi
m_fileHandle = FileSystem::openFile(fileData.filename, FileSystem::FileOpenMode::Read);

if (!FileSystem::isHandleValid(m_fileHandle)) {
LOG(Network, "Haiku - Failed while trying to open %s for upload\n", fileData.filename.utf8().data());
//LOG(Network, "Haiku - Failed while trying to open %s for upload\n", fileData.filename.utf8().data());

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

Why? Is it just a missing include to define the LOG macro?

{
fprintf(stderr,"its painting\n");
WebCore::Region unpainted;
BView* surface = new BView("drawing_surface",B_WILL_DRAW);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

BWebView is a BView, and it's the one you should draw on. Here you are creating a new BView every time this is called, which is a huge memory leak, and you are not going to draw anything onto the BWebView itself.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

Oops im still learning so please forgive me. I do understand that it is wrong now. Uh how can i miss it out I am supposed to use the existing view to draw but im just overlaying it over and over.

@@ -26,12 +26,83 @@
#include "config.h"
#include "ProcessLauncher.h"

#define __STDC_FORMAT_MACROS

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

Is this still needed?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

Yes to get uint64 into a string(as a format specifier) we need this

@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

What's this file?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

I think its like a configuration file created by webkit for mini browser

@@ -39,13 +39,15 @@ class WebProcessMain final : public AuxiliaryProcessMainBase {
public:
bool platformInitialize() override
{
BApplication* app = new BApplication("application/x-vnd.haiku-webkit.webprocess");

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

You really need a BApplication to do anything. Where is it created, if not here?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

The ui process is a bapplication right wont that suffice?

This comment has been minimized.

Copy link
@anevilyak

anevilyak Apr 30, 2019

Member

No. Without the BApplication, 1) the worker will have no link to the app_server for things like drawing ops, and 2) It won't be registered with the roster, and consequently you'd have no way of sending e.g. BMessages between it and the UI process.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

I see so that explains why my messages were not recieved

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN Apr 30, 2019

Author

so this will be my main run loop that gets all my messages right?

This comment has been minimized.

Copy link
@anevilyak

anevilyak Apr 30, 2019

Member

For inter-process communications, yes.

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy Apr 30, 2019

Member

Yes. I think the runloop implementation detects that it is in the main thread, and in that cases it will run the BApplication looper (using be_app->Run()). Otherwise (for various worker threads) it will create its own worker instead.

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

This loop is for messaging to webprocess app

@RAJAGOPALAN-GANGADHARAN

This comment has been minimized.

I will fix this in the next commit just was focussed on setting up the message loop to work

@pulkomandy

This comment has been minimized.

Copy link
Member

commented on 1e1c1ad May 2, 2019

Hi,
Can you explain why is there a WebViewBase and a WebView class?

Also, if WebView is going to be part of the public API, it should be outside the WebKit namespace, and named BWebView, as it was before. So that it goes well with the other parts of the API (BWindow, BView, etc).

This comment has been minimized.

Copy link
Author

replied May 2, 2019

WebViewBase is like internal WebView to do configuration and loading all abstracted from users. Whereas WebView will be like the public api indeed ( alright i will get it renamed to bwebview).
Oh ya sorry i will move it out of webkit namespace was doing it at night 1 so half sleep problem 😢 .

@pulkomandy pulkomandy merged commit b0584fd into haiku:webkit2 May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.