Insert default stylesheet for platform apps.

Insert default stylesheet for platform apps.

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

[miket_OOO, 2011-11-11 18:06:48]

Aaron, thanks for your comments on http://codereview.chromium.org/8447017/. If I
read your comments correctly, the approach wasn't sound, so I've tossed it and
started anew. Let me know what you think, and if it feels right, I'll turn it
into a real CL.

[DRAFT - not suitable for checkin or review]

[Aaron Boodman, 2011-11-11 20:05:59]

Yes, seems right.

Consider modifying ViewMsg_ExtensionLoaded to accept a set of extensions rather
than just one. Then I think you won't need the batchstart/batchend logic.

[Aaron Boodman, 2011-11-11 20:06:13]

http://codereview.chromium.org/8542001/diff/1/chrome/common/extensions/extens...
File chrome/common/extensions/extension_set_unittest.cc (right):

http://codereview.chromium.org/8542001/diff/1/chrome/common/extensions/extens...
chrome/common/extensions/extension_set_unittest.cc:1: 
whoops

[miket_OOO, 2011-11-14 21:08:59]

Fixed the oops, and changed ExtensionMsg_Loaded to take a batch. Please
re-review (running unit tests now after successful manual testing).

[Aaron Boodman, 2011-11-14 22:48:07]

LGTM w/ nits

+mpcomplete for help with IPC

http://codereview.chromium.org/8542001/diff/6012/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_service.cc:2325: // communicated to
renderers.
Nit: seems like change is unnecessary.

http://codereview.chromium.org/8542001/diff/6012/chrome/common/extensions/ext...
File chrome/common/extensions/extension_messages.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/common/extensions/ext...
chrome/common/extensions/extension_messages.cc:32: ExtensionMsg_Loaded_Params&
ExtensionMsg_Loaded_Params::operator=(
Hm, yeah, it does seem weird/unfortunate to need both of these.

http://codereview.chromium.org/8542001/diff/6012/chrome/renderer/extensions/e...
File chrome/renderer/extensions/extension_dispatcher.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/renderer/extensions/e...
chrome/renderer/extensions/extension_dispatcher.cc:212: // TODO: consider
enhancing WebView to allow removing single stylesheets,
The convention is TODO(<whoever knows the most about this TODO>), so in this
case, TODO(miket).

[miket_OOO, 2011-11-14 23:48:50]

I think this is the one. Thanks, everyone.

http://codereview.chromium.org/8542001/diff/6012/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_service.cc:2325: // communicated to
renderers.
On 2011/11/14 22:48:07, Aaron Boodman wrote:
> Nit: seems like change is unnecessary.

OK. Exactly 80 columns is ugly in Emacs, that's all. I put it back.

http://codereview.chromium.org/8542001/diff/6012/chrome/common/extensions/ext...
File chrome/common/extensions/extension_messages.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/common/extensions/ext...
chrome/common/extensions/extension_messages.cc:32: ExtensionMsg_Loaded_Params&
ExtensionMsg_Loaded_Params::operator=(
On 2011/11/14 22:48:07, Aaron Boodman wrote:
> Hm, yeah, it does seem weird/unfortunate to need both of these.

See #crx. mpcomplete suggested using linked_ptr, and indeed it cleaned up the
whole thing.

http://codereview.chromium.org/8542001/diff/6012/chrome/renderer/extensions/e...
File chrome/renderer/extensions/extension_dispatcher.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/renderer/extensions/e...
chrome/renderer/extensions/extension_dispatcher.cc:212: // TODO: consider
enhancing WebView to allow removing single stylesheets,
Done.

[Aaron Boodman, 2011-11-14 23:55:09]

lgtm, thanks matt

http://codereview.chromium.org/8542001/diff/6012/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_service.cc (right):

http://codereview.chromium.org/8542001/diff/6012/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_service.cc:2325: // communicated to
renderers.
On 2011/11/14 23:48:51, miket wrote:
> On 2011/11/14 22:48:07, Aaron Boodman wrote:
> > Nit: seems like change is unnecessary.
> 
> OK. Exactly 80 columns is ugly in Emacs, that's all. I put it back.

Make window wider? :)

[Mihai Parparita -not on Chrome, 2011-11-15 00:48:28]

LGTM

http://codereview.chromium.org/8542001/diff/11001/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_dispatcher.cc (right):

http://codereview.chromium.org/8542001/diff/11001/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_dispatcher.cc:187: std::vector<WebString>
temp_patterns;
Nit: Can this be called platform_app_patterns or something else more specific?

http://codereview.chromium.org/8542001/diff/11001/chrome/renderer/resources/e...
File chrome/renderer/resources/extensions/platform_app.css (right):

http://codereview.chromium.org/8542001/diff/11001/chrome/renderer/resources/e...
chrome/renderer/resources/extensions/platform_app.css:5: @namespace
"http://www.w3.org/1999/xhtml";
Whoa, did not know about @namespace.

[commit-bot: I haz the power, 2011-11-15 01:02:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miket@chromium.org/8542001/8003

[commit-bot: I haz the power, 2011-11-15 02:17:55]

Change committed as 110017