Closed Bug 1590102 Opened 5 years ago Closed 5 years ago

Implement Network.deleteCookies

Categories

(Remote Protocol :: CDP, task, P1)

task

Tracking

(firefox74 fixed)

RESOLVED FIXED
Firefox 74
Tracking Status
firefox74 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Depends on 1 open bug, Blocks 1 open bug, )

Details

(Whiteboard: [puppeteer-beta-mvp])

Attachments

(1 file)

For Puppeteer support it is necessary to get the Network.deleteCookies domain implemented. It's specification can be found at:

https://chromedevtools.github.io/devtools-protocol/tot/Network#method-deleteCookies

The implementation can be similar to what we currently have in Marionette:

https://searchfox.org/mozilla-central/rev/d7537a9cd2974efa45141d974e5fc7c727f1a78f/testing/marionette/cookie.js#200-207

Here the interface of the remove() cookie API:

https://searchfox.org/mozilla-central/rev/d7537a9cd2974efa45141d974e5fc7c727f1a78f/netwerk/cookie/nsICookieManager.idl#50-69

Note that some pre-processing might be necessary to distinguish between the url or host/path variations.

Blocks: 1602829

Greetings Henrik,

I would like to work on this but I saw that it shows "Blocks: 1602829".

And I have checked these cookie.js where to make changes and nslCookieManager.idl. I totally new to open source so I need your guidance. Help me!

I followed the instructions in this link https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction for setting up the environment, I selected Firefox for Desktop and make build was successful.

Please tell me if anything I missed or needed.

Flags: needinfo?(hskupin)

It's nearly fine. Instead of Firefox for Desktop please select the Firefox Desktop (Artifact) mode. It has way shorter build times and changes in the Remote Agent are directly reflected when Firefox gets restarted. Now please check https://firefox-source-docs.mozilla.org/remote/Testing.html in how to run the remote tests. Once that works you are ready to start.

For faster replies feel free to join us on IRC, and ask your questions there.

Flags: needinfo?(hskupin)
Depends on: 1603776
Priority: P3 → P2
Whiteboard: [lang=js] → [puppeteer-beta-mvp] [lang=js]

Hi Henrik,

I followed the instruction and setup environment for Firefox Desktop (Artifact), and build was successful. After that I tried running and it Firefox Browser opened up.

I have one doubt that, do I need to add deleteCookies() interface separately, or need to made changes in the remove() itself.
For now, I have done like this below:

Added this in netwerk/cookie/nslCookieMangaer.idl:
void deleteCookies(in ACString aName,
in AUTF8String aUrl,
in AUTF8String aDomain,
in AUTF8String aPath);

And did changes for cookie.remove() in testing/marionette/cookie.js:
cookie.remove = function(toDelete) {
cookie.manager.deleteCookies(
toDelete.name,
toDelete.url,
toDelete.domain,
toDelete.path,
);
};

I was thinking to use Regex to distinguish URL and domain. But while doing I got some conflict that, domain can be used in URL. And please tell me how to distinguish path than URL?
This is the Regex:
RegExp("(?:(?:https?|ftp|file)://|www.|ftp.)
(?:([-A-Z0-9+&@#/%=_|$?!:,.]*)
|[-A-Z0-9+&@#/%=
|$?!:,.])*(?:([-A-Z0-9+&@#/%=~|$?!:,.]*)
|[A-Z0-9+&@#/%=~_|$])", "ig");

And I tried to check the above two changes made. So I again build and run the, started successfully.
Next I tried to do testing following this https://firefox-source-docs.mozilla.org/remote/Testing.html
Whatever testing command I ran getting following error.
ERROR Log:

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

IOError: [Errno 2] No such file or directory: u'c:/mymozilla/mozilla-central/obj-x86_64-pc-mingw32\all-tests.pkl'

File "c:\mymozilla\mozilla-central\testing/mach_commands.py", line 365, in test
argv=extra_args, **kwargs)
File "c:\mymozilla\mozilla-central\python/mach\mach\registrar.py", line 152, in dispatch
return self._run_command_handler(handler, context=context, **kwargs)
File "c:\mymozilla\mozilla-central\python/mach\mach\registrar.py", line 109, in _run_command_handler
result = fn(**kwargs)
File "c:\mymozilla\mozilla-central\testing/mochitest/mach_commands.py", line 332, in run_mochitest_general
tests = mochitest.resolve_tests(test_paths, test_objects, cwd=self._mach_context.cwd)
File "c:\mymozilla\mozilla-central\testing/mochitest/mach_commands.py", line 96, in resolve_tests
tests = list(resolver.resolve_tests(paths=test_paths, cwd=cwd))
File "c:\mymozilla\mozilla-central\testing/mozbase/moztest\moztest\resolve.py", line 741, in resolve_tests
for test in self._resolve(**kwargs):
File "c:\mymozilla\mozilla-central\testing/mozbase/moztest\moztest\resolve.py", line 565, in _resolve
candidate_paths |= set(self.tests_by_path.keys())
File "c:\mymozilla\mozilla-central\testing/mozbase/moztest\moztest\resolve.py", line 508, in tests_by_path
for test in self.tests:
File "c:\mymozilla\mozilla-central\testing/mozbase/moztest\moztest\resolve.py", line 500, in tests
for test in self.load_tests():
File "c:\mymozilla\mozilla-central\testing/mozbase/moztest\moztest\resolve.py", line 391, in call
with open(all_tests, 'rb') as fh:

Please guide me, if I am missing any steps or I did wrong.

Thank You

Flags: needinfo?(hskupin)

Well, please do not make changes to any IDL or cpp file. All the files necessary to be modified can be found in comment 0. As such your patch has to only modify files under /remote/.

Maybe to give you a better sense I will upload a work in progress patch for bug 1590098 (get cookies) so that you can see which changes would be necessary for this bug. It will happen in the next couple of hours.

Flags: needinfo?(hskupin)

Abhishek, the patch is actually up as https://phabricator.services.mozilla.com/D57614.

Hi Henrik,

Sorry for the late response, I checked the mentioned patch by you, and I tried to implement the deleteCookies in a similar way, but I am facing difficulty while doing it. I think I need to understand Mozilla code working flow better, and how it is working.
If someone else is there ready to take this issue, please give it to him/her.
Please accept my apology if I wasted your time.

Thank You

Flags: needinfo?(hskupin)

No problem. If you think it is too difficult for you we should be able to find something simpler. Just let me know here or via IRC.

Flags: needinfo?(hskupin)

Hi Henrik,

Thanks! Please guide me for something simpler, I check many good-first-issue, but some are occupied, in-discussion to confirm, many already someone did, there are some for which I may need help. So please me to pick,

Thank you so much for your support.

Flags: needinfo?(hskupin)

Abhishek, would you be happy to work on bug 1590451? If yes then please add a comment over there. That one is definitely simpler for a starter bug.

Without this API we are currently failing 16 Puppeteer unit tests. Let me pick this up given that it's on our beta list anyway.

Assignee: nobody → hskupin
Mentor: hskupin
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [puppeteer-beta-mvp] [lang=js] → [puppeteer-beta-mvp]
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a752d438b017 [remote] Implement Network.deleteCookies. r=remote-protocol-reviewers,ato
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74
Component: CDP: Network → CDP
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: