Closed Bug 978913 Opened 10 years ago Closed 10 years ago

[e10s] Dragging e10s window between HiDPI and non-HiDPI displays causes text to be scaled incorrectly


(Core :: Graphics: Text, defect)

Not set



Tracking Status
e10s + ---


(Reporter: cpeterson, Assigned: mconley)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

I have a Retina MacBook Pro with a non-HiDPI external display. When I drag an e10s window from the external display to the MacBook Pro's Retina display, the page text becomes very tiny. If I drag an e10s window from the Retina display to the external display, the page text becomes very large.
This can happen even with a non-e10s window, thanks to a bug in e10s code.  See bug 974616.
See Also: → 974616
Actually bug 974616 has now been fixed (by the patches for bug 974007).
I can still repro this bug with an e10s window in Nightly 30 build 2014-03-04. Non-e10s windows work correctly.
Are there any kind of notifications that fire when a window is dragged across monitors? Maybe we're just not passing the notification to the content process.
I can reproduce this bug as of 31.0a1 (2014-03-19).
tracking-e10s: --- → +
Assignee: nobody → mconley
The event that billm is referring to is windowDidChangeScreen, which is received by nsCocoaWindow, and fires mGeckoWindow->BackingScaleFactorChanged();.

BenWa helped me figure that one out, and we determined that the child process is definitely not having BackingScaleFactorChanged on it, so we might need to forward this to the child.
So here's what I've found out:

When an nsCocoaWindow hears windowDidChangeScreen, it calls its own BackingScaleFactorChanged function, which in turn calls nsPresContext::UIResolutionChanged for the associated nsPresContext, which runs this function in a runnable:

  mPendingUIResolutionChanged = false;

  if (mCurAppUnitsPerDevPixel != AppUnitsPerDevPixel()) {


That UIResolutionChangedSubdocumentCallback is this:

/*static*/ bool
nsPresContext::UIResolutionChangedSubdocumentCallback(nsIDocument* aDocument,
                                                      void* aData)
  nsIPresShell* shell = aDocument->GetShell();
  if (shell) {
    nsPresContext* pc = shell->GetPresContext();
    if (pc) {
  return true;

I think this is a spot (hopefully the right one) where we can call the TabParents for all of these documents, to signal to their TabChildren to call UIResolutionChanged on all of their nsIDocument's nsPresContexts.

So, the question is: is it possible to somehow go from each nsIDocument passed to this UIResolutionChangedSubdocumentCallback to their respective TabParents? If so, how?

billm, do you know?
Flags: needinfo?(wmccloskey)
I don't know anything helpful here. I don't see any easy way to find all the TabParents for a given window. The message manager keeps that sort of per-window data, but it doesn't look easy to get out what you need. Another option would be to enumerate over all TabParents and filter out the ones that are in the window that moved. That's not all that easy either though.

Maybe smaug has a better idea?
Flags: needinfo?(wmccloskey) → needinfo?(bugs)
Talked to smaug about this yesterday, and what I'm going to try is to get to the XUL document when the UIResolutionChanged method is called. From there, get to the "window" object, and then try to broadcast via window.messageManager to all of the remote browsers to update their nsPresContexts as well.

I'm seeing a lot of message manager use via from JS, but fewer examples in C++, but it _seems_ like it should be possible.
Clearing needinfo on smaug for now, based on above.
Flags: needinfo?(bugs)
Attached patch WIP Patch 1 (obsolete) — Splinter Review
So originally, I was trying to iterate the subdocuments inside nsPresContext::UIResolutionChangedSubdocumentCallback to trace them to their respective TabParents to send the UIResolutionChanged signal to the children.

Turns out that's really not easy - in fact, I found some other code that uses a hack to cast the callback passed to the nsFrameMessageManager in its Init call back into the nsFrameLoader, from which we can get to the TabParent.

That code is here:

My suggestion is to use the same hack, and then to file a new bug to make it easier to go from an nsIMessageListenerManager back to a nsFrameLoader.

That's what I'm doing here with this WIP, and I also have to invalidate some caches, because TabParent and TabChild both cache the DPI and default scales it seems.
Comment on attachment 8426663 [details] [diff] [review]
WIP Patch 1

Am I on the right track here?
Attachment #8426663 - Flags: feedback?(wmccloskey)
Comment on attachment 8426663 [details] [diff] [review]
WIP Patch 1

Review of attachment 8426663 [details] [diff] [review]:

This seems fine to me.

::: layout/base/nsPresContext.cpp
@@ +1771,5 @@
>    if (mCurAppUnitsPerDevPixel != AppUnitsPerDevPixel()) {
>      AppUnitsPerDevPixelChanged();
>    }
> +  if (XRE_GetProcessType() == GeckoProcessType_Default) {

Do we need this check? I didn't think we had a chrome window in the child process, but I could be wrong. Maybe you could just turn it into a MOZ_ASSERT.

@@ +1786,5 @@
> +        windowMM->GetChildAt(j, getter_AddRefs(childMM));
> +        if (!childMM) {
> +          continue;
> +        }
> +        nsCOMPtr<nsIMessageSender> strongTabMM = do_QueryInterface(childMM);

I think if you use strongTabMM.get() in the cast below, you won't need separate strong and weak variables.
Attachment #8426663 - Flags: feedback+
Attachment #8426663 - Attachment is obsolete: true
Attachment #8426663 - Flags: feedback?(wmccloskey)
Comment on attachment 8427090 [details] [diff] [review]
Dragging e10s window between HiDPI and non-HiDPI displays causes text to be scaled incorrectly. r=?

Thanks for the feedback+, billm!

Addressed feedback, and cleaned it up some. Ready for a real review.
Attachment #8427090 - Flags: review?(bugs)
Comment on attachment 8427090 [details] [diff] [review]
Dragging e10s window between HiDPI and non-HiDPI displays causes text to be scaled incorrectly. r=?

>+  virtual void ClearBackingScaleCache() {
Nit, { goes to its own line
Attachment #8427090 - Flags: review?(bugs) → review+
Ok, found the issue - needed to bring in nsFrameLoader.h for non-desktop builds, I guess.

Here's a try push:


Weird to have to request so many tests and systems to run, but, well, that's platform I guess.
Ok, I think it's landable now with that missing include.

Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Blocks: 1111957
Blocks: 1215659
You need to log in before you can comment on or make changes to this bug.