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)

defect
Not set
normal

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: nobody → mozilla
Blocks: 1006868, 1006881
Depends on: 1143922
Depends on: 1182537
Depends on: 1182539
Depends on: 1182540
Depends on: 1182543
Depends on: 1182544
Depends on: 1182546
Depends on: 1182569
Depends on: 1182571
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
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.
Depends on: 1187165
Depends on: 1188028
Depends on: 1188040
Depends on: 1188637
Depends on: 1188642
Depends on: 1188644
Depends on: 1191645
Depends on: 1192333
Depends on: 1192943
Depends on: 1192945
Depends on: 1192946
Depends on: 1192948
Depends on: 1192955
Depends on: 1193558
Depends on: 1193924
Depends on: 1194523
Depends on: 1194524
Depends on: 1194526
Depends on: 1195162
Depends on: 1195167
Depends on: 1195168
Depends on: 1195172
Depends on: 1195173
Depends on: 1195504
Depends on: 1195519
Depends on: 1195606
Depends on: 1195611
Depends on: 1196013
Depends on: 1196021
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)
(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)
Depends on: 1196057
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.
(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.
Depends on: 1197002
Depends on: 1197918
Depends on: 1197923
Depends on: 1197925
Depends on: 1197926
Depends on: 1199295
Depends on: 1199491
Depends on: 1201636
Depends on: 1205151
Depends on: 1205153
Depends on: 1205154
Depends on: 1205158
Status: NEW → ASSIGNED
Depends on: 1206146
Depends on: 1206149
Depends on: 1206952
Depends on: 1206955
Depends on: 1206956
Depends on: 1206957
Depends on: 1206958
Depends on: 1206961
Depends on: 1206962
Depends on: 1206964
Depends on: 1206966
Depends on: 1218433
Depends on: 1222297
Depends on: 1223225
Depends on: 1223231
Depends on: 1223234
Depends on: 1223431
Depends on: 1223435
Depends on: 1223437
Depends on: 1225355
Depends on: 1225357
Depends on: 1225360
Depends on: 1225361
Depends on: 1225362
Depends on: 1225641
Depends on: 1229888
Depends on: 1229889
Depends on: 1229890
Depends on: 1230220
Depends on: 1230221
Depends on: 1232430
Depends on: 1232432
Depends on: 1232782
Depends on: 1232783
Depends on: 1232786
Depends on: 1232792
Depends on: 1232803
Depends on: 1232901
No longer depends on: 1232901
Depends on: 1239097
Depends on: 1239107
Depends on: 1239913
Depends on: 1241561
Depends on: 1241569
Depends on: 1241576
Depends on: 1241579
Depends on: 1242730
Depends on: 1242731
Depends on: 1242857
Depends on: 1243924
Depends on: 1243936
Depends on: 1244890
Depends on: 1246227
No longer depends on: 1246227
Depends on: 1246568
Depends on: 1246578
Depends on: 1246736
Depends on: 1246725
Depends on: 1254303
Depends on: 1254308
Depends on: 1254299
Depends on: 1254320
Depends on: 1254689
Depends on: 1254691
Depends on: 1255499
Depends on: 1257924
Depends on: 1257930
Keywords: meta
Whiteboard: [domsecurity-meta]
Depends on: 1267075
Depends on: 1268044
Depends on: 1269264
Depends on: 1269267
Depends on: 1269271
Depends on: 1269279
Depends on: 1269353
Depends on: 1271188
Depends on: 1271198
Depends on: 1271317
Depends on: 1272305
Depends on: 1272320
Depends on: 1272590
Depends on: 1274219
Depends on: 1278271
Depends on: 1278272
Depends on: 1279481
Depends on: 1280198
Depends on: 1280200
Depends on: 1280204
Depends on: 1281114
Depends on: 1282750
Blocks: 1272594
Depends on: 1283759
Depends on: 1284202
Depends on: 1297370
Depends on: 1329186
Depends on: 1347066
Do we still have any asyncOpen callers?
Flags: needinfo?(ckerschb)
(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)
Blocks: 1411609
(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
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.