Closed
Bug 1173665
Opened 9 years ago
Closed 9 years ago
[nested_oop] Some functions in ContentCache cannot be used in content process
Categories
(Core :: DOM: Content Processes, defect)
Core
DOM: Content Processes
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
firefox41 | --- | affected |
People
(Reporter: kershaw, Assigned: kershaw)
References
Details
Attachments
(1 file, 2 obsolete files)
35.73 KB,
patch
|
kershaw
:
review+
|
Details | Diff | Splinter Review |
Since TabParent may live in content process in nested oop case, some functions in ContentCache calling from TabParent, such as [1], are not worked. It causes that Selection can not get valid values and makes some tests failed due to [2]. [1] https://dxr.mozilla.org/mozilla-central/source/dom/ipc/TabParent.cpp#1995 [2] https://dxr.mozilla.org/mozilla-central/source/widget/ContentCache.h#191
Assignee | ||
Comment 1•9 years ago
|
||
Hi Masayuki, For the current time being, I think we just simply not to call the functions in ContentCache if the TabParent is in content process. May I know your feedback? Thanks.
Attachment #8620830 -
Flags: feedback?(masayuki)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → kechang
Comment 2•9 years ago
|
||
Wow, what's case? I assumed that TabParent is always created in chrome process. If it can be in content process and may be received its child PuppetWidget, it should work as chrome's.
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #2) > Wow, what's case? I assumed that TabParent is always created in chrome > process. If it can be in content process and may be received its child > PuppetWidget, it should work as chrome's. Please take a look at https://wiki.mozilla.org/Nested_Content_Processes. It is possible to create a nested oop iframe, so TabParent may be created in content process. I am working on bug 1139286 to make nested oop to be enabled on try. I've found a test failure [1] due to that the Selection has invalid values. I want to enable nested oop on try as soon as possible in order to protect this feature and also inform others that TabParent is possible to be created in content process. That's why I just want to simply return in IME related IPC functions in TabParent. Is it easy to modify ContentCache for this case? Which way is better? [1] https://treeherder.mozilla.org/logviewer.html#?job_id=12588&repo=jamun
Comment 4•9 years ago
|
||
Okay, then, I think that ContentCache's constructor should take a bool argument like aInParent. Then, mInChrome should be mInParent. And TabParent should do mContentCache(true), PuppetWidget should do mContentCache(false) in the initializing list.
Assignee | ||
Comment 5•9 years ago
|
||
Hi Masayuki, could you please take a look at this patch?
Attachment #8620830 -
Attachment is obsolete: true
Attachment #8620830 -
Flags: feedback?(masayuki)
Attachment #8622863 -
Flags: review?(masayuki)
Comment 6•9 years ago
|
||
Comment on attachment 8622863 [details] [diff] [review] Replace mInChrome with mInParent Great!
Attachment #8622863 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Carry reviewer's name. Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=996d920c5daa
Attachment #8622863 -
Attachment is obsolete: true
Attachment #8623448 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
Could you don't land the patch? I'll remove the variable in bug 1177388.
Depends on: 1177388
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #9) > Could you don't land the patch? I'll remove the variable in bug 1177388. Ok. So, I assume this problem will also be solved once bug 1177388 is fixed?
Comment 11•9 years ago
|
||
Exactlly.
Comment 12•9 years ago
|
||
Should be fixed. Please verify that.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•9 years ago
|
||
(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #12) > Should be fixed. Please verify that. I can confirm this is fixed. Thanks.
Comment 14•9 years ago
|
||
(In reply to Kershaw Chang [:kershaw] from comment #13) > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #12) > > Should be fixed. Please verify that. > > I can confirm this is fixed. Thanks. -> v.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•