Closed
Bug 586693
Opened 14 years ago
Closed 14 years ago
Do we still need to marshal browser events?
Categories
(Firefox Graveyard :: Panorama, defect)
Firefox Graveyard
Panorama
Tracking
(blocking2.0 beta4+)
RESOLVED
FIXED
Firefox 4.0b4
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta4+ |
People
(Reporter: iangilman, Assigned: raymondlee)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Dolske
:
review+
|
Details | Diff | Splinter Review |
From dolske's review of bug 574217:
> >+ setTimeout(function() { // Marshal event from chrome thread to DOM thread
>
> I'm not sure what that comment means. Everything is running on one thread.
Things may have changed since I started this pattern, but at the time (a few months ago), events about xul:tabs (such as TabSelect) were in fact arriving on a different thread than the main UI thread of the Tab Candy frame. This was readily apparent with some logging; two of our routines were running simultaneously in parallel. This caused all sorts of extremely unpleasant, unpredictable bugs. Since we've added those setTimeouts to event end points, all of those bugs have gone away.
Might as well take a look and see if this is still necessary.
Updated•14 years ago
|
Whiteboard: b5
Comment 1•14 years ago
|
||
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy. Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Comment 2•14 years ago
|
||
It's not possible in our embedding for two JS functions to be running "in parallel". It's possible to return to the event loop between invocations (using a timeout, e.g.), and even to spin the event loop in JS (using nsIThreadManager, if you're evil) and run nested JS code, but two pieces of JS code cannot run concurrently, so you must have been seeing something else.
Updated•14 years ago
|
Assignee: nobody → raymond
Comment 3•14 years ago
|
||
I tested out removing the setTimeouts and seemed to cause the failing tests to pass when applying the patch from bug 574875. There were some broken tabcandy zooming, etc, but Raymond has fixed those up together in this patch.
Attachment #466248 -
Flags: review?(dolske)
Attachment #466248 -
Flags: feedback?(ian)
Updated•14 years ago
|
Attachment #466248 -
Attachment is obsolete: true
Attachment #466248 -
Flags: review?(dolske)
Attachment #466248 -
Flags: feedback?(ian)
Comment 4•14 years ago
|
||
Attachment #466255 -
Flags: review?(dolske)
Attachment #466255 -
Flags: feedback?(ian)
Comment 5•14 years ago
|
||
Removal of setTimeout fixes a number of bugs and beta blockers without leading to test failures: bug 587029 bug 586552 bug 574875
Updated•14 years ago
|
blocking2.0: ? → beta4+
Whiteboard: b5
Comment 7•14 years ago
|
||
Comment on attachment 466255 [details] [diff] [review]
v1
Looks larger than it really is because of some format/indent changes, but setTimeout remove is righteous.
Attachment #466255 -
Flags: review?(dolske) → review+
Updated•14 years ago
|
Attachment #466255 -
Flags: approval2.0?
Updated•14 years ago
|
Attachment #466255 -
Flags: approval2.0?
Comment 9•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a15f73b8d431
* Removed some timeouts and fixed some broken user interactions that fixes various other bugs and test failures.
Bug 587029 - Tab Candy : closing last tab of a group leads to an isolated tab
Bug 586552 - GroupItem.newTab feedback should be immediate
* Init TabItems before handling firstrun tab grouping
* Removed _stopZoomPreparation related code since we are not using it anymore.
* Fixed the issue related to using move to other group feature. The moved tab is still visible in the tab bar after moving it to other group.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b4
Updated•14 years ago
|
Attachment #466255 -
Flags: feedback?(ian)
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•