search

electron / libchromiumcontent

Shared library build of Chromium’s Content module
MIT License
485 stars 183 forks source link

feat: Add patch for the login delegate #608

Closed nitsakh closed 6 years ago

nitsakh commented 6 years ago

Pass in the referrer and the request method arguments to fix the login handler api n electron.

nornagon commented 6 years ago

I suspect that this will be a somewhat difficult patch to maintain. The code is already different in M67. Is there any chance we could either: a) get the information we need some other way that's less intrusive, b) upstream this or something like it into Chromium itself, or c) change the Electron API to not require this extra info?

deepak1556 commented 6 years ago

get the information we need some other way that's less intrusive,

Originally the plan was to rely on the global unique request id and extract request details based on it, but content layer doesn't provide with such utility, hence it was decided to add this patch. But upon further investigation, we might be able to achieve this on electron without the patch. Currently we setup the login delegate which is used by resource loader to handle request auth but even before the resource loader gets a chance the network layer allows network delegate to handle it but currently we deny handling it there, if we move the login handler to it we will have all the required details. I will take a stab at it tomorrow.

nitsakh commented 6 years ago

@deepak1556 wasn't the plan to deprecate this API and now and remove it in the next version? So that the patch will have to stay only till the next version.

alexeykuzmin commented 6 years ago

@nitsakh Do you still need this patch? If yes, can you please rebase it onto the latest master?

deepak1556 commented 6 years ago

This has been solved in https://github.com/electron/electron/pull/14231 , closing this PR. Thanks @nitsakh !