502715 - Enable the canvas 2d context to function without an actual canvas element

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Rob Arnold [:robarnold]]

Windows 7 taskbar previews would like this. The surface needs to be backed by a windows surface so that we can get an HBITMAP out and the preview does not have access to the document (nor should it really be required).

Comment 1

[Rob Arnold [:robarnold]]

Attachment: v1.0

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 388274 [details] [diff] [review]
v1.0

mmmm. So I don't like just ignoring the security checks here.  If we don't have a canvas element, we should ensure at least that we have the system/chrome principal.

Also, looking at this, can we create mCSSParser lazily?  Maybe create a GetCSSParser() method that will create one if it's not created and use that instead of mCSSParser everywhere.

Comment 3

[Rob Arnold [:robarnold]]

(In reply to comment #2)
> (From update of attachment 388274 [details] [diff] [review])
> mmmm. So I don't like just ignoring the security checks here.  If we don't have
> a canvas element, we should ensure at least that we have the system/chrome
> principal.

How can we have a context without an element in a non-system environment?

> 
> Also, looking at this, can we create mCSSParser lazily?  Maybe create a
> GetCSSParser() method that will create one if it's not created and use that
> instead of mCSSParser everywhere.

Sounds good.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 388274 [details] [diff] [review]
v1.0

Ok, yeah, this is fine; I guess we can't have a no-content canvas without chrome privs.  Setting sr on roc just to make sure that's true...

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why are you asking me if it's true? I assume it's true just because InitializeWithSurface is non-scriptable.

+    if (!mCSSParser) {
+        mCSSParser = do_CreateInstance("@mozilla.org/content/css-parser;1");
+    }
+
+

Superfluous blank line

Can we have a GetPresShell helper function here to abstract over the if (mDocShell) { mDocShell->GetPresShell(); }?

Comment 6

[Rob Arnold [:robarnold]]

Attachment: v1.1

(In reply to comment #5)
> Superfluous blank line

Now removed.

> Can we have a GetPresShell helper function here to abstract over the if
> (mDocShell) { mDocShell->GetPresShell(); }?

Done. Code looks much cleaner. jst suggested that we call GetOwnerDoc instead GetCurrentDoc. The document from the pres shell should be the same as before this change.

Comment 7

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/d41cc3134424

Comment 8

[Rob Arnold [:robarnold]]

Reopened due to crashes.

Comment 9

[Rob Arnold [:robarnold]]

Attachment: v1.1.1

This patch actually makes the change I talked about (not making it was the cause of the crashes).

Comment 10

[Rob Arnold [:robarnold]]

Pushed to mozilla-central (again):
http://hg.mozilla.org/mozilla-central/rev/fe96f2059e54

Comment 11

[Takanori MATSUURA]

When compiling nsCanvasRenderingContextGL.cpp, I got the following error:

In file included from nsCanvasRenderingContextGL.h:56,
                 from nsCanvasRenderingContextGL.cpp:42:
../../../dist/include/nsICanvasRenderingContextInternal.h:65: error: 'nsIDocShell' has not been declared

I suppose to include nsIDocShell.h in nsICanvasRenderingContextInternal.h.

Comment 12

[Rob Arnold [:robarnold]]

Can you file a follow up bug for that? That fix seems fine at a glance.

Comment 13

[Takanori MATSUURA]

(In reply to comment #12)
Comment #11 has been filed as bug 508923.

After fixing bug 208923, I'm still in problem due to bug 507949.

Comment 14

[Takanori MATSUURA]

(In reply to comment #13)
> After fixing bug 208923, I'm still in problem due to bug 507949.

After fixing bug 508923, I'm still in problem due to bug 507949.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 392274 [details] [diff] [review]
v1.1.1


>+    /**
>+     * Gets the pres shell from either the canvas element or the doc shell
>+     */
>+    nsIPresShell *GetPresShell() {
>+      nsIPresShell *presShell = nsnull;
>+      nsCOMPtr<nsIContent> content = do_QueryInterface(mCanvasElement);
>+      if (content) {
>+          presShell = content->GetOwnerDoc()->GetPrimaryShell();
>+      } else if (mDocShell) {
>+          mDocShell->GetPresShell(&presShell);
>+      }
>+      return presShell;
>+    }
GetOwnerDoc() may return null, so this may crash, and
mDocShell->GetPresShell(...) addrefs so this leaks.