Remove appId from origin attributes

RESOLVED FIXED in Firefox 68

Status

()

defect
RESOLVED FIXED
3 years ago
Last month

People

(Reporter: Ehsan, Assigned: baku)

Tracking

(Blocks 2 bugs)

unspecified
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(4 attachments)

Reporter

Description

3 years ago
There is now only one real consumer for OriginAttributes::mAppId, which we can port to use firstPartyDomain (see bug 1320402).  After that, I think we can remove mAppId and shrink down OriginAttributes even more.

Kris, is this something that interests you?  :-)
Flags: needinfo?(kmaglione+bmo)
Sure, why not.
Assignee: nobody → kmaglione+bmo
Flags: needinfo?(kmaglione+bmo)
Reporter

Comment 2

3 years ago
\o/ Thank you!

Updated

2 years ago
Blocks: 1339081

Updated

2 years ago
No longer blocks: 1339081
No longer blocks: 1369194
Adding ddn, just to check the docs for this.
Keywords: dev-doc-needed
Kris, I hope you don't mind if I take this.
Assignee: kmaglione+bmo → valentin.gosu

Comment 5

10 months ago
:valentine can I take this?
Flags: needinfo?(valentin.gosu)
Sure!
Assignee: valentin.gosu → 1991manish.kumar
Flags: needinfo?(valentin.gosu)

Comment 7

10 months ago
seems OriginAttributes::mAppId is not available anywhere!
https://searchfox.org/mozilla-central/search?q=OriginAttributes%3A%3AmAppId&path=
Flags: needinfo?(ehsan)
(In reply to Manish [:manishkk] from comment #7)
> seems OriginAttributes::mAppId is not available anywhere!
> https://searchfox.org/mozilla-central/
> search?q=OriginAttributes%3A%3AmAppId&path=

https://searchfox.org/mozilla-central/rev/05d91d3e02a0780f44599371005591d7988e2809/dom/chrome-webidl/ChromeUtils.webidl#409,416

Comment 9

10 months ago
only need to remove these 2 lines?

> https://searchfox.org/mozilla-central/rev/
> 05d91d3e02a0780f44599371005591d7988e2809/dom/chrome-webidl/ChromeUtils.
> webidl#409,416
Flags: needinfo?(ehsan) → needinfo?(valentin.gosu)
(In reply to Manish [:manishkk] from comment #9)
> only need to remove these 2 lines?
> 
> > https://searchfox.org/mozilla-central/rev/
> > 05d91d3e02a0780f44599371005591d7988e2809/dom/chrome-webidl/ChromeUtils.
> > webidl#409,416

No, there are quite a lot of other references that need to be removed as well:

https://searchfox.org/mozilla-central/search?q=mAppId&case=true&path=
Flags: needinfo?(valentin.gosu)
I'll note, though, that we need to decide what to do about appId in the origin suffix parser:

https://searchfox.org/mozilla-central/source/caps/OriginAttributes.cpp#209

When we removed the addonId origin attribute, we decided to just ignore that parameter when parsing origin attributes, since it's automatically generated from the URL, anyway.

Any remaining references to principals with an appId in their origin attribute would likely cause problems if we decided to treat them as the same origin with the appId stripped. Which means we'll probably have to fail to parse in that case.

I'm not entirely sure what side-effects that will have in all possible cases...

Updated

6 months ago
Assignee: 1991manish.kumar → nobody
Reporter

Updated

2 months ago
See Also: → 1547993
Assignee

Updated

2 months ago
Assignee: nobody → amarchesini

Comment 17

2 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5a20b5f43280
Remove appId from origin attributes - part 1 - OriginAttributes and nsIPrincipal, r=Ehsan,flod
https://hg.mozilla.org/integration/autoland/rev/557b586f774a
Remove appId from origin attributes - part 2 - NO_APP_ID UNKNOWN_APP_ID, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/fed7c475d75c
Remove appId from origin attributes - part 3 - Permissionmanager, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/fbacf18b6532
Remove appId from origin attributes - part 4 - necko, r=valentin

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&searchStr=xperf&revision=fbacf18b653259954711b20fcefad7c8a82ce2b1&selectedJob=244071797

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244071797&repo=autoland&lineNumber=1678
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244052943&repo=autoland&lineNumber=5112

Backout link: https://hg.mozilla.org/integration/autoland/rev/ad04ccedc21ed4373acff2d310388bc55725182b

01:37:29 INFO - Merged Etl: test.etl
01:39:54 INFO - reading etl filename: test.etl
01:39:54 INFO - etlparser: in readfile: test.etl.csv
01:39:54 INFO - TEST-UNEXPECTED-FAIL : xperf: File '{profile}\permissions.sqlite-journal' (normalized from 'C:\Users\task_1556757239\AppData\Local\Temp\tmps8ccdp\profile\permissions.sqlite-journal') was accessed and we were not expecting it. DiskReadCount: 0, DiskWriteCount: 18, DiskReadBytes: 0, DiskWriteBytes: 7208
01:39:56 INFO - extending with xperf!
01:39:56 INFO - Detected a regression for tp5n
01:39:56 INFO - TEST-UNEXPECTED-FAIL | tp5n | Talos has found a regression, if you have questions ask for help in irc on #perf
01:39:56 ERROR - Traceback (most recent call last):
01:39:56 INFO - File "Z:\task_1556757239\build\tests\talos\talos\run_tests.py", line 300, in run_tests
01:39:56 INFO - talos_results.add(mytest.runTest(browser_config, test))
01:39:56 INFO - File "Z:\task_1556757239\build\tests\talos\talos\ttest.py", line 64, in runTest
01:39:56 INFO - return self._runTest(browser_config, test_config, setup)
01:39:56 INFO - File "Z:\task_1556757239\build\tests\talos\talos\ttest.py", line 268, in _runTest
01:39:56 INFO - 'Talos has found a regression, if you have questions'
01:39:56 INFO - TalosRegression: Talos has found a regression, if you have questions ask for help in irc on #perf
01:39:56 INFO - TEST-INFO took 289967ms
01:39:56 INFO - SUITE-END | took 289s
01:39:56 ERROR - Return code: 1
01:39:56 WARNING - setting return code to 1

Flags: needinfo?(amarchesini)
Assignee

Updated

2 months ago
Flags: needinfo?(amarchesini)

Comment 19

2 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/de9073c57d20
Remove appId from origin attributes - part 1 - OriginAttributes and nsIPrincipal, r=Ehsan,flod
https://hg.mozilla.org/integration/autoland/rev/dd741b25a244
Remove appId from origin attributes - part 2 - NO_APP_ID UNKNOWN_APP_ID, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/a7e7c0251179
Remove appId from origin attributes - part 3 - Permissionmanager, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/7c2f4e64d38e
Remove appId from origin attributes - part 4 - necko, r=valentin

Backed out 4 changesets (Bug 1320404) for test_permmanager_load_invalid_entries.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2Cx64%2Casan%2Cxpcshell%2Ctests%2Ctest-linux64-asan%2Fopt-xpcshell-e10s-3%2Cx%28x3%29&fromchange=0ef6e3e9552df27c04a2e839d686aaa45ef094e2&tochange=04557fa70ce8a1dd168482b42b734647753c5b70&selectedJob=244393680

Backout link: https://hg.mozilla.org/integration/autoland/rev/04557fa70ce8a1dd168482b42b734647753c5b70

Failure link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244393680&repo=autoland&lineNumber=2104

[task 2019-05-03T03:09:50.274Z] 03:09:50 INFO - TEST-START | dom/push/test/xpcshell/test_clearAll_successful.js
[task 2019-05-03T03:09:52.113Z] 03:09:52 INFO - TEST-PASS | dom/push/test/xpcshell/test_clearAll_successful.js | took 1843ms
[task 2019-05-03T03:09:52.117Z] 03:09:52 INFO - Retrying tests that failed when run in parallel.
[task 2019-05-03T03:09:52.125Z] 03:09:52 INFO - TEST-START | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js
[task 2019-05-03T03:09:52.881Z] 03:09:52 WARNING - TEST-UNEXPECTED-FAIL | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js | xpcshell return code: 0
[task 2019-05-03T03:09:52.882Z] 03:09:52 INFO - TEST-INFO took 754ms
[task 2019-05-03T03:09:52.883Z] 03:09:52 INFO - >>>>>>>
[task 2019-05-03T03:09:52.884Z] 03:09:52 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2019-05-03T03:09:52.885Z] 03:09:52 INFO - TEST-PASS | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js | run_test - [run_test : 19] true == true
[task 2019-05-03T03:09:52.887Z] 03:09:52 WARNING - TEST-UNEXPECTED-FAIL | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js | run_test - [run_test : 124] 10 == 9
[task 2019-05-03T03:09:52.888Z] 03:09:52 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js:run_test:124
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - /builds/worker/workspace/build/tests/xpcshell/head.js:_execute_test:523
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - -e:null:1
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - exiting test
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - <<<<<<<
[task 2019-05-03T03:09:52.891Z] 03:09:52 INFO - INFO | Result summary:
[task 2019-05-03T03:09:52.892Z] 03:09:52 INFO - INFO | Passed: 382
[task 2019-05-03T03:09:52.893Z] 03:09:52 WARNING - INFO | Failed: 1
[task 2019-05-03T03:09:52.893Z] 03:09:52 WARNING - One or more unittests failed.
[task 2019-05-03T03:09:52.894Z] 03:09:52 INFO - INFO | Todo: 0
[task 2019-05-03T03:09:52.895Z] 03:09:52 INFO - INFO | Retried: 1
[task 2019-05-03T03:09:52.895Z] 03:09:52 INFO - SUITE-END | took 230s
[task 2019-05-03T03:09:52.900Z] 03:09:52 INFO - Node moz-http2 server shutting down ...
[task 2019-05-03T03:09:52.973Z] 03:09:52 ERROR - Return code: 1
[task 2019-05-03T03:09:52.975Z] 03:09:52 INFO - TinderboxPrint: xpcshell-xpcshell<br/>382/<em class="testfail">1</em>/0
[task 2019-05-03T03:09:52.976Z] 03:09:52 WARNING - # TBPL FAILURE #
[task 2019-05-03T03:09:52.978Z] 03:09:52 WARNING - setting return code to 2
[task 2019-05-03T03:09:52.980Z] 03:09:52 WARNING - The xpcshell suite: xpcshell ran with return status: FAILURE

Flags: needinfo?(amarchesini)
Assignee

Updated

2 months ago
Flags: needinfo?(amarchesini)

Comment 21

2 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ef64a5380eb
Remove appId from origin attributes - part 1 - OriginAttributes and nsIPrincipal, r=Ehsan,flod
https://hg.mozilla.org/integration/autoland/rev/34023e7e4908
Remove appId from origin attributes - part 2 - NO_APP_ID UNKNOWN_APP_ID, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/b0f584907e03
Remove appId from origin attributes - part 3 - Permissionmanager, r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/e720637a07b8
Remove appId from origin attributes - part 4 - necko, r=valentin
Regressions: 1549912

Updated

Last month
Regressions: 1550004
You need to log in before you can comment on or make changes to this bug.