I see lot of messages in child process: WARNING: nsWindow::GetNativeData not implemented for this type: PuppetWidget.cpp, line 633 with backtrace: #0 mozilla::widget::PuppetWidget::GetNativeData (this=0xb16dda20, aDataType=0) at widget/src/xpwidgets/PuppetWidget.cpp:633 #1 0xb57104e0 in nsDeviceContext::FindScreen (this=0xadefd5c0, outScreen=0xbfc5dd88) at gfx/src/nsDeviceContext.cpp:718 #2 0xb5710e3e in nsDeviceContext::ComputeClientRectUsingScreen (this=0xadefd5c0, outRect=0xbfc5de00) at gfx/src/nsDeviceContext.cpp:673 #3 0xb5710eec in nsDeviceContext::GetClientRect (this=0xadefd5c0, aRect=...) at gfx/src/nsDeviceContext.cpp:559 #4 0xb5765bef in MinimumFontSizeFor (aPresContext=<optimized out>, aContainerWidth=114600) at layout/base/nsLayoutUtils.cpp:4526 #5 0xb5769268 in nsLayoutUtils::FontSizeInflationFor (aReflowState=...) at layout/base/nsLayoutUtils.cpp:4763 #6 0xb57f1aa8 in nsHTMLReflowState::CalcLineHeight (this=0xbfc5e358) at layout/generic/nsHTMLReflowState.cpp:2198 pref("font.size.inflation.minTwips", 160); - is not null, so we requesting screen size for each Line height calculation... I guess we should somehow cache that value, or check in deviceContext NATIVE_WINDOW prop if we are in content process (puppet widget don't have that implemented for sure.) should it be fixed in layout/gfx or widget side?

GetClientRect is returning sensible results despite the warning; I'm not sure how relevant it is. That said, I should probably write some code to cache this result anyway.

P4 per font inflation scrub

This thing with a different stacktrace (bug 887581) is spamming the console quite massively. I am not sure if we need to ask the parent for the screen size, but as noted in comment #1 we actually get sensible results.

Can you take a look at this?

I think this needs whoever owns the Android widget code to look at it. The widget code's preferred mechanism to get the screen size ought not issue a warning.

Oh, but I guess there was comment 4; maybe Scott can look at that, if we haven't done it already?

Hey, can you take a look at this?

I can write a caching mechanism for this. Is there a specific testcase you want me to test against (i.e. reduction in number of warnings for a given case)?

That would be very nice! This basically ends up being called by window.screen.width when running with remote tabs.

Here's an initial patch for this. However, I'm not 100% confident that the screen rect won't change. Specifically, if we're on a multiple-screen system (e.g. windows/gtk/osx), it's possible that the screen size could change if a native window were moved from one screen to another. I don't know how relevant this is for font inflation, though, since we're only going to be using it on Android/B2G, where the number of screens is limited to 1. We will be using font inflation on desktop for tests, but, again, I'm not sure this is relevant. jfkthame, I was wondering if you could give me some idea of whether we should be more dynamic about changing this if the screen changes? I'd have to do something like caching the screen in nsPresContext, and then comparing it to the result of nsDeviceContext::FindScreen() to determine if we need to recompute the screen client rect. I would rather avoid this, though, since, as I said, it's probably not necessary, and will add a bit of complexity/inefficiency to the solution.

Sounds to me like you can probably ignore the case of moving the window between screens, at least for now. But won't the screen rect change dynamically on android/b2g any time the device orientation changes?

(In reply to Jonathan Kew (:jfkthame) from comment #12) > Sounds to me like you can probably ignore the case of moving the window > between screens, at least for now. But won't the screen rect change > dynamically on android/b2g any time the device orientation changes? Hm, yes, you're right. I'm not sure where this gets changed, though, other than in the Java code. Do you happen to know offhand where in the platform this is stored/modified? I thought it would be handled by nsScreen::Notify(), but that doesn't seem to be called when the screen orientation changes on android.

This is the new version with code that recomputes the cached rect when the screen size changes (specifically, I tested when the screen orientation changes on android).

I'm curious why you need to hook into nsDOMWindowUtils::SetCSSViewport here; that feels kinda hacky to me. There's code that looks (at a cursory glance) as though it's supposed to dispatch events to gecko when the screen orientation changes, and end up notifying the screen manager. Is there something broken about that? (Are we forgetting to call EnableScreenConfigurationNotifications() somewhere appropriate, so that mShouldNotify in GeckoScreenOrientationListener.java never gets turned on? Offhand, I can't seem to find it in mxr....)

Comment on attachment 810040 [details] [diff] [review] b706765 Clearing r? for now, until someone picks this bug up again; the questions in comment 15 still stand, afaik.

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".

I think mconely fixed this with his screen manager work.

I've certainly fixed at least one of these cases (nsDeviceContext was attempting to get NS_NATIVE_WIDGET via GetNativeData), but not all (I'm still seeing this warning periodically in my console spew, even with my screen manager patches). Anything that attempts to query PuppetWidget for: NS_NATIVE_WINDOW NS_NATIVE_DISPLAY NS_NATIVE_PLUGIN_PORT NS_NATIVE_GRAPHIC NS_NATIVE_SHELLWIDGET NS_NATIVE_WIDGET is going to display this message. We might be able to help drill into this by having the warning also give the value being passed to GetNativeData, which might help find the caller.

Some of these warnings were investigated and silenced in bug 1243599 and bug 1183828. The remaining values kept the warning in place to pick up future bugs. I think it's safe to close this out. The original bug here appears to be obsolete as well. It was filed back in 2011.

(In reply to Jim Mathies [:jimm] from comment #21) > Some of these warnings were investigated and silenced in bug 1243599 and bug > 1183828. The remaining values kept the warning in place to pick up future > bugs. I think it's safe to close this out. > > The original bug here appears to be obsolete as well. It was filed back in > 2011. s/bug 1243599/bug 1240891