Removes NPAPI support from JS plugin actors, most of which was CTP-and-fallback overlay related, leaving them to only do GMP crash handling.
Details
Details
Diff Detail
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Lint Not Applicable - Unit
Tests Not Applicable - Build Status
Buildable 308097 Build 401748: arc lint + arc unit
Event Timeline
Comment Actions
Code analysis found 13 defects in the diff 408004:
- 13 build errors found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.
You have touched the documentation in diff 408004, you can find it rendered here for a week.
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
Comment Actions
Nice, but we can remove even more dead stuff!
| browser/actors/ContextMenuChild.jsm | ||
|---|---|---|
| 1124 | Nit: keep the separating blank line here please | |
| browser/actors/PluginParent.jsm | ||
| 29–30 | crashReports is unused now - we only write to it. | |
| 30–31 | this.crashReports is dead code now. this.gmpCrashes is just a map with some plain JS objects in it. We don't need to clean it up on shutdown, the JS engine will take care of that. We used to have to run ensureInitialized to make sure we got notified if the plugin list changed, and then needed to clean up the observer later. But we don't need that anymore. So instead, I think we can just nuke both ensureInitialized and destroy and remove the profile-after-change handling in the observer code. | |
| browser/base/content/browser-context.inc | ||
| 217–223 ↗ | (On Diff #408004) | Note that you might conflict with all the proton-related changes we're making to the context menu here. Sorry. If you want you could split off the removal of these 3 items and the parent process nsContextMenu.js code that touches them to avoid having it interfere with your patch stack. Up to you. |
| browser/components/BrowserGlue.jsm | ||
| 617–618 | Does this still fire? I can't see anything creating it, even in current m-c without all these patches. Looks like maybe https://bugzilla.mozilla.org/show_bug.cgi?id=1687239 removed it and forgot to remove these bits? | |
| xpcom/ds/StaticAtoms.py | ||
| 1918–1936 | This probably belongs in a different patch - the frontend code doesn't depend on atoms. | |
| browser/actors/PluginParent.jsm | ||
|---|---|---|
| 30–31 | Gone. | |
| browser/base/content/browser-context.inc | ||
| 217–223 ↗ | (On Diff #408004) | Are you just saying that we need to leave this code in browser-context.inc untouched? I'll leave this if that is helpful but I wouldn't want to try to resurrect the gContextMenu functions -- at most I'd stub them to no-ops. They can be removed anytime. Is that all you are looking for? |
| browser/components/BrowserGlue.jsm | ||
| 617–618 | Nope, not needed. Removed. | |
| xpcom/ds/StaticAtoms.py | ||
| 1918–1936 | They seem to have gone in as part of a mass event-optimization: Bug 1542885 - Make some event related atoms static. | |
Comment Actions
Code analysis found 13 defects in the diff 414011:
- 13 build errors found by clang-tidy
You can run this analysis locally with:
- ./mach static-analysis check --outgoing (C/C++)
The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.
You have touched the documentation in diff 414011, you can find it rendered here for a week.
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
| browser/base/content/browser-context.inc | ||
|---|---|---|
| 217–223 ↗ | (On Diff #408004) | Sorry for the confusion. All I meant was, you're probably going to hit conflicts and this is a massive patch stack, so I was trying to make it easier for you and appear to have inadvertently done the opposite... It should be relatively straightforward to have a separate patch *just* to make the changes to browser-context.inc and nsContextMenu.js, and nothing else, and land them immediately, without having to rebase your patches around all the proton changes all the time. In the intervening time, I think we're 99% done with context menu structural changes for Proton now so you could also put it back in here. Apologies again for the confusion. :-( |
Comment Actions
Code analysis found 15 defects in the diff 421892:
- 13 build errors found by clang-tidy
- 2 defects found by private static analysis
You can run this analysis locally with:
- For private static analysis, please see our private docs in Mana, if you cannot access this resource, ask your reviewer to help you resolve the issue.
- ./mach static-analysis check --outgoing (C/C++)
The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.
You have touched the documentation in diff 421892, you can find it rendered here for a week.
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
Comment Actions
Code analysis found 15 defects in the diff 422362:
- 13 build errors found by clang-tidy
- 2 defects found by private static analysis
You can run this analysis locally with:
- For private static analysis, please see our private docs in Mana, if you cannot access this resource, ask your reviewer to help you resolve the issue.
- ./mach static-analysis check --outgoing (C/C++)
The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.
The analysis task source-test-clang-format failed, but we could not detect any issue.
Please check this task manually.
You have touched the documentation in diff 422362, you can find it rendered here for a week.
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
Comment Actions
Code analysis found 3 defects in the diff 422931:
- 1 build error found by clang-tidy
- 2 defects found by private static analysis
You can run this analysis locally with:
- For private static analysis, please see our private docs in Mana, if you cannot access this resource, ask your reviewer to help you resolve the issue.
- ./mach static-analysis check --outgoing (C/C++)
The analysis task source-test-mozlint-l10n-conflicts failed, but we could not detect any issue.
Please check this task manually.
You have touched the documentation in diff 422931, you can find it rendered here for a week.
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.
Comment Actions
Code analysis found 3 defects in the diff 423042:
- 1 build error found by clang-tidy
- 2 defects found by private static analysis
You can run this analysis locally with:
- For private static analysis, please see our private docs in Mana, if you cannot access this resource, ask your reviewer to help you resolve the issue.
- ./mach static-analysis check --outgoing (C/C++)
The analysis task source-test-coverity-coverity failed, but we could not detect any issue.
Please check this task manually.
The analysis task source-test-mozlint-codespell failed, but we could not detect any issue.
Please check this task manually.
The analysis task source-test-mozlint-yaml failed, but we could not detect any issue.
Please check this task manually.
The analysis task source-test-doc-upload failed, but we could not detect any issue.
Please check this task manually.
The analysis task source-test-mozlint-test-manifest failed, but we could not detect any issue.
Please check this task manually.
If you see a problem in this automated review, please report it here.
You can view these defects on the code-review frontend and on Treeherder.