Closed Bug 1082127 Opened 10 years ago Closed 10 years ago

[e10s] window.moveTo and window.resizeTo not implemented

Categories

(Core :: DOM: Content Processes, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
e10s m5+ ---

People

(Reporter: billm, Assigned: gw280)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Same goes for window.resizeTo

I commented out the window.moveTo line in talos/startup_test/tresize-test.html, and then it failed on on next line of window.resizeTo with the same error:

[Exception... "Method not implemented"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: http://localhost:15707/startup_test/tresize-test.html :: <TOP_LEVEL> :: line 88"  data: no]
Summary: [e10s] window.moveTo not implemented → [e10s] window.moveTo and window.resizeTo not implemented
I see this is m5+, I assume there are no short term plans to fix this issue.
Flags: needinfo?(wmccloskey)
We're on m4 right now - so m5 is coming up.
oh cool
Flags: needinfo?(wmccloskey)
No longer blocks: 1050706
This bug no longer blocks talos-tresize on e10s. See bug 1050706 comment 89 for more details.

In a nutshell, tresize should resize the window from the patent process because it's closer to the user resizing the window manually.
I guess this bug will be addressed before e10s ships to release?
It's affecting our web app (trading application, traders like to popout components which we resize).
(In reply to Brian Di Palma from comment #6)
> I guess this bug will be addressed before e10s ships to release?
> It's affecting our web app (trading application, traders like to popout
> components which we resize).

Yes - this is a web-compat issue, and will be fixed before e10s ships to release.
Assignee: nobody → gwright
Comment on attachment 8547858 [details] [diff] [review]
0001-Bug-1082127-Implement-TabChild-SetDimensions-so-that.patch

mett.
Attachment #8547858 - Flags: review?(davidp99) → review?(matt.woodrow)
Comment on attachment 8547858 [details] [diff] [review]
0001-Bug-1082127-Implement-TabChild-SetDimensions-so-that.patch

Review of attachment 8547858 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a widget/dom peer sorry.
Attachment #8547858 - Flags: review?(matt.woodrow)
Comment on attachment 8547858 [details] [diff] [review]
0001-Bug-1082127-Implement-TabChild-SetDimensions-so-that.patch

I think :smaug should be able to review this then?

I'm currently looking into tests for this, specifically why test_resize_move_windows.html wasn't being triggered for this.
Attachment #8547858 - Flags: review?(bugs)
So I've identified a couple of issues with this patch. It looks like (independent of what's going on here), the "size" of a window in e10s is different to that of non-e10s windows. I'll file a bug about that separately from this one.

The other issue is that we don't actually get a resize call back to TabChild from TabParent like I thought we would, but that's easily remedied by a quick call to SendUpdateDimensions in TabParent. I'm not a huge fan of the ping-ponging, but :handyman thinks it makes sense and I think it probably does too.
Depends on: 1121791
Comment on attachment 8547858 [details] [diff] [review]
0001-Bug-1082127-Implement-TabChild-SetDimensions-so-that.patch

So on parent side I would pass the params to the treeowner the same way as
what happens in nsGlobalWindow

  NS_ENSURE_TRUE(mFrameElement, true);
  nsCOMPtr<nsIDocShell> docShell = mFrameElement->OwnerDoc()->GetDocShell();
  NS_ENSURE_TRUE(docShell, true);
  nsCOMPtr<nsIDocShellTreeOwner> treeOwner;
  docShell->GetTreeOwner(getter_AddRefs(treeOwner));
  nsCOMPtr<nsIBaseWindow> treeOwnerAsWin = do_QueryInterface(treeOwner);
  NS_ENSURE_TRUE(treeOwnerAsWin, true);
  and then do call the methods on treeOwnerAsWin


That way we'd store the persistent attributes related to window size etc.
and would use closer the same code paths what nsGlobalWindow uses.
Attachment #8547858 - Flags: review?(bugs) → review-
Mostly just took smaug's suggestion here. We can't call SetPositionAndSize directly as we need to check aFlags to ensure we only set position or size (we often get, e.g. SetDimensions with a size of 0x0).

Tests we can't enable yet because of bug 1121791 and bug 1075670.
Attachment #8547858 - Attachment is obsolete: true
Attachment #8550418 - Flags: review?(bugs)
Comment on attachment 8550418 [details] [diff] [review]
0001-Bug-1082127-Implement-TabChild-SetDimensions-so-that.patch

>+TabParent::RecvSetDimensions(const uint32_t& aFlags,
>+                             const int32_t& aX, const int32_t& aY,
>+                             const int32_t& aCx, const int32_t& aCy)
>+{
>+  nsCOMPtr<nsIWidget> widget = GetWidget();
You don't need widget for anything.

>+  NS_ENSURE_TRUE(mFrameElement, true);
>+  nsCOMPtr<nsIDocShell> docShell = mFrameElement->OwnerDoc()->GetDocShell();
>+  NS_ENSURE_TRUE(docShell, true);
>+  nsCOMPtr<nsIDocShellTreeOwner> treeOwner;
>+  docShell->GetTreeOwner(getter_AddRefs(treeOwner));
>+  nsCOMPtr<nsIBaseWindow> treeOwnerAsWin = do_QueryInterface(treeOwner);
>+  NS_ENSURE_TRUE(treeOwnerAsWin, true);
>+
>+  if (aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_POSITION) {
>+    treeOwnerAsWin->SetPosition(aX, aY);
>+  }
>+
>+  if (aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_SIZE_INNER ||
Looks like DIM_FLAGS_SIZE_INNER isn't actually used ever.
Perhaps better to just add MOZ_ASSERT that aFlags never contains it.


>+      aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_SIZE_OUTER) {
>+    treeOwnerAsWin->SetSize(aCx, aCy, true);
>+  }


Based on http://mxr.mozilla.org/mozilla-central/source/embedding/browser/nsDocShellTreeOwner.cpp#530
we might want to deal explicitly the case when both DIM_FLAGS_POSITION and DIM_FLAGS_SIZE_OUTER are set.

So perhaps something like.
  if ((aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_POSITION) &&
      (aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_SIZE_OUTER)) {
    treeOwnerAsWin->SetPositionAndSize...
    return true;
  }

  if (aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_POSITION) {
    treeOwnerAsWin->SetPosition(aX, aY);
    return true;
  }

  if (aFlags & nsIEmbeddingSiteWindow::DIM_FLAGS_SIZE_OUTER) {
    treeOwnerAsWin->SetSize(aCx, aCy, true);
    return true;
  }
MOZ_ASSERT(false, "unknown flags!");
return true;
Attachment #8550418 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/50981656f6b7
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Depends on: 1258925
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: