UnifiedComplete: Close connections on Sqlite.shutdown.addBlocker rather than TOPIC_SHUTDOWN

RESOLVED FIXED in mozilla35

Status

()

Toolkit
Places
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mano, Assigned: mak)

Tracking

unspecified
mozilla35
x86
Mac OS X
Points:
2
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Blocks: 995091
(Assignee)

Updated

3 years ago
Flags: firefox-backlog+
(Assignee)

Updated

3 years ago
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Hi Marco, can you provide a point value.
Flags: qe-verify?
Flags: needinfo?(mak77)
(Assignee)

Updated

3 years ago
Points: --- → 2
Flags: qe-verify?
Flags: qe-verify-
Flags: needinfo?(mak77)
(Assignee)

Comment 2

3 years ago
Created attachment 8497135 [details] [diff] [review]
patch v1
Attachment #8497135 - Flags: review?(mano)
Comment on attachment 8497135 [details] [diff] [review]
patch v1

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

::: toolkit/components/places/UnifiedComplete.js
@@ +1497,5 @@
> +                                      }));
> +         } catch (ex) {
> +           // It's too late to block shutdown, just close the connection.
> +           yield conn.close();
> +           throw ex;

I'd not propagate the exception (btw, my caller in PlacesUtils doesn't do so either).

@@ +1504,1 @@
>          // Autocomplete often fallbacks to a table scan due to lack of text

Fix the indention of the catch block (all 5 lines).
Attachment #8497135 - Flags: review?(mano) → review+
(Assignee)

Comment 4

3 years ago
(In reply to Mano (::mano, needinfo? for any questions; not reading general bugmail) from comment #3)
> ::: toolkit/components/places/UnifiedComplete.js
> @@ +1497,5 @@
> > +                                      }));
> > +         } catch (ex) {
> > +           // It's too late to block shutdown, just close the connection.
> > +           yield conn.close();
> > +           throw ex;
> 
> I'd not propagate the exception (btw, my caller in PlacesUtils doesn't do so
> either).

I think we should reject though, otherwise we would be hiding a mistake. And this does.
(Assignee)

Comment 5

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/6938bcd1cd76
(Assignee)

Updated

3 years ago
Target Milestone: --- → mozilla35

Updated

3 years ago
Iteration: --- → 35.3
https://hg.mozilla.org/mozilla-central/rev/6938bcd1cd76
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.