Closed Bug 855638 Opened 7 years ago Closed 7 years ago

Remove previous calls to PlacesUtils.backups and move them to PlacesBackups

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: raymondlee, Assigned: raymondlee)

References

Details

Attachments

(2 files, 6 obsolete files)

Once PlacesBackups.jsm is landed, we should remove previous calls to PlacesUtils.backups and move them to PlacesBackups.
Blocks: 852030
Blocks: 857429
This bug is to move the calls, not to remove PlacesUtils.backups.  Bug 857429 would handle remove and deprecate PlacesUtils.backups.
Assignee: nobody → raymond
Attached patch Part 1 - Update browser/ (obsolete) — Splinter Review
Attached patch Part 2 - Update toolkit/places (obsolete) — Splinter Review
Attachment #732734 - Flags: review?(mak77)
Attachment #732734 - Attachment description: Part 2 - toolkit/places → Part 2 - Update toolkit/places
Attachment #732716 - Flags: review?(mak77)
Comment on attachment 732716 [details] [diff] [review]
Part 1 - Update browser/

Review of attachment 732716 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/Makefile.in
@@ +67,5 @@
>  
>  EXTRA_PP_JS_MODULES = \
>    BookmarkHTMLUtils.jsm \
>    PlacesUtils.jsm \
> +  PlacesBackups.jsm \

should be done in the PlacesBackups patch (just added a note)

bot not in the _PP_ section since it doesn't appear to have any preprocessor directives, rather in an EXTRA_JS_MODULE

::: toolkit/components/places/PlacesBackups.jsm
@@ +38,5 @@
>      delete this.folder;
>      return this.folder = bookmarksBackupDir;
>    },
>  
> +  get profileRelativeFolderPath() "bookmarkbackups",

please file a bug to check if we can remove profileRelativeFolderPath from PlacesUtils, or otherwise mark it as deprecated if add-ons rely on it.
Attachment #732716 - Flags: review?(mak77) → review+
Comment on attachment 732734 [details] [diff] [review]
Part 2 - Update toolkit/places

Review of attachment 732734 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/tests/head_common.js
@@ +36,5 @@
>  
>  // This imports various other objects in addition to PlacesUtils.
>  Cu.import("resource://gre/modules/PlacesUtils.jsm");
>  Cu.import("resource://gre/modules/BookmarkJSONUtils.jsm");
> +Cu.import("resource://gre/modules/PlacesBackups.jsm");

the last 2 should rather be defineLazyModuleGetter
Attachment #732734 - Flags: review?(mak77) → review+
Attached patch Part 1 - Update browser/ v2 (obsolete) — Splinter Review
> please file a bug to check if we can remove profileRelativeFolderPath from
> PlacesUtils, or otherwise mark it as deprecated if add-ons rely on it.

Filed bug 859151
Attachment #732716 - Attachment is obsolete: true
Attached patch Part 2 - toolkit/places v2 (obsolete) — Splinter Review
(In reply to Marco Bonardo [:mak] from comment #5)
> Comment on attachment 732734 [details] [diff] [review]
> Part 2 - Update toolkit/places
> 
> Review of attachment 732734 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/places/tests/head_common.js
> @@ +36,5 @@
> >  
> >  // This imports various other objects in addition to PlacesUtils.
> >  Cu.import("resource://gre/modules/PlacesUtils.jsm");
> >  Cu.import("resource://gre/modules/BookmarkJSONUtils.jsm");
> > +Cu.import("resource://gre/modules/PlacesBackups.jsm");
> 
> the last 2 should rather be defineLazyModuleGetter

Fixed
Attachment #732734 - Attachment is obsolete: true
Blocks: 859151
Attached patch Part 1 - Update browser/ v3 (obsolete) — Splinter Review
Added back the profileRelativeFolderPath() into PlacesBackups
Attachment #734412 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Blocks: 855218
No longer blocks: 855218
Blocks: 818593
Attached patch Part 1 - Update browser/ v4 (obsolete) — Splinter Review
Resolve conflict with latest code
Attachment #734965 - Attachment is obsolete: true
Depends on: 852034
Attachment #737404 - Attachment is obsolete: true
Passed try
https://tbpl.mozilla.org/?tree=Try&rev=d26551269adb

Please ensure that bug 852034 is checked in first.
Attachment #734420 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aa7426b6e43f
https://hg.mozilla.org/mozilla-central/rev/0e7949ba1f68
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You missed /services/!
Blocks: 865643
Blocks: 867008
You need to log in before you can comment on or make changes to this bug.