Intermittent test_pluginstream_referer.html,plugin-asyncbitmap-sanity.html | Assertion count 8 is greater than expected range Received "nonqueued" message n during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass")

RESOLVED FIXED in Firefox 33, Firefox OS v2.1

Status

()

Core
Plug-ins
P3
normal
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: RyanVM, Assigned: jimm)

Tracking

(Blocks: 2 bugs, {assertion, intermittent-failure})

Trunk
mozilla34
x86
Windows XP
assertion, intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox32 wontfix, firefox33 fixed, firefox34 fixed, firefox-esr24 wontfix, firefox-esr31 fixed, b2g-v1.3 wontfix, b2g-v1.3T wontfix, b2g-v1.4 wontfix, b2g-v2.0 wontfix, b2g-v2.1 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Not sure if this belongs to plugins or IPC, so CCing a representative from both :)

https://tbpl.mozilla.org/php/getParsedLog.php?id=23200093&tree=Mozilla-Inbound

Windows XP 32-bit mozilla-inbound debug test mochitest-3 on 2013-05-21 04:17:01 PDT for push da94e7f02dad
slave: t-xp32-ix-055

05:30:43     INFO -  [Parent 3988] ###!!! ASSERTION: Received "nonqueued" message 49450 during a synchronous IPC message for window 459024 ("MozillaHiddenWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file e:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/ipc/glue/WindowsMessageLoop.cpp, line 297
05:30:43     INFO -  NeuteredWindowProc(HWND__ *,unsigned int,unsigned int,long) [ipc/glue/WindowsMessageLoop.cpp:335]
05:30:43     INFO -  USER32 + 0x8734
05:30:43     INFO -  USER32 + 0x8816
05:30:43     INFO -  USER32 + 0x18ea0
05:30:43     INFO -  USER32 + 0x18eec
05:30:43     INFO -  ntdll + 0xe453
05:30:43     INFO -  USER32 + 0x9402
05:30:43     INFO -  mozilla::ipc::RPCChannel::WaitForNotify() [ipc/glue/WindowsMessageLoop.cpp:931]
05:30:43     INFO -  mozilla::ipc::RPCChannel::Call(IPC::Message *,IPC::Message *) [ipc/glue/RPCChannel.cpp:170]
05:30:43     INFO -  mozilla::plugins::PPluginInstanceParent::CallNPP_Destroy(short *) [obj-firefox/ipc/ipdl/PPluginInstanceParent.cpp:891]
05:30:43     INFO -  mozilla::plugins::PluginInstanceParent::Destroy() [dom/plugins/ipc/PluginInstanceParent.cpp:149]
05:30:43     INFO -  mozilla::plugins::PluginModuleParent::NPP_Destroy(_NPP *,_NPSavedData * *) [dom/plugins/ipc/PluginModuleParent.cpp:864]
05:30:43     INFO -  nsNPAPIPluginInstance::Stop() [dom/plugins/base/nsNPAPIPluginInstance.cpp:321]
05:30:43     INFO -  nsPluginHost::StopPluginInstance(nsNPAPIPluginInstance *) [dom/plugins/base/nsPluginHost.cpp:3130]
05:30:43     INFO -  nsObjectLoadingContent::StopPluginInstance() [content/base/src/nsObjectLoadingContent.cpp:2631]
05:30:43     INFO -  nsObjectLoadingContent::UnloadObject(bool) [content/base/src/nsObjectLoadingContent.cpp:2190]
05:30:43     INFO -  CheckPluginStopEvent::Run() [content/base/src/nsObjectLoadingContent.cpp:176]
05:30:43     INFO -  nsBaseAppShell::RunSyncSectionsInternal(bool,unsigned int) [widget/xpwidgets/nsBaseAppShell.cpp:354]
05:30:43     INFO -  nsBaseAppShell::AfterProcessNextEvent(nsIThreadInternal *,unsigned int) [widget/xpwidgets/nsBaseAppShell.cpp:409]
05:30:43     INFO -  nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:642]
05:30:43     INFO -  NS_ProcessNextEvent(nsIThread *,bool) [obj-firefox/xpcom/build/nsThreadUtils.cpp:238]
05:30:43     INFO -  mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:82]
05:30:43     INFO -  MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:219]
05:30:43     INFO -  MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:213]
05:30:43     INFO -  MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:187]
05:30:43     INFO -  nsBaseAppShell::Run() [widget/xpwidgets/nsBaseAppShell.cpp:165]
05:30:43     INFO -  nsAppShell::Run() [widget/windows/nsAppShell.cpp:113]
05:30:43     INFO -  nsAppStartup::Run() [toolkit/components/startup/nsAppStartup.cpp:270]
05:30:43     INFO -  XREMain::XRE_mainRun() [toolkit/xre/nsAppRunner.cpp:3872]
05:30:43     INFO -  XREMain::XRE_main(int,char * * const,nsXREAppData const *) [toolkit/xre/nsAppRunner.cpp:3939]
05:30:43     INFO -  XRE_main [toolkit/xre/nsAppRunner.cpp:4151]
05:30:43     INFO -  do_main [browser/app/nsBrowserApp.cpp:272]
05:30:43     INFO -  NS_internal_main(int,char * *) [browser/app/nsBrowserApp.cpp:632]
05:30:43     INFO -  wmain [toolkit/xre/nsWindowsWMain.cpp:112]
05:30:43     INFO -  __tmainCRTStartup [f:/dd/vctools/crt_bld/self_x86/crt/src/crtexe.c:552]
05:30:43     INFO -  wmainCRTStartup [f:/dd/vctools/crt_bld/self_x86/crt/src/crtexe.c:371]
05:30:43     INFO -  kernel32 + 0x17067
05:30:43     INFO -  [Parent 3988] ###!!! ASSERTION: Received "nonqueued" message 49450 during a synchronous IPC message for window 327920 ("nsAppShell:EventWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file e:/builds/moz2_slave/m-in-w32-d-0000000000000000000/build/ipc/glue/WindowsMessageLoop.cpp, line 297
05:30:43     INFO -  19852 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/plugins/test/mochitest/test_pluginstream_referer.html | Assertion count 8 is greater than expected range 0-0 assertions.
Priority: -- → P3
https://tbpl.mozilla.org/php/getParsedLog.php?id=23429858&tree=Mozilla-Inbound
Summary: Intermittent test_pluginstream_referer.html | Assertion count 8 is greater than expected range (Received "nonqueued" message 49450 during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass"), sending it to DefWindowProc instead of...) → Intermittent test_pluginstream_referer.html,plugin-asyncbitmap-sanity.html | Assertion count 8 is greater than expected range Received "nonqueued" message n during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass")
(Reporter)

Updated

5 years ago
Blocks: 877367
Duplicate of this bug: 877367
From bug 877367:

> https://tbpl.mozilla.org/php/getParsedLog.php?id=23530081&tree=Mozilla-Central

(Georg Fritzsche [:gfritzsche] from comment #1)
> > [Parent 3524] ###!!! ASSERTION: Received "nonqueued" message 49431 during a synchronous IPC message for window 393456 ("nsAppShell:EventWindowClass"), sending it to DefWindowProc instead of the normal window procedure.: 'Error', file [...]/build/ipc/glue/WindowsMessageLoop.cpp, line 297
> 
> Hm, that is a surprisingly high message-identifier (WM_USER + 48407/0xBD17).
> I'm not sure what to make of this or to check where it could come from.

Both cases here also have rather high message-ids.
Summary: Intermittent test_pluginstream_referer.html,plugin-asyncbitmap-sanity.html | Assertion count 8 is greater than expected range Received "nonqueued" message n during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass") → Intermittent test_pluginstream_referer.html,plugin-asyncbitmap-sanity.html,test_bug532208.htm | Assertion count 8 is greater than expected range Received "nonqueued" message n during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass")
(Reporter)

Updated

5 years ago
Summary: Intermittent test_pluginstream_referer.html,plugin-asyncbitmap-sanity.html,test_bug532208.htm | Assertion count 8 is greater than expected range Received "nonqueued" message n during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass") → Intermittent test_pluginstream_referer.html,plugin-asyncbitmap-sanity.html | Assertion count 8 is greater than expected range Received "nonqueued" message n during a synchronous IPC message for window xxx ("nsAppShell:EventWindowClass")
Blocks: 884233
(Reporter)

Updated

4 years ago
Blocks: 900112
(Reporter)

Comment 4

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=26385672&tree=Mozilla-Aurora
https://tbpl.mozilla.org/php/getParsedLog.php?id=32460879&tree=Mozilla-Inbound
(Reporter)

Comment 6

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=41283917&full=1&branch=mozilla-inbound#error0
(Reporter)

Comment 7

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=41299419&tree=Mozilla-Inbound&full=1#error0
(Reporter)

Comment 8

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=41298523&tree=Mozilla-Inbound&full=1#error0
(Reporter)

Comment 9

4 years ago
Can't help but notice that bug 1022140 landed right before this started spiking today.
Flags: needinfo?(jwatt)
(Reporter)

Comment 10

4 years ago
Nevermind, comment 7 was prior to that.
Flags: needinfo?(jwatt)
(Reporter)

Comment 11

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=41302607&tree=Mozilla-Inbound
(Reporter)

Comment 12

4 years ago
I haven't been able to reproduce it on any pushes earlier than bug 992521.
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Windows%20XP%2032-bit%20mozilla-inbound%20debug%20test%20reftest&fromchange=6cf2f1cd655c&tochange=8730ce555151
Flags: needinfo?(wchen)
For no particular reason I'd been favoring bug 884233 lately, though that flare-up (or those multiple flare-ups, dunno) might have been different than the current one.
https://tbpl.mozilla.org/php/getParsedLog.php?id=41413374&tree=Mozilla-Inbound
The stacks in the orange test run doesn't seem to suggest it was caused by the changes in bug 992521.
Flags: needinfo?(wchen)
https://tbpl.mozilla.org/php/getParsedLog.php?id=41755798&tree=Mozilla-Inbound
(Reporter)

Comment 17

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=41974976&tree=Fx-Team
https://tbpl.mozilla.org/php/getParsedLog.php?id=42439600&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42710210&tree=Fx-Team
And we're back with the reftests, with an equally useless starting position. https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=XP.*debug%20test%20reftest&fromchange=c134a28bc143&tochange=ee3e74ba96a9 would make you think it was bug 1018551/bug 1018034, especially since they landed and then got backed out earlier, but according to https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=874437&entireHistory=true&tree=trunk the previous spike of this in reftests started before them, and stopped after them. Dunno.

https://tbpl.mozilla.org/php/getParsedLog.php?id=42724520&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724133&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724143&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724104&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42710627&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42712892&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42712911&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42711548&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42712923&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42721671&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724207&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724550&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724134&tree=Mozilla-Central
https://tbpl.mozilla.org/php/getParsedLog.php?id=42724205&tree=Mozilla-Central
https://tbpl.mozilla.org/php/getParsedLog.php?id=43683151&tree=Mozilla-Central&full=1#error0
(Reporter)

Comment 22

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=43683787&full=1&branch=mozilla-inbound#error0
(Reporter)

Comment 23

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=43717347&full=1&branch=fx-team#error0
(Reporter)

Comment 24

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=44202472&full=1&branch=fx-team#error0
(Reporter)

Comment 25

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=44225167&tree=Mozilla-Inbound
(Reporter)

Comment 26

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=44228252&tree=Mozilla-Central
(Reporter)

Comment 27

3 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=45540521&tree=Fx-Team

This assert seems to be hitting us in more and more random places. Can we just downgrade it already since it doesn't seem likely it's going to be fixed any time soon.
Flags: needinfo?(benjamin)

Comment 28

3 years ago
No. The current version of this assertion appear properly-serious. Here is the stack:

08:55:05     INFO -  mozilla::ipc::MessageChannel::WaitForSyncNotify() [ipc/glue/WindowsMessageLoop.cpp:886]
08:55:05     INFO -  mozilla::ipc::MessageChannel::SendAndWait(IPC::Message *,IPC::Message *) [ipc/glue/MessageChannel.cpp:712]
08:55:05     INFO -  mozilla::ipc::MessageChannel::Send(IPC::Message *,IPC::Message *) [ipc/glue/MessageChannel.cpp:619]
08:55:05     INFO -  mozilla::layers::PCompositorChild::SendMakeSnapshot(mozilla::layers::SurfaceDescriptor const &,nsIntRect const &) [obj-firefox/ipc/ipdl/PCompositorChild.cpp:305]
08:55:05     INFO -  mozilla::layers::ClientLayerManager::MakeSnapshotIfRequired() [gfx/layers/client/ClientLayerManager.cpp:395]
08:55:05     INFO -  mozilla::layers::ClientLayerManager::EndTransaction(void (*)(mozilla::layers::ThebesLayer *,gfxContext *,nsIntRegion const &,mozilla::layers::DrawRegionClip,nsIntRegion const &,void *),void *,mozilla::layers::LayerManager::EndTransactionFlags) [gfx/layers/client/ClientLayerManager.cpp:259]
08:55:05     INFO -  nsDisplayList::PaintForFrame(nsDisplayListBuilder *,nsRenderingContext *,nsIFrame *,unsigned int) [layout/base/nsDisplayList.cpp:1333]
08:55:05     INFO -  nsDisplayList::PaintRoot(nsDisplayListBuilder *,nsRenderingContext *,unsigned int) [layout/base/nsDisplayList.cpp:1180]
08:55:05     INFO -  nsLayoutUtils::PaintFrame(nsRenderingContext *,nsIFrame *,nsRegion const &,unsigned int,unsigned int) [layout/base/nsLayoutUtils.cpp:3065]
08:55:05     INFO -  PresShell::RenderDocument(nsRect const &,unsigned int,unsigned int,gfxContext *) [layout/base/nsPresShell.cpp:4869]
08:55:05     INFO -  mozilla::dom::CanvasRenderingContext2D::DrawWindow(nsGlobalWindow &,double,double,double,double,nsAString_internal const &,unsigned int,mozilla::ErrorResult &) [dom/canvas/CanvasRenderingContext2D.cpp:3697]
08:55:05     INFO -  mozilla::dom::CanvasRenderingContext2DBinding::drawWindow [obj-firefox/dom/bindings/CanvasRenderingContext2DBinding.cpp:4358]
08:55:05     INFO -  mozilla::dom::GenericBindingMethod(JSContext *,unsigned int,JS::Value *) [dom/bindings/BindingUtils.cpp:2452]

So for OMTC we're using IPDL synchronous messages, which is triggering windows message interception/unblocking. But that code really should only be used for plugin processes, and should not be active for same-process OMTC or GMP plugins. It should also probably not be active for content processes, although CPOWs might mean we have to do that.

Jim, do you know why we're using the message-deferring code for OMTC and how we should stop doing that?
Flags: needinfo?(benjamin) → needinfo?(jmathies)
https://tbpl.mozilla.org/php/getParsedLog.php?id=45637044&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=45628923&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=45625828&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=45641906&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=45668657&tree=Fx-Team
(Assignee)

Updated

3 years ago
Blocks: 1052362
(Assignee)

Comment 33

3 years ago
> So for OMTC we're using IPDL synchronous messages, which is triggering
> windows message interception/unblocking. But that code really should only be
> used for plugin processes, and should not be active for same-process OMTC or
> GMP plugins. It should also probably not be active for content processes,
> although CPOWs might mean we have to do that.
> 
> Jim, do you know why we're using the message-deferring code for OMTC and how
> we should stop doing that?

The child side of OMTC is on the UI thread so all our deferred messaging is going to be active on that side of the connection.
Flags: needinfo?(jmathies)

Comment 34

3 years ago
Yes, but do you agree that it shouldn't be active here? The compositor thread isn't going to be sending windows messages, so solving deadlocks shouldn't be necessary.

Updated

3 years ago
Flags: needinfo?(jmathies)
(Assignee)

Comment 35

3 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #34)
> Yes, but do you agree that it shouldn't be active here? The compositor
> thread isn't going to be sending windows messages, so solving deadlocks
> shouldn't be necessary.

Hard to say, not sure what *could* happen when we're executing accelerated Windows rendering code. It'd be interesting to land a fix for this and see what happens. :)
Flags: needinfo?(jmathies)

Comment 36

3 years ago
Jim, what do you recommend here? We really need to either fix these messages which may be causing unsafe re-entrance, or we need to stop processing them at all, which may run the risk of deadlocking with the OMTC thread if code on that thread is using sync messages. Rock and hard place, but we've got to do something.
Flags: needinfo?(jmathies)
(Assignee)

Comment 37

3 years ago
Lets ask Bas, see what he thinks. Bas, I'm not sure about message processing and d2d apis. I know we pass a top level hwnd to d2d in some cases, and obviously d2d is COM based which means internally Windows might block on some sort of interprocess call that results in a message pump getting called. bsmedberg is suggesting we turn message trapping off on both sides of the omtc channel. Curious if this rings any alarm bells for you based on your understanding of d2d's internal logic.
Flags: needinfo?(bas)
(Assignee)

Comment 38

3 years ago
bug 1021053 is related to this, some of the stacks in there are freaking scary. If we can turn message trapping off for omtc bug 1021053 should get fixed too.
Blocks: 1021053
(Assignee)

Updated

3 years ago
Assignee: nobody → jmathies
Flags: needinfo?(jmathies)
(Assignee)

Comment 39

3 years ago
Created attachment 8472658 [details] [diff] [review]
wip

This is a start, there's still some things to work out. I need to deal with streams, which don't have easy to set up initialization like the other plugin protocol classes.

I'm wondering if top level protocols should hand their flags down to sub protocols when they initialize. Those changes though will be more invasive, so I'm not sure its worth it since the only protocols using this will be plugins, so it seems ok to me to handle the calls to SetChannelFlags manually.

The other thing I'm wondering is if parents should pass their flags to their children after a call to the constructor, which would simplify getting children configured right. Again, I think this would require more invasive changes, so I'm not sure if it's needed.

One of the biggest pitas here is that mChannel is set post the call to the constructor in ipdl generated code. If that was set prior, I could place SetChannelFlags calls in each constructor, which would simplify this quite a bit.

