Closed Bug 654127 Opened 11 years ago Closed 11 years ago

Cache "contentMightCaptureMouse" attribute per tab instead of globally

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 6

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
This reduces the number of IPC messages we send, since we don't need a round-trip to update this when activating a tab.
Attachment #529490 - Flags: review?(wjohnston)
Comment on attachment 529490 [details] [diff] [review]
patch

Looks fine to me.
Attachment #529490 - Flags: review?(wjohnston) → review+
http://hg.mozilla.org/mozilla-central/rev/bf1db7f29686
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 6
Attached patch followup (obsolete) — Splinter Review
One remaining problem: updateContentCapture uses the global message manager, so it broadcasts a message to all tabs and gets responses from all of them.  This doesn't cause any incorrect behavior, but it causes some unneeded messages.

This patch makes updateContentCapture work send a message to the relevant tab only, and moves it out of ContentTouchHandler to the Tab/Browser objects.
Attachment #529490 - Attachment is obsolete: true
Attachment #530229 - Flags: review?(wjohnston)
Comment on attachment 530229 [details] [diff] [review]
followup


>-  updateContentCapture: function() {
>-    this._messageId++;
>-    messageManager.sendAsyncMessage("Browser:CanCaptureMouse", { messageId: this._messageId });
>-  },

>+  updateContentCapture: function() {
>+    this._browser.messageManager.sendAsyncMessage("Browser:CanCaptureMouse", {});
>+  },

You dropped the messageId, but the content listener is still using it. Well, it's just passing it back. Do we need the ID for sync? Or can we really drop it? If we can drop it, remove it from content too.
Attached patch followup v2Splinter Review
We don't need a messageID for this message anymore.  We can remove it from the response too.
Attachment #530229 - Attachment is obsolete: true
Attachment #530229 - Flags: review?(wjohnston)
Attachment #530301 - Flags: review?(wjohnston)
Comment on attachment 530301 [details] [diff] [review]
followup v2

Ever so slowly, my evil plot to put this in the browser bindings comes to fruition.
Attachment #530301 - Flags: review?(wjohnston) → review+
Backed out the followup because something that landed with this caused a new orange (Win debug Moth tabview/browser_tabview_bug597248.js):
http://hg.mozilla.org/mozilla-central/rev/eaaa24ff93a0

This can probably land again once we figure out the cause of the test failures.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
followup re-landed: http://hg.mozilla.org/mozilla-central/rev/cefb7b2e5565
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.