Re-enable extension GPU features

Re-enable extension GPU features.

Re-enabling GPU-related features for extension pages, incl. background pages.

One new ExtensionBrowserTest
ExtensionBrowserTest.Canvas2D: tests using Canvas2D from an extension background page

I had also created a second test, but subsequently removed it due to failures on linux_chromeos trybots that don't reproduce locally:
ExtensionBrowserTest.BackgroundGL: tests creating a WebGL context from extension background page

BUG=chromium-os:26524
TEST=browser_tests ExtensionBrowserTest.*
TEST=Run Chat for Google extension with PPAPI gtalk/o3d


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=125966

[Josh Horwich, 2012-03-08 22:27:42]

Please take a look.

I discussed with saintlou@, he wanted to keep this confined to aura builds as
well for now.

We really only need to re-enable accelerated_compositing_enabled for the bug, so
please let me know if I should only change that one.

I also removed the BackgroundGL test mentioned in the original description for
now since I don't believe it runs on linux_chromeos VM bots, despite working
fine on my workstation.

[jamesr, 2012-03-08 23:41:44]

I think it is unwise to restrict this to aura - that's basically saying we want
a behavior change that we do not think is tied to any Aura behavior, but we
don't want to give it the test coverage that all of the non-Aura bots,
infrastructure, and users can provide.

[Emmanuel Saint-loubert-Bié, 2012-03-08 23:55:41]

James, 

I understand your concerns, however Aura is what is driving this change in the
first place. At this point the top priority is to get GTalk working on Aura on
R19. 

I do not feel comfortable to enable this on all configs at this stage. Which is
why I told Josh to limit it to Aura for now.

-- Emmanuel

[jamesr, 2012-03-08 23:58:15]

On 2012/03/08 23:55:41, Emmanuel Saint-loubert wrote:
> James, 
> 
> I understand your concerns, however Aura is what is driving this change in the
> first place. At this point the top priority is to get GTalk working on Aura on
> R19. 
> 
> I do not feel comfortable to enable this on all configs at this stage. Which
is
> why I told Josh to limit it to Aura for now.

I think the change is highly unlikely to be correct on Aura and incorrect on
non-Aura. It probably either is correct everywhere or is incorrect everywhere. 
If it's incorrect everywhere, then I think it's a lot safer to get as much
exposure as possible on it to figure out what is broken and how rather than
shipping something underbaked to Aura.

> 
> -- Emmanuel

[Emmanuel Saint-loubert-Bié, 2012-03-09 00:14:20]

I see your point James. We will change our approach then.

[Josh Horwich, 2012-03-09 00:15:35]

OK, I can try it without the AURA guards.
One question though, would it be preferable to turn back on all three features,
or should I just turn on accelerated_compositing at this point in the cycle?  
The bug in particular only cares about accelerated_compositing but perhaps we'd
want to enable WebGL use / accelerated_canvas too. please let me know.

[Aaron Boodman, 2012-03-09 00:56:30]

Ok, this is one way to solve the problem :)

extension changes lgtm

[Aaron Boodman, 2012-03-09 01:00:06]

