Closed
Bug 874167
Opened 11 years ago
Closed 9 years ago
Use OOPP for Java on Windows (again)
Categories
(Core Graveyard :: Plug-ins, defect, P2)
Tracking
(e10sm7+, firefox42 fixed, firefox43 fixed)
RESOLVED
FIXED
mozilla43
People
(Reporter: benjamin, Assigned: jimm)
References
()
Details
Attachments
(5 files, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/x-review-board-request
|
jimm
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details |
(deleted),
text/x-review-board-request
|
jimm
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jimm
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jimm
:
review+
|
Details |
The only real reason we turned OOPP off on Windows/Java is because Java dialogs such as their security signing dialog don't spin a nested event loop and therefore crash after the hang timeout. But we are seeing a new increase in crashes caused by Java, and they appear unwilling to fix these without specific steps to reproduce. To protect Firefox, I'd like to propose: * switch OOPP back on for Java on Windows * When the hang monitor UI shows, don't even auto-timeout and kill Java; just keep the UI showing until the user explicitly kills Java or the plugin recovers * If the hang UI is disabled or fails for some reason, kill Java after the normal (45-second) timeout.
Comment 1•11 years ago
|
||
How about having QA do a test-run with settings that produce matching behaviour now, so we see what scope this is? I think just setting dom.ipc.plugins.hangUITimeoutSecs to a very high value should do it?
Priority: -- → P2
Comment 2•11 years ago
|
||
Aaron, do you think setting hangUITimeoutSecs models the proposed changes sufficiently for testing?
Assignee: nobody → georg.fritzsche
Flags: needinfo?(aklotz)
Comment 3•11 years ago
|
||
To model the proposed changes, you would need to set the value of dom.ipc.plugins.timeoutSecs very high, as that pref still governs auto-timeout even when the hang UI is enabled.
Flags: needinfo?(aklotz)
Comment 4•11 years ago
|
||
Ah, thanks Aaron. re qawanted: Could we have some test-runs on Windows with the common test-applets to see if that causes any major issues? You'd need to set * dom.ipc.plugins.java.enabled to true * dom.ipc.plugins.timeoutSecs to a high value (say 3600)
Keywords: qawanted
Comment 5•11 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #4) > You'd need to set > * dom.ipc.plugins.java.enabled to true > * dom.ipc.plugins.timeoutSecs to a high value (say 3600) Firefox freezes running several applets: http://www.walter-fendt.de/ph14e/ nightly 2013-05-22 Win 7 x64 Everything looked ok with the default settings.
Keywords: qawanted
Updated•10 years ago
|
Flags: firefox-backlog?
Updated•10 years ago
|
Flags: firefox-backlog? → firefox-backlog+
Whiteboard: p=0
Comment 6•10 years ago
|
||
@jimm - this is the bug we talked about for java OOP
Assignee | ||
Updated•10 years ago
|
tracking-e10s:
--- → ?
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Blocks: e10s-plugins
Comment 7•10 years ago
|
||
Unassigning for now to reflect the current status, but added to our prioritization list.
Assignee: georg.fritzsche → nobody
Assignee | ||
Comment 8•10 years ago
|
||
Does anyone have any good old bugs related to this with test cases? afaict java should be running out of process now on windows with e10s.
Updated•10 years ago
|
Assignee | ||
Comment 13•10 years ago
|
||
We need to test java way before m7, so lets get this turned back on for e10s on Windows.
Assignee: nobody → jmathies
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Attachment #8535618 -
Flags: review?(wmccloskey)
Comment on attachment 8535618 [details] [diff] [review] pref patch Review of attachment 8535618 [details] [diff] [review]: ----------------------------------------------------------------- r+ for the changes to nsNPAPIPlugin.cpp. It looks like everything else was for debugging?
Attachment #8535618 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8535618 [details] [diff] [review] pref patch oops, sorry
Attachment #8535618 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
Assignee | ||
Comment 17•10 years ago
|
||
- whitespace cleanup
Attachment #8535707 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 18•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4c74ff43913e
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/38116e28a7b8
Keywords: checkin-needed
Comment 20•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/38116e28a7b8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•10 years ago
|
Iteration: --- → 37.2
Flags: qe-verify?
Whiteboard: p=0
Comment 21•10 years ago
|
||
Was the infinite timeout implemented? Will the security signing dialog crash after 45-sec?
Flags: needinfo?(jmathies)
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #21) > Was the infinite timeout implemented? Not currently, the hang dialog is currently disabled (bug 1096664). > Will the security signing dialog crash > after 45-sec? My hope would be that it doesn't lock up with the current configuration. I've been testing with the little java I can find on the web and haven't run into any issues.
Flags: needinfo?(jmathies)
Reporter | ||
Comment 23•9 years ago
|
||
Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, r=jimm
Attachment #8648099 -
Flags: review?(jmathies)
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8648099 [details] MozReview Request: Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, https://reviewboard.mozilla.org/r/16111/#review14447 Ship It!
Attachment #8648099 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 25•9 years ago
|
||
I do see an occasional java hang but they get caught by the e10s hang monitor. I haven't seen a chrome hang yet, that might change once we get out to beta.
Reporter | ||
Comment 26•9 years ago
|
||
Comment on attachment 8648099 [details] MozReview Request: Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, r=jimm
Attachment #8648099 -
Attachment description: , → , r=jimm
Reporter | ||
Comment 27•9 years ago
|
||
Bug 1090864 part A - remove nsNPAPIPlugin::RunPluginOOP and unused code paths, r=jimm
Attachment #8648248 -
Flags: review?(jmathies)
Reporter | ||
Comment 28•9 years ago
|
||
Bug 1098064 part B - remove nsIPluginHost.isPluginOOP, r?jimm
Attachment #8648249 -
Flags: review?(jmathies)
Reporter | ||
Comment 29•9 years ago
|
||
Bug 1098064 part C - remove SimpleTest and reftest testPluginIsOOP and related usage, r=jimm
Attachment #8648250 -
Flags: review?(jmathies)
Assignee | ||
Comment 30•9 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #25) > I do see an occasional java hang but they get caught by the e10s hang > monitor. I haven't seen a chrome hang yet, that might change once we get out > to beta. example: https://crash-stats.mozilla.com/report/index/7ef03c41-addb-481c-b00b-0344a2150815
Assignee | ||
Comment 31•9 years ago
|
||
Comment on attachment 8648248 [details] MozReview Request: Bug 1090864 part A - remove nsNPAPIPlugin::RunPluginOOP and unused code paths, r=jimm https://reviewboard.mozilla.org/r/16145/#review14499 Ship It! ::: dom/plugins/base/nsNPAPIPlugin.cpp (Diff revision 1) > - useA11yPref = a11y::Compatibility::IsJAWS(); We might want to flag dbolter on this. Looks like we disable OOP still on XP when we detect jaws in out process.
Attachment #8648248 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 32•9 years ago
|
||
Comment on attachment 8648249 [details] MozReview Request: Bug 1098064 part B - remove nsIPluginHost.isPluginOOP, r?jimm https://reviewboard.mozilla.org/r/16147/#review14501 Ship It!
Attachment #8648249 -
Flags: review?(jmathies) → review+
Assignee | ||
Comment 33•9 years ago
|
||
Comment on attachment 8648250 [details] MozReview Request: Bug 1098064 part C - remove SimpleTest and reftest testPluginIsOOP and related usage, r=jimm https://reviewboard.mozilla.org/r/16149/#review14503 Ship It!
Attachment #8648250 -
Flags: review?(jmathies) → review+
Reporter | ||
Updated•9 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 34•9 years ago
|
||
(In reply to Jim Mathies [:jimm] from comment #31) > Comment on attachment 8648248 [details] > MozReview Request: Bug 1090864 part A - remove nsNPAPIPlugin::RunPluginOOP > and unused code paths, r=jimm > > https://reviewboard.mozilla.org/r/16145/#review14499 > > Ship It! > > ::: dom/plugins/base/nsNPAPIPlugin.cpp > (Diff revision 1) > > - useA11yPref = a11y::Compatibility::IsJAWS(); > > We might want to flag dbolter on this. Looks like we disable OOP still on XP > when we detect jaws in out process. We're not (yet) sure how much to worry about this potential intersection.
Comment 36•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/103d2a9af2ff These 3 commits landed with bug numbers that don't seem to fit. Please comment/resolve the appropriate bugs for them (and probably add notes to the other bugs noting that commits landed that were misattributed). https://hg.mozilla.org/mozilla-central/rev/32e5b68506f4 https://hg.mozilla.org/mozilla-central/rev/e4c9fd404528 https://hg.mozilla.org/mozilla-central/rev/4e8302a104fb
Status: REOPENED → RESOLVED
Closed: 10 years ago → 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: mozilla37 → mozilla43
Depends on: 1196115
Reporter | ||
Updated•9 years ago
|
Summary: Use OOPP for Java on Windows (again), with infinite plugin timeout if the hang UI shows → Use OOPP for Java on Windows (again)
Looks like the commits in comment 36 landed in bug 1194780.
Reporter | ||
Comment 38•9 years ago
|
||
Comment on attachment 8648099 [details] MozReview Request: Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, Approval Request Comment [Feature/regressing bug #]: Longstanding Java issue [User impact if declined]: Java crashes will take down the entire browser. This is more urgent because of more rigorous threading checks in Firefox causing more crashes. [Describe test coverage new/current, TreeHerder]: Limited manual testing. [Risks and why]: This will cause a regression for Java applets which show various modal dialogs (including certificate dialogs). We've decided to accept this regression to solve the larger issue. [String/UUID change made/needed]: None
Attachment #8648099 -
Attachment description: MozReview Request: Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, → MozReview Request: Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point,
Attachment #8648099 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox42:
--- → affected
Comment 39•9 years ago
|
||
Comment on attachment 8648099 [details] MozReview Request: Bug 874167 - Move Java to be out-of-process like other plugins, so that it's weird threading behavior doesn't cause Firefox instability. This will likely cause reoccurance of bug 555699 and that is an acceptable tradeoff at this point, OK, let's take it in 42 then.
Attachment #8648099 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•