Closed Bug 572214 Opened 12 years ago Closed 12 years ago

[e10s] Add support for remote and local rendering

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(2 files)

To support rendering remote content, Fennec needs to support the new canvas.MozGetIPCContext and asyncDrawXULElement.

This patch gets basic rendering working in both mozilla-central and electrolysis trees using mobile-browser. We still have more work to do to get things working without contentWindow/docShell/contentDocument in e10s, but this is a start.

This patch also adds "browser.tabs.remote" pref (defaulted to false for now). To test this patch in e10s, change it to true.

Next steps will include moving the panning/scrolling code to content process (different bug).
Attached patch patchSplinter Review
Oops, attachment failed last time
Assignee: nobody → mark.finkle
Attachment #451389 - Flags: review?(21)
Comment on attachment 451389 [details] [diff] [review]
patch

>-      bv.browserToViewportCanvasContext(ctx);
>-      ctx.scale(scalex, scaley);
>+    if (bv._contentWindow) {
>       ctx.drawWindow(bv._contentWindow,
>                      srcRect.left, srcRect.top, srcRect.width, srcRect.height,
>                      "white",
>                      (ctx.DRAWWINDOW_DO_NOT_FLUSH | ctx.DRAWWINDOW_DRAW_CARET));
>+    } else {
>+      ctx.asyncDrawXULElement(bv._browser,
>+                     srcRect.left, srcRect.top, srcRect.width, srcRect.height,
>+                     "white",
>+                     (ctx.DRAWWINDOW_DO_NOT_FLUSH | ctx.DRAWWINDOW_DRAW_CARET));
>+    }

I dislike duplication the source code here since the arguments are the same. Can't we just assign the function ato a var and call it?


>+
>+    if (browserView._contentWindow) {
>+      // We expand the rect to working around a gfx issue (bug 534054)
>+      ctx.drawWindow(browserView._contentWindow,
>+                     rect.left , rect.top,
>+                     rect.right - rect.left + 1, rect.bottom - rect.top + 1,
>+                     "white",
>+                     (ctx.DRAWWINDOW_DRAW_CARET));
>+    } else {
>+      try {
>+        // We expand the rect to working around a gfx issue (bug 534054)
>+        ctx.asyncDrawXULElement(browserView._browser,
>+                       rect.left , rect.top,
>+                       rect.right - rect.left + 1, rect.bottom - rect.top + 1,
>+                       "white",
>+                       (ctx.DRAWWINDOW_DRAW_CARET));
>+      } catch (e) {
>+        dump(e + "\n")
>+      }
>+    }

Same as previous and we could remove the try/catch

Also i assume we could safely remove some Tiles optimizations because Layers are coming right?

r+ with questions adressed.
Attachment #451389 - Flags: review?(21) → review+
(In reply to comment #2)
> (From update of attachment 451389 [details] [diff] [review])
> >-      bv.browserToViewportCanvasContext(ctx);
> >-      ctx.scale(scalex, scaley);
> >+    if (bv._contentWindow) {
> >       ctx.drawWindow(bv._contentWindow,
> >                      srcRect.left, srcRect.top, srcRect.width, srcRect.height,
> >                      "white",
> >                      (ctx.DRAWWINDOW_DO_NOT_FLUSH | ctx.DRAWWINDOW_DRAW_CARET));
> >+    } else {
> >+      ctx.asyncDrawXULElement(bv._browser,
> >+                     srcRect.left, srcRect.top, srcRect.width, srcRect.height,
> >+                     "white",
> >+                     (ctx.DRAWWINDOW_DO_NOT_FLUSH | ctx.DRAWWINDOW_DRAW_CARET));
> >+    }
> 
> I dislike duplication the source code here since the arguments are the same.
> Can't we just assign the function ato a var and call it?

I can try. Note that the first parameter is different.

> >+      try {
> >+        // We expand the rect to working around a gfx issue (bug 534054)
> >+        ctx.asyncDrawXULElement(browserView._browser,

> >+      } catch (e) {
> 
> Same as previous and we could remove the try/catch

I have seen asyncDrawXULElement throw exceptions. I guess Ben did too, since he put the  try/catch there in mobile-e10s. I'll remove it and see what happens.

> Also i assume we could safely remove some Tiles optimizations because Layers
> are coming right?

Correct. Also, they can be difficult to get working in e10s anyway
Patch with review comments addressed. The duplicate code was moved into a helper wrapper.
pushed to m-b:
http://hg.mozilla.org/mobile-browser/rev/a4a753b705f1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.