spoke too soon

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/browser/exte...
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/browser/exte...
chrome/browser/extensions/extension_browsertests_misc.cc:701:
IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Canvas2D) {
Did you see the note at the bottom of the file (don't blame you if you didn't,
the file is huge)? You should put this test in
chrome/browser/extensions/background_page_apitest.cc. See
background_scripts_apitest.cc for an example.

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/test/data/ex...
File chrome/test/data/extensions/canvas/background.html (right):

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/test/data/ex...
chrome/test/data/extensions/canvas/background.html:1: <html>
Add a license header.

[Josh Horwich, 2012-03-09 01:21:56]

A quick question for Aaron

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/browser/exte...
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/browser/exte...
chrome/browser/extensions/extension_browsertests_misc.cc:701:
IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Canvas2D) {
On 2012/03/09 01:00:06, Aaron Boodman wrote:
> Did you see the note at the bottom of the file (don't blame you if you didn't,
> the file is huge)? You should put this test in
> chrome/browser/extensions/background_page_apitest.cc. See
> background_scripts_apitest.cc for an example.

Didn't see the note.

Do you mean app_background_page_apitest.cc or do you mean creating a new file
(background_page_apitest.cc) for this one test?

Either way I can switch it over to using the techniques used in that example,
thank you for the pointer.

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/test/data/ex...
File chrome/test/data/extensions/canvas/background.html (right):

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/test/data/ex...
chrome/test/data/extensions/canvas/background.html:1: <html>
On 2012/03/09 01:00:06, Aaron Boodman wrote:
> Add a license header.

Will do

[Aaron Boodman, 2012-03-09 01:45:20]

On Thu, Mar 8, 2012 at 5:21 PM,  <jhorwich@chromium.org> wrote:
> Didn't see the note.
>
> Do you mean app_background_page_apitest.cc or do you mean creating a new
> file (background_page_apitest.cc) for this one test?
>
> Either way I can switch it over to using the techniques used in that
> example, thank you for the pointer.

I meant a new file. There's no existing api tests for the
background.page feature. The app_background_page thing is a different
feature.

- a

[Josh Horwich, 2012-03-09 02:25:01]

On 2012/03/09 01:45:20, Aaron Boodman wrote:
> On Thu, Mar 8, 2012 at 5:21 PM,  <mailto:jhorwich@chromium.org> wrote:
> > Didn't see the note.
> >
> > Do you mean app_background_page_apitest.cc or do you mean creating a new
> > file (background_page_apitest.cc) for this one test?
> >
> > Either way I can switch it over to using the techniques used in that
> > example, thank you for the pointer.
> 
> I meant a new file. There's no existing api tests for the
> background.page feature. The app_background_page thing is a different
> feature.
> 
> - a

Understood - I added it to background_page_apitest.cc and used the test
framework similarly to the example provided. thank you!

[Josh Horwich, 2012-03-09 02:26:25]

Please take a look.

I also removed a now-incorrect test for no-WebGL-for-background-pages as well.

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/browser/exte...
File chrome/browser/extensions/extension_browsertests_misc.cc (right):

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/browser/exte...
chrome/browser/extensions/extension_browsertests_misc.cc:701:
IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, Canvas2D) {
On 2012/03/09 01:00:06, Aaron Boodman wrote:
> Did you see the note at the bottom of the file (don't blame you if you didn't,
> the file is huge)? You should put this test in
> chrome/browser/extensions/background_page_apitest.cc. See
> background_scripts_apitest.cc for an example.

Done.

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/test/data/ex...
File chrome/test/data/extensions/canvas/background.html (right):

https://chromiumcodereview.appspot.com/9624013/diff/14001/chrome/test/data/ex...
chrome/test/data/extensions/canvas/background.html:1: <html>
On 2012/03/09 01:21:56, Josh Horwich wrote:
> On 2012/03/09 01:00:06, Aaron Boodman wrote:
> > Add a license header.
> 
> Will do

Done.

[Rick Byers, 2012-03-09 18:15:02]

I believe accelerated compositing was initially disabled on background pages (by
sadrul@) as a work-around for a hang that totally broke offline gmail - see
crosbug.com/19915.  The real solution is claimed to be fixed in crbug.com/96005,
but I'm not convinced we ever fully understood the cause.

Before landing this change, can you please briefly test offline gmail?  Install
it from the webstore, start it up and let it sync some e-mail,  then restart
chrome (verify the background page is running by looking at the task manager)
and open an offline gmail tab.  Just verify the renderer doesn't hang.

Thanks!
   Rick

[Josh Horwich, 2012-03-09 20:14:22]

Rick,
  I did the steps you described (and verified the tab for the offline "GMail -
Inbox" page ended up in the same renderer process as "Google Offline Mail
Background Page" via chrome://memory-redirect), and did not notice a renderer
hang.

- Josh

On 2012/03/09 18:15:02, Rick Byers wrote:
> I believe accelerated compositing was initially disabled on background pages
(by
> sadrul@) as a work-around for a hang that totally broke offline gmail - see
> crosbug.com/19915.  The real solution is claimed to be fixed in
crbug.com/96005,
> but I'm not convinced we ever fully understood the cause.
> 
> Before landing this change, can you please briefly test offline gmail? 
Install
> it from the webstore, start it up and let it sync some e-mail,  then restart
> chrome (verify the background page is running by looking at the task manager)
> and open an offline gmail tab.  Just verify the renderer doesn't hang.
> 
> Thanks!
>    Rick

[Rick Byers, 2012-03-09 20:22:48]

On 2012/03/09 20:14:22, Josh Horwich wrote:
> Rick,
>   I did the steps you described (and verified the tab for the offline "GMail -
> Inbox" page ended up in the same renderer process as "Google Offline Mail
> Background Page" via chrome://memory-redirect), and did not notice a renderer
> hang.
> 
> - Josh

Thank you very much Josh!  This issue was a major pain for offline gmail issues,
glad to hear it looks like the underlying problem is indeed actually resolved.

[commit-bot: I haz the power, 2012-03-09 21:46:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jhorwich@chromium.org/9624013/21001

[commit-bot: I haz the power, 2012-03-10 01:02:39]

Change committed as 125966