Rob Arnold [:robarnold] Assignee
	Description • 15 years ago

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).

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 2 • 15 years ago

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.

	Rob Arnold [:robarnold] Assignee
	Comment 3 • 15 years ago

(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.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 4 • 15 years ago

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...

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 5 • 15 years ago

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(); }?

	Rob Arnold [:robarnold] Assignee
	Comment 6 • 15 years ago

(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.

	Rob Arnold [:robarnold] Assignee
	Comment 7 • 15 years ago

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

	Rob Arnold [:robarnold] Assignee
	Comment 8 • 15 years ago

Reopened due to crashes.

	Rob Arnold [:robarnold] Assignee
	Comment 9 • 15 years ago

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

	Rob Arnold [:robarnold] Assignee
	Comment 10 • 15 years ago

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

	Takanori MATSUURA
	Comment 11 • 15 years ago

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.

	Rob Arnold [:robarnold] Assignee
	Comment 12 • 15 years ago

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

	Takanori MATSUURA
	Comment 13 • 15 years ago

(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.

	Takanori MATSUURA
	Comment 14 • 15 years ago

(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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 15 • 15 years ago

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.