Closed Bug 1173681 Opened 5 years ago Closed 5 years ago

profdirserviceprovidersa_s is mostly unused

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox41 fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(4 files)

So, there is this library in profile/dirserviceprovider, called profdirserviceprovidersa_s. It contains nsProfileLock.cpp and ProfileUnlockerWin.cpp, which are used in toolkit/profile, but not through the library, they're used directly:
http://hg.mozilla.org/mozilla-central/file/bfd82015df48/toolkit/profile/moz.build#l18

Now, that profdirserviceprovidersa_s library is only used by winEmbed, which is only build with xulrunner.

So here are my thoughts:
- nsProfileLock.{cpp,h} and ProfileUnlockerWin.{cpp,h} should move in toolkit/profile.
- profdirserviceprovidersa_s should use the source from toolkit/profile instead of the other way around, and only be built when winEmbed is built.
- or, instead of that second point, we kill both profdirserviceprovidersa_s and winEmbed.

Benjamin, thought?
Flags: needinfo?(benjamin)
I think it's time to kill winembed and profdirservice..sa_s
Flags: needinfo?(benjamin)
Bug 1173681 - Remove winEmbed
Attachment #8621340 - Flags: review?(benjamin)
Bug 1173681 - Don't export the ProfileUnlockerWin.h header
Attachment #8621341 - Flags: review?(benjamin)
Bug 1173681 - Move nsProfileLock related files in toolkit/profile and remove profile/dirserviceprovider
Attachment #8621342 - Flags: review?(benjamin)
Bug 1173681 - Move nsIProfileUnlocker.idl to toolkit/profile and remove profile
Attachment #8621343 - Flags: review?(benjamin)
Unhelpfully, both mozreview and hgweb display file moves as delete + create :( (and splinter doesn't display them at all)
Blocks: 1174032
Comment on attachment 8621340 [details]
MozReview Request: Bug 1173681 - Remove winEmbed

https://reviewboard.mozilla.org/r/10951/#review10271

Ship It!
Attachment #8621340 - Flags: review?(benjamin) → review+
Comment on attachment 8621341 [details]
MozReview Request: Bug 1173681 - Don't export the ProfileUnlockerWin.h header

https://reviewboard.mozilla.org/r/10953/#review10273

Ship It!
Attachment #8621341 - Flags: review?(benjamin) → review+
Comment on attachment 8621342 [details]
MozReview Request: Bug 1173681 - Move nsProfileLock related files in toolkit/profile and remove profile/dirserviceprovider

https://reviewboard.mozilla.org/r/10955/#review10275

::: toolkit/profile/ProfileUnlockerWin.h:1
(Diff revision 1)
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

Does this match use proper hg moves? I'm still figuring out the best way to review the moves that appears as remove/add.
Attachment #8621342 - Flags: review?(benjamin)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> Comment on attachment 8621342 [details]
> MozReview Request: Bug 1173681 - Move nsProfileLock related files in
> toolkit/profile and remove profile/dirserviceprovider
> 
> https://reviewboard.mozilla.org/r/10955/#review10275
> 
> ::: toolkit/profile/ProfileUnlockerWin.h:1
> (Diff revision 1)
> > +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> 
> Does this match use proper hg moves? I'm still figuring out the best way to
> review the moves that appears as remove/add.

It does, but mozreview doesn't display them as such, hgweb on reviewboard doesn't either, and splinter doesn't either. Presently, the only way would be to hg pull the revision and look at it with hg log -p, which should display the moves.
Attachment #8621342 - Flags: review+
Comment on attachment 8621342 [details]
MozReview Request: Bug 1173681 - Move nsProfileLock related files in toolkit/profile and remove profile/dirserviceprovider

https://reviewboard.mozilla.org/r/10955/#review10281

Ship It!
Comment on attachment 8621343 [details]
MozReview Request: Bug 1173681 - Move nsIProfileUnlocker.idl to toolkit/profile and remove profile

https://reviewboard.mozilla.org/r/10957/#review10283

Ship It!
Attachment #8621343 - Flags: review?(benjamin) → review+
Depends on: 1177330
Depends on: 1181715
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.