Closed Bug 748562 Opened 9 years ago Closed 9 years ago

Kill AddPageWithDetails, in Sync

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: Paolo, Assigned: Paolo)

References

Details

Attachments

(1 file, 2 obsolete files)

We're removing AddPageWithDetails in bug 739213.
Extracted from the patch in bug 739213.
Attachment #618059 - Flags: review?(rnewman)
This also removes some code that is not required anymore.
Attachment #618059 - Attachment is obsolete: true
Attachment #618059 - Flags: review?(rnewman)
Attachment #618072 - Flags: review?(rnewman)
Comment on attachment 618072 [details] [diff] [review]
Replace calls to AddPageWithDetails

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

Looks good, thanks! r+ with nits.

::: services/sync/tests/unit/test_history_store.js
@@ +91,5 @@
> +      transitionType: Ci.nsINavHistoryService.TRANSITION_LINK
> +    }]
> +  };
> +  PlacesUtils.asyncHistory.updatePlaces(place, {
> +    handleError: function () do_throw("Unexpected error in adding visit."),

Please use full {} function style to avoid tripping up Emacs users. (I know, I know...)

Also, please name all functions:

  handleError: function handleError() {
    do_throw(...);
  },

@@ +92,5 @@
> +    }]
> +  };
> +  PlacesUtils.asyncHistory.updatePlaces(place, {
> +    handleError: function () do_throw("Unexpected error in adding visit."),
> +    handleResult: function () { },

No space between {}, name function.

@@ +93,5 @@
> +  };
> +  PlacesUtils.asyncHistory.updatePlaces(place, {
> +    handleError: function () do_throw("Unexpected error in adding visit."),
> +    handleResult: function () { },
> +    handleCompletion: function () afterAddVisit()

Just 

  handleCompletion: afterAddVisit

@@ +99,2 @@
>  
> +  function afterAddVisit() {

Perhaps 'onVisitAdded'?

::: services/sync/tests/unit/test_places_guid_downgrade.js
@@ +104,5 @@
> +  ];
> +  PlacesUtils.asyncHistory.updatePlaces(places, {
> +    handleError: function () do_throw("Unexpected error in adding visit."),
> +    handleResult: function () { },
> +    handleCompletion: function () afterAddVisit()

Same comments for these three lines.

@@ +201,5 @@
>  
>  function run_test() {
>    setPlacesDatabase("places_v10_from_v11.sqlite");
>  
>    _("Verify initial setup: v11 database is available");

Please move this comment into test_initial_state.
Attachment #618072 - Flags: review?(rnewman) → review+
Attached patch Updated patchSplinter Review
Attachment #618072 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/d030e76e9ed3
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/d030e76e9ed3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.