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

Patch sets after setting up a clean history #13

Merged
merged 3 commits into from May 18, 2019

Conversation

@RAJAGOPALAN-GANGADHARAN
Copy link

commented May 11, 2019

No description provided.

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch from b265414 to 981e253 May 11, 2019

Fixed invalid arguement and infinite loop error while launching with …
…BRoster

I guess the problem was it doesnt consider null as a good arguement just a marker so we need to pass i-1 as the arg count. This briefly
allows me to launch the webprocess app and i was able to see the debug messages on the terminal itself.
}
#endif

entry_ref processRef(BString path)

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 11, 2019

Member

This could be in ProcessLauncherHaiku, so you don't need to modify the .h file.

{
BEntry pathEntry(path);
if(!pathEntry.Exists())
ASSERT_NOT_REACHED();

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 11, 2019

Member

Indentation is incorrect. Also using assert is not very nice, could the function instead return an error code?

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch 2 times, most recently from 4ccc8c4 to ce47621 May 11, 2019

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch 2 times, most recently from 03e7494 to 1411e7c May 12, 2019

namespace WebKit {

SharedMemory::Handle::Handle()
{

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 12, 2019

Author

Since we disabled sockets we have to write this on our own

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

Yes, and we can use create_area and that family of functions to do it the native way

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 16, 2019

Author

Oh so no need to use Add data and use Add Pointers instead as i believe they use shared memory?

}
bool Connection::sendOutgoingMessage(std::unique_ptr<Encoder> encoder)
{
fprintf(stderr,"%s\n",__PRETTY_FUNCTION__);

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 12, 2019

Author

A BMessage can be created and encoder can be attached to it by adding parameters like encoder.buffer and encoder.size onto bmessage where we can decode from recieveing end

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 12, 2019

Member

Does Encoder encode the data to a flat buffer necessarily? I thought it would be able to natively fill a BMessage with the high level data (so that PrintToStream on the message does something meaningful, for example).

Well, either way works.

}
void Connection::runReadEventLoop()
{

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 12, 2019

Author

probably start a loop here and attach handler to look for messages

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 12, 2019

Member

We already have a BApplication running, so it will be a little more complex I guess. Unless these things are run as separate threads?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 13, 2019

Author

Hmm what if we just attach a handler to exisiting looper?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 13, 2019

Author

Ok sorry that doesnt make any sense 😅

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch from 1411e7c to ebecb1d May 14, 2019

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch from ebecb1d to 35f77da May 15, 2019

}
bool Connection::sendOutgoingMessage(std::unique_ptr<Encoder> encoder)
{
BMessage processMessage('ipcm');

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 15, 2019

Author

This one is incomplete the buffer is of type uint8_t* so i did add data im not sure if its right

@@ -39,7 +39,8 @@ void initializeAuxiliaryProcess<NetworkProcess>(AuxiliaryProcessInitializationPa

int NetworkProcessMainUnix(int argc, char** argv)
{
return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);
//return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 15, 2019

Author

for now focussing on webprocess so will probably create a template class for both webprocess and netwrokprocess BApplication

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

It would be better if we could use the standard AuxiliaryProcessMain here. Especially as this now returns immediately, whereas before it was blocking the execution, right?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 16, 2019

Author

Can you explain this me again I couldnt understand this 😕

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

I think AuxiliaryProcessMain is like a "main" function, it does not return until the program is done, and when it returns, the program exits.

So if we do nothing here, the program exits immediately, right? And that's not what we want.

Well, let's work a bit on AuxiliaryProcess and get back to this.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 16, 2019

Author

Oh ok got it so we could Run our Application inside AuxProcessMain so it exists only after its done

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN force-pushed the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch from 35f77da to 99b70f1 May 15, 2019

IPC for haiku
Initial implementation of IPC for haiku will add it to description each time i amend
1)added empty functions
2)fixed building issues and successfully created a connection between webprocess
3)Added a message handler to look out for incoming messages to runloop
4) Created new aux support so we can attach all handlers to the main apps looper
@@ -143,6 +148,9 @@ class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThrea
typedef HANDLE Identifier;
static bool createServerAndClientIdentifiers(Identifier& serverIdentifier, Identifier& clientIdentifier);
static bool identifierIsValid(Identifier identifier) { return !!identifier; }
#elif PLATFORM(HAIKU)
typedef team_id Identifier;
static bool identifierIsValid(Identifier identifier) { return BMessenger(NULL,identifier).IsValid(); }

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