More generally, I've been testing with this for a bit, and things seems stable. Will push this to try to see what shows up and continue work on remaining issues.
Attachment #8472658 - Flags: feedback?(benjamin)
(In reply to Jim Mathies [:jimm] from comment #37)
> Lets ask Bas, see what he thinks. Bas, I'm not sure about message processing
> and d2d apis. I know we pass a top level hwnd to d2d in some cases, and
> obviously d2d is COM based which means internally Windows might block on
> some sort of interprocess call that results in a message pump getting
> called. bsmedberg is suggesting we turn message trapping off on both sides
> of the omtc channel. Curious if this rings any alarm bells for you based on
> your understanding of d2d's internal logic.

Currently we shouldn't ever be passing a windows directly to Direct2D anymore. We are still doing so for Direct3D, but I expect during these events we should never be dependent on messages with our D3D usage.
Flags: needinfo?(bas)
(Assignee)

Comment 41

3 years ago
The try run on this looks good. There were some infra issues yesterday that appear to be the cause of messed up Win7 runs, so I've repushed to be sure.

https://tbpl.mozilla.org/?tree=Try&rev=32813e6ae7b2
(Assignee)

Comment 42

3 years ago
(In reply to Bas Schouten (:bas.schouten) from comment #40)
> (In reply to Jim Mathies [:jimm] from comment #37)
> > Lets ask Bas, see what he thinks. Bas, I'm not sure about message processing
> > and d2d apis. I know we pass a top level hwnd to d2d in some cases, and
> > obviously d2d is COM based which means internally Windows might block on
> > some sort of interprocess call that results in a message pump getting
> > called. bsmedberg is suggesting we turn message trapping off on both sides
> > of the omtc channel. Curious if this rings any alarm bells for you based on
> > your understanding of d2d's internal logic.
> 
> Currently we shouldn't ever be passing a windows directly to Direct2D
> anymore. We are still doing so for Direct3D, but I expect during these
> events we should never be dependent on messages with our D3D usage.

Thanks for the feedback!

Comment 43

3 years ago
Comment on attachment 8472658 [details] [diff] [review]
wip

Doesn't an entire protocol hierarchy use a single channel? So it should only be necessary to set the protocol flags in PluginModuleParent/Child when we set up the channel, and not in any of the subprotocols.
Attachment #8472658 - Flags: feedback?(benjamin)
(Assignee)

Comment 44

3 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #43)
> Comment on attachment 8472658 [details] [diff] [review]
> wip
> 
> Doesn't an entire protocol hierarchy use a single channel? So it should only
> be necessary to set the protocol flags in PluginModuleParent/Child when we
> set up the channel, and not in any of the subprotocols.

Really? I was under the impression each protocol, including sub protocols had their own channel. Interesting question, I'll investigate.
(Assignee)

Comment 45

3 years ago
(In reply to Jim Mathies [:jimm] from comment #44)
> (In reply to Benjamin Smedberg  [:bsmedberg] from comment #43)
> > Comment on attachment 8472658 [details] [diff] [review]
> > wip
> > 
> > Doesn't an entire protocol hierarchy use a single channel? So it should only
> > be necessary to set the protocol flags in PluginModuleParent/Child when we
> > set up the channel, and not in any of the subprotocols.
> 
> Really? I was under the impression each protocol, including sub protocols
> had their own channel. Interesting question, I'll investigate.

They do share a channel! This will simplify things quite a bit.
(Assignee)

Comment 46

3 years ago
Created attachment 8473107 [details] [diff] [review]
patch v.1

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=01494c2ed191
Attachment #8472658 - Attachment is obsolete: true
(Assignee)

Comment 47

3 years ago
Comment on attachment 8473107 [details] [diff] [review]
patch v.1

Try run looks good!
Attachment #8473107 - Flags: review?(benjamin)
Attachment #8473107 - Flags: feedback?(bent.mozilla)
(Assignee)

Comment 48

3 years ago
try run with annotations - 
https://tbpl.mozilla.org/?tree=Try&showall=0&rev=01494c2ed191

Updated

3 years ago
Attachment #8473107 - Flags: review?(benjamin) → review+
(Assignee)

Comment 49

3 years ago
Since there may be fallout from this, I'm going to wait until Monday to land on inbound.
(Assignee)

Comment 50

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce3f106abcd2
(Assignee)

Updated

3 years ago
Blocks: 1054744
Hrm, I think you've caused leaks in B2G ICS Emulator Debug. Can you take a look? I'll back it out on Monday morning IST if we're still having problems.
(Reporter)

Comment 52

3 years ago
(In reply to Nigel Babu [:nigelb] from comment #51)
> Hrm, I think you've caused leaks in B2G ICS Emulator Debug. Can you take a
> look? I'll back it out on Monday morning IST if we're still having problems.

I don't think this caused new leaks. Looking at a log from before and after this landed, the exact same number of objects are leaking. This patch just increased the size of some already-leaking data structures and pushed the total number of bytes over the allowable leak threshold for B2G. I'm going to just bump the limit accordingly. CCing Kyle to yell at me in case I got that wrong.

https://hg.mozilla.org/integration/mozilla-inbound/rev/356e4cb982fa
(Assignee)

Comment 53

3 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #52)
> (In reply to Nigel Babu [:nigelb] from comment #51)
> > Hrm, I think you've caused leaks in B2G ICS Emulator Debug. Can you take a
> > look? I'll back it out on Monday morning IST if we're still having problems.
> 
> I don't think this caused new leaks. Looking at a log from before and after
> this landed, the exact same number of objects are leaking. This patch just
> increased the size of some already-leaking data structures and pushed the
> total number of bytes over the allowable leak threshold for B2G. I'm going
> to just bump the limit accordingly. CCing Kyle to yell at me in case I got
> that wrong.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/356e4cb982fa

Thanks!
(Reporter)

Comment 54

3 years ago
https://hg.mozilla.org/mozilla-central/rev/ce3f106abcd2
https://hg.mozilla.org/mozilla-central/rev/356e4cb982fa

How easily could this (and whatever deps it has) be backported to earlier releases?
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
(Reporter)

Updated

3 years ago
status-b2g-v1.3: --- → wontfix
status-b2g-v1.3T: --- → wontfix
status-b2g-v1.4: --- → wontfix
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
status-firefox32: --- → affected
status-firefox33: --- → affected
status-firefox34: --- → fixed
status-firefox-esr24: --- → wontfix
status-firefox-esr31: --- → affected
(Assignee)

Comment 55

3 years ago
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #54)
> https://hg.mozilla.org/mozilla-central/rev/ce3f106abcd2
> https://hg.mozilla.org/mozilla-central/rev/356e4cb982fa
> 
> How easily could this (and whatever deps it has) be backported to earlier
> releases?

Deps would include bug 1044245 which is currently on mc and aurora. The work here replaced all of that work but we'd need 1044245 so this applies cleanly. I'd bet we could apply these pretty far back without issue.

We could also include bug 1052395 (better ipc debug output, currently on mc) and bug 1047842 (WM_GETTEXTLENGTH, currently on mc, aurora).
(Reporter)

Comment 56

3 years ago
I'll defer to your judgement here, but I'd love to get this backported as far back as esr31 if at all possible.
Thanks Ryan for the save :)
(Assignee)

Updated

3 years ago
Attachment #8473107 - Flags: feedback?(bent.mozilla)
(Assignee)

Updated

3 years ago
Flags: needinfo?(jmathies)
(Assignee)

Comment 58

3 years ago
Comment on attachment 8473107 [details] [diff] [review]
patch v.1

Approval Request Comment

[Feature/regressing bug #]:
OMTC enabled by default

[User impact if declined]:
Crashy browser, also dep bug 1021053 blocks OMTC on by default. This is currently the #3 top crasher on mc, #1 top crasher on aurora.

[Describe test coverage new/current, TBPL]:
In one nightly so far, no bugs filed afaict related to this, and crash stats indicates bug 1021053 has been addressed by this work.

[Risks and why]:
Medium, this involves an invasive change to how we handle ipc messaging on Windows.

[String/UUID change made/needed]:
none
Attachment #8473107 - Flags: approval-mozilla-aurora?
Flags: needinfo?(jmathies)
Attachment #8473107 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Jim, What is the status in beta? wontfix or disabled? Thanks
Flags: needinfo?(jmathies)
(Assignee)

Comment 60

3 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #59)
> Jim, What is the status in beta? wontfix or disabled? Thanks

no omtc, so wontfix. we want as much bake time as we can get.
Flags: needinfo?(jmathies)
(Assignee)

Updated

3 years ago
status-firefox32: affected → wontfix
(Assignee)

Comment 61

3 years ago
Created attachment 8475312 [details] [diff] [review]
patch merged to aurora (checkedin)

try push:

https://tbpl.mozilla.org/?tree=Try&rev=eb0e409c873e
(Assignee)

Comment 62

3 years ago
Created attachment 8475449 [details] [diff] [review]
rollup of patches (1009590, 1044245, 874437) for esr31

try push:
https://tbpl.mozilla.org/?tree=Try&rev=56dd17464ba5

This is primarily for tree management purposes if the try run comes up green.
(Reporter)

Comment 63

3 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/f6452d912168
status-b2g-v2.0: affected → wontfix
status-firefox33: affected → fixed
(Reporter)

Comment 64

3 years ago
Copy/paste error on Aurora. The proper rev was:
https://hg.mozilla.org/releases/mozilla-aurora/rev/61d81c3acd46
QA Whiteboard: [qa-]
(Reporter)

Comment 65

3 years ago
(In reply to Jim Mathies [:jimm] from comment #62)
> Created attachment 8475449 [details] [diff] [review]
> rollup of patches (1009590, 1044245, 874437) for esr31
> 
> try push:
> https://tbpl.mozilla.org/?tree=Try&rev=56dd17464ba5
> 
> This is primarily for tree management purposes if the try run comes up green.

Looks good to me. All the failures there were expected given esr31's lack of support for them.
(Assignee)

Comment 66

3 years ago
Comment on attachment 8475449 [details] [diff] [review]
rollup of patches (1009590, 1044245, 874437) for esr31

[Approval Request Comment]

If this is not a sec:{high,crit} bug, please state case for ESR consideration:

Tree management update requested by tree managers.

User impact if declined:

Tree manager frustration levels exceeding acceptable levels.

Fix Landed on Version:

fx 34, fx 33

Risk to taking this patch (and alternatives if risky): 

Try push indicates this is a safe landing. No issues found.
https://tbpl.mozilla.org/?tree=Try&rev=56dd17464ba5

String or UUID changes made by this patch: 

none.
Attachment #8475449 - Flags: approval-mozilla-esr31?
(Assignee)

Updated

3 years ago
Attachment #8475312 - Attachment description: patch merged to aurora → patch merged to aurora (checkedin)
I started the build of ESR yesterday. So, except if that it super important, this is too late for esr31.1.0 but we will take it for 31.2.0
Attachment #8475449 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
(Reporter)

Comment 68

3 years ago
https://hg.mozilla.org/releases/mozilla-esr31/rev/523f176d1820
status-firefox-esr31: affected → fixed
You need to log in before you can comment on or make changes to this bug.