Closed
Bug 1182535
Opened 9 years ago
Closed 7 years ago
[meta] [tracking bug] change all callsites to asyncOpen2 instead of asyncOpen
Categories
(Core :: DOM: Security, defect)
Core
DOM: Security
Tracking
()
RESOLVED
FIXED
People
(Reporter: ckerschb, Assigned: ckerschb)
References
(Depends on 2 open bugs, Blocks 3 open bugs)
Details
(Keywords: meta, Whiteboard: [domsecurity-meta])
Performing a dxr search reveals we have to convert: C++: 87 callsites JS: 340 callsites to use asyncOpen2.
Assignee | ||
Updated•9 years ago
|
Comment 1•9 years ago
|
||
Hi, I refreshed the local C-C thunderbird source repository from the net, and noticed a few link time errors regarding Open2 and AsyncOpen2: /home/ishikawa/objdir-tb3/toolkit/library/../../mailnews/base/util/nsMsgProtocol.o:nsMsgProtocol.cpp:vtable for nsMsgProtocol: error: undefined reference to 'nsMsgProtocol::Open2(nsIInputStream**)' /home/ishikawa/objdir-tb3/toolkit/library/../../mailnews/base/util/nsMsgProtocol.o:nsMsgProtocol.cpp:vtable for nsMsgProtocol: error: undefined reference to 'nsMsgProtocol::AsyncOpen2(nsIStreamListener*)' I wonder if these errors are related to this bugzilla entries. Do we need to replace the calls to AsyncOpen2() with something else? TIA
Comment 2•9 years ago
|
||
Oops sorry. Searching for "Open2" instead of "AsyncOpen2", I noticed there is already a bug opened for the issue. Bug 1185583 Thunderbird broken by mozilla-central Bug 1143922 nsPop3Protocol.obj : error LNK2001: unresolved external symbol "public: virtual enum nsresult __stdcall nsMsgProtocol::Open2(class nsIInputStream * *)" ... I will monitor that bug.
Hey Christoph, do you think if I chipped in on some of these conversions it would help in moving ServiceWorkers forward? Anything I should keep in mind when making changes other than keeping the tree green?
Flags: needinfo?(mozilla)
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Nikhil Marathe [:nsm] (please needinfo?) from comment #3) > Hey Christoph, do you think if I chipped in on some of these conversions it > would help in moving ServiceWorkers forward? Anything I should keep in mind > when making changes other than keeping the tree green? Well, the faster we get all 400+ callsites (C++ and JS) converted, the better I suppose. Please feel free to start converting. Thanks for offering the help. The most important thing is, remove security checks from the callsite and make sure to pass the right security flags to the loadInfo, keep the tree green and don't introduce any security holes :-)
Flags: needinfo?(mozilla)
Comment 5•9 years ago
|
||
I would say the most important call sites for service workers are: 1) navigations (DOCUMENT and SUBDOCUMENT nsContentPolicyTypes) 2) top level worker scripts 3) FetchDriver (1) and (2) are needed to get our RequestMode set correctly. (3) is simply because we're likely to conflict with changes there.
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #5) > I would say the most important call sites for service workers are: > > 1) navigations (DOCUMENT and SUBDOCUMENT nsContentPolicyTypes) See Bug 1182569, which is blocked by bug 1105556. > 2) top level worker scripts I assume we are loading them using our scriptloader, see bug 1194526. > 3) FetchDriver see bug 1195167 > (1) and (2) are needed to get our RequestMode set correctly. (3) is simply > because we're likely to conflict with changes there. To sum it up, docshell (1) (which is responsible for loading type_document and type_subdocument) is most likely the most 'blocking' one at the moment. I already have an initial implementation for (2) and (3), they should land soonish.
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #7) > Do we still have any asyncOpen callers? We shouldn't. Eventually we are going to remove asyncOpen and only have asyncOpen2. So far that was not possible because of addons, but I think now we should be able to do that.
Flags: needinfo?(ckerschb)
Comment 9•7 years ago
|
||
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #8) > (In reply to Masatoshi Kimura [:emk] from comment #7) > > Do we still have any asyncOpen callers? > > We shouldn't. Eventually we are going to remove asyncOpen and only have > asyncOpen2. So far that was not possible because of addons, but I think now > we should be able to do that. Thank you. Filed bug 1411609 and closing this bug.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•3 years ago
|
Summary: [tracking bug] change all callsites to asyncOpen2 instead of asyncOpen → [meta] [tracking bug] change all callsites to asyncOpen2 instead of asyncOpen
You need to log in
before you can comment on or make changes to this bug.
Description
•