Isn't ther a cheaper way? For example can we ask BRoster if the team_id exists and is a BApplication?

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

be_roster.GetRunningAppInfo sounds like it should work.

@@ -39,7 +39,8 @@ void initializeAuxiliaryProcess<NetworkProcess>(AuxiliaryProcessInitializationPa

int NetworkProcessMainUnix(int argc, char** argv)
{
return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);
//return AuxiliaryProcessMain<NetworkProcess, AuxiliaryProcessMainBase>(argc, argv);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

It would be better if we could use the standard AuxiliaryProcessMain here. Especially as this now returns immediately, whereas before it was blocking the execution, right?

@@ -398,6 +406,12 @@ class Connection : public ThreadSafeRefCounted<Connection, WTF::DestructionThrea
std::unique_ptr<Encoder> m_pendingWriteEncoder;
EventListener m_writeListener;
HANDLE m_connectionPipe { INVALID_HANDLE_VALUE };
#elif PLATFORM(HAIKU)
Identifier m_connectedProcess;

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

beware of indentation

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 16, 2019

Author

Whats the identation to be followed this is not how how it looks in Pe it looks rightly idented but here it looks differently

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

I would recommend Koder instead of Pe then! I think it can show tabs and spaces.
Unfortunately the coding style is a mixup of both, so try to stick to whatever is used in the file you're editing, for now.


}
void Connection::runReadEventLoop()
{

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

What does this do on other platforms? Does it start a new thread there?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 16, 2019

Author

No they get attached to the work queues thread. In our case we attach to application loop

BString processIdentifier;
BString processIdentifier,processID;
team_id UIProcessID = getpid();
processID.SetToFormat("%ld",UIProcessID);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

what's the difference between processID and processIdentifier? Can we use better naming?

@@ -77,15 +98,16 @@ void ProcessLauncher::launchProcess()
#endif
argv[i++] = executablePath.String();
argv[i++] = processIdentifier.String();
argv[i++] = processID.String();
// TODO pass our team_id so the web process can message us?
argv[i++] = nullptr;

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

We can remove this and the i-1 below.

@@ -90,6 +90,8 @@ void WebInspector::setFrontendConnection(IPC::Attachment encodedConnectionIdenti
IPC::Connection::Identifier connectionIdentifier(encodedConnectionIdentifier.port());
#elif OS(WINDOWS)
IPC::Connection::Identifier connectionIdentifier(encodedConnectionIdentifier.handle());
#elif PLATFORM(HAIKU)
IPC::Connection::Identifier connectionIdentifier(NULL);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

I think this can be properly implemented now?

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN
fprintf(stderr, "%s\n", __PRETTY_FUNCTION__);
RunLoop::initializeMainRunLoop();
RunLoop::run();
AuxiliaryProcessMain<WebProcess>(argc,argv);

This comment has been minimized.

Copy link
@pulkomandy

pulkomandy May 16, 2019

Member

This seems a bit backwards. I think AuxiliaryProcessMain should be in charge of creating a BApplication, if that's possible.

This comment has been minimized.

Copy link
@RAJAGOPALAN-GANGADHARAN

RAJAGOPALAN-GANGADHARAN May 16, 2019

Author

Yes AuxiliaryProcessMain creating will be much better as it will be easier to create a template BApllication so that we can use for both webprocess and network process

@pulkomandy pulkomandy merged commit 8cad8e3 into haiku:webkit2 May 18, 2019

@RAJAGOPALAN-GANGADHARAN RAJAGOPALAN-GANGADHARAN deleted the RAJAGOPALAN-GANGADHARAN:copywebkit2 branch Jun 29, 2019

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