654127 - Cache "contentMightCaptureMouse" attribute per tab instead of globally

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This reduces the number of IPC messages we send, since we don't need a round-trip to update this when activating a tab.

Comment 1

[Wesley Johnston (:wesj)]

Comment on attachment 529490 [details] [diff] [review]
patch

Looks fine to me.

Comment 2

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mozilla-central/rev/bf1db7f29686

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: followup

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.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

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.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: followup v2

We don't need a messageID for this message anymore.  We can remove it from the response too.

Comment 6

[Wesley Johnston (:wesj)]

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.

Comment 7

[Matt Brubeck (:mbrubeck)]

Pushed followup: http://hg.mozilla.org/mozilla-central/rev/94f06945b818

Comment 8

[Matt Brubeck (:mbrubeck)]

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.

Comment 9

[Matt Brubeck (:mbrubeck)]

followup re-landed: http://hg.mozilla.org/mozilla-central/rev/cefb7b2e5565