Closed Bug 584731 Opened 10 years ago Closed 10 years ago

new History.cpp should check aReason for HandleCompletion

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b4
Tracking Status
blocking2.0 --- final+

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(1 file, 1 obsolete file)

If aReason is not REASON_FINISHED we could end up doing additional work that could be unneeded, or even completely wrong since previous steps failed.
For example if adding a new page fails, we could keep going on adding a visit to the placeId of another page. I've seen this happening in a browser-chrome test and it was not funny to debug why 2 different uris had the same placeId.
asking blocking because this can bring to unexpected results and corruptions.
blocking2.0: --- → ?
Blocks: 556400
blocking2.0: ? → final+
Assignee: nobody → mak77
Attached patch patch v1.0 (obsolete) — Splinter Review
Attachment #463559 - Flags: review?(sdwilsh)
Comment on attachment 463559 [details] [diff] [review]
patch v1.0

>+    if (aReason != mozIStorageStatementCallback::REASON_FINISHED)
>+      return NS_OK;
>+
nit: brace this

r=sdwilsh
Attachment #463559 - Flags: review?(sdwilsh) → review+
Attached patch patch v1.1Splinter Review
Attachment #463559 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/e0c443c09d67
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
I had to backout this, looks like we have a BUNCH of tests that are failing on add page contentions. Something sync tries to add a page/visit when something async is doing the same. I'll file a bug to track and fix the issue.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 585966
repushed this some day ago.
http://hg.mozilla.org/mozilla-central/rev/08ca624c8616
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.