Closed Bug 840823 Opened 8 years ago Closed 8 years ago

Race condition when removing tabs at startup

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 21
Tracking Status
firefox19 --- affected
firefox20 --- fixed
firefox21 --- fixed

People

(Reporter: bnicholson, Assigned: bnicholson)

References

Details

Attachments

(1 file)

When we do an OOM restore startup, we stub tabs in the UI before Gecko actually loads them. If a stub is removed before its tab has been created in Gecko, this causes the stub to be re-added in Java and results in glitchy behavior.
Comment on attachment 713216 [details] [diff] [review]
Check whether new tabs were made from stubs to prevent removal race condition

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>         tabID: this.id,
>         uri: uri,
>         parentId: ("parentId" in aParams) ? aParams.parentId : -1,
>         external: ("external" in aParams) ? aParams.external : false,
>         selected: ("selected" in aParams) ? aParams.selected : true,
>         title: title,
>         delayLoad: aParams.delayLoad || false,
>         desktopMode: this.desktopMode,
>-        isPrivate: isPrivate
>+        isPrivate: isPrivate,
>+        isStub: isStub

At some point we got all Java-ish and started "isXxx". We don't use the pattern for "external" or "selected". Can we change "isStub" and "isPrivate" to "stub" and "private" ? I would accept a second patch for that change.
Attachment #713216 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 713216 [details] [diff] [review]
> Check whether new tabs were made from stubs to prevent removal race condition
> 
> At some point we got all Java-ish and started "isXxx". We don't use the
> pattern for "external" or "selected". Can we change "isStub" and "isPrivate"
> to "stub" and "private" ? I would accept a second patch for that change.

We may still want to keep "isPrivate" over "private" since "private" is a reserved word in JavaScript: https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Reserved_Words
https://hg.mozilla.org/mozilla-central/rev/84f27f437dda
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Comment on attachment 713216 [details] [diff] [review]
Check whether new tabs were made from stubs to prevent removal race condition

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 797075
User impact if declined: removing tabs during startup restore will break things
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Attachment #713216 - Flags: approval-mozilla-aurora?
Comment on attachment 713216 [details] [diff] [review]
Check whether new tabs were made from stubs to prevent removal race condition

Requesting approval for fx20
Attachment #713216 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 713216 [details] [diff] [review]
Check whether new tabs were made from stubs to prevent removal race condition

Approving for uplift, low risk and early in beta.
Attachment #713216 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.