Allow webview API in an unblessed extension process

Allow webview API in an unblessed extension process

BUG=293724
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225493

[guohui, 2013-09-19 18:15:26]

Hey,

could you please take a look at the CL?

For motivation, please refer to the attached bug 293724.

Thanks,

Hui

[guohui, 2013-09-19 18:15:28]

Hey,

could you please take a look at the CL?

For motivation, please refer to the attached bug 293724.

Thanks,

Hui

[Fady Samuel, 2013-09-19 18:50:49]

https://codereview.chromium.org/24243007/diff/1/chrome/renderer/extensions/di...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/24243007/diff/1/chrome/renderer/extensions/di...
chrome/renderer/extensions/dispatcher.cc:1118: // the <webview> tag, because the
"webView" and "denyWebView" modules will
This is only currently true of denyWebview now as webview is a custom element.

https://codereview.chromium.org/24243007/diff/1/chrome/renderer/extensions/di...
chrome/renderer/extensions/dispatcher.cc:1120: //  We used to limit WebView to
|BLESSED_EXTENSION_CONTEXT| within platform
Remove extra space before We.

[guohui, 2013-09-19 19:32:29]

https://codereview.chromium.org/24243007/diff/1/chrome/renderer/extensions/di...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/24243007/diff/1/chrome/renderer/extensions/di...
chrome/renderer/extensions/dispatcher.cc:1118: // the <webview> tag, because the
"webView" and "denyWebView" modules will
On 2013/09/19 18:50:49, Fady Samuel wrote:
> This is only currently true of denyWebview now as webview is a custom element.

Done.

https://codereview.chromium.org/24243007/diff/1/chrome/renderer/extensions/di...
chrome/renderer/extensions/dispatcher.cc:1120: //  We used to limit WebView to
|BLESSED_EXTENSION_CONTEXT| within platform
On 2013/09/19 18:50:49, Fady Samuel wrote:
> Remove extra space before We.

Done.

[Fady Samuel, 2013-09-19 19:52:14]

https://codereview.chromium.org/24243007/diff/9001/chrome/renderer/extensions...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/24243007/diff/9001/chrome/renderer/extensions...
chrome/renderer/extensions/dispatcher.cc:1118: // the <webview> tag, because the
"denyWebView" modules will affect the
s/modules/module.

[guohui1, 2013-09-19 20:04:48]

https://codereview.chromium.org/24243007/diff/9001/chrome/renderer/extensions...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/24243007/diff/9001/chrome/renderer/extensions...
chrome/renderer/extensions/dispatcher.cc:1118: // the <webview> tag, because the
"denyWebView" modules will affect the
On 2013/09/19 19:52:14, Fady Samuel wrote:
> s/modules/module.

Done.

[Charlie Reis, 2013-09-19 21:41:03]

Seems right to me, once we verify that this is a safe thing to do.

[guohui1, 2013-09-19 21:45:15]

On 2013/09/19 21:41:03, creis wrote:
> Seems right to me, once we verify that this is a safe thing to do.

hmm, so how should we do that?

[Charlie Reis, 2013-09-19 22:05:40]

On 2013/09/19 21:45:15, guohui1 wrote:
> On 2013/09/19 21:41:03, creis wrote:
> > Seems right to me, once we verify that this is a safe thing to do.
> 
> hmm, so how should we do that?

That's the security review I mentioned earlier.  It's on my plate.

[not at google - send to devlin, 2013-09-20 15:59:21]

On 2013/09/19 21:45:15, guohui1 wrote:
> On 2013/09/19 21:41:03, creis wrote:
> > Seems right to me, once we verify that this is a safe thing to do.
> 
> hmm, so how should we do that?

Also: lots of tests. This makes me nervous.

[Fady Samuel, 2013-09-20 16:05:34]

On 2013/09/20 15:59:21, kalman wrote:
> On 2013/09/19 21:45:15, guohui1 wrote:
> > On 2013/09/19 21:41:03, creis wrote:
> > > Seems right to me, once we verify that this is a safe thing to do.
> > 
> > hmm, so how should we do that?
> 
> Also: lots of tests. This makes me nervous.

+1, although, I did put some thought into making sure there are the appropriate
security checks in the chrome layer of the browser process when I moved the
<webview> API out of content and into chrome over the last few months. In fact,
security checks came up multiple times in design discussions as these were
intended to prevent an embedder from mucking with <adview> ads by using the
<webview> API. Hopefully, I didn't miss anything glaringly obvious.

[not at google - send to devlin, 2013-09-20 16:41:27]

On 2013/09/20 16:05:34, Fady Samuel wrote:
> On 2013/09/20 15:59:21, kalman wrote:
> > On 2013/09/19 21:45:15, guohui1 wrote:
> > > On 2013/09/19 21:41:03, creis wrote:
> > > > Seems right to me, once we verify that this is a safe thing to do.
> > > 
> > > hmm, so how should we do that?
> > 
> > Also: lots of tests. This makes me nervous.
> 
> +1, although, I did put some thought into making sure there are the
appropriate
> security checks in the chrome layer of the browser process when I moved the
> <webview> API out of content and into chrome over the last few months. In
fact,
> security checks came up multiple times in design discussions as these were
> intended to prevent an embedder from mucking with <adview> ads by using the
> <webview> API. Hopefully, I didn't miss anything glaringly obvious.

Ok thanks Fady. I would have expected some tests to need updating though; so I
would at least test (or make sure it's tested) in a web page, blessed, and from
within webui.

[guohui, 2013-09-23 19:00:37]

On 2013/09/20 16:41:27, kalman wrote:
> On 2013/09/20 16:05:34, Fady Samuel wrote:
> > On 2013/09/20 15:59:21, kalman wrote:
> > > On 2013/09/19 21:45:15, guohui1 wrote:
> > > > On 2013/09/19 21:41:03, creis wrote:
> > > > > Seems right to me, once we verify that this is a safe thing to do.
> > > > 
> > > > hmm, so how should we do that?
> > > 
> > > Also: lots of tests. This makes me nervous.
> > 
> > +1, although, I did put some thought into making sure there are the
> appropriate
> > security checks in the chrome layer of the browser process when I moved the
> > <webview> API out of content and into chrome over the last few months. In
> fact,
> > security checks came up multiple times in design discussions as these were
> > intended to prevent an embedder from mucking with <adview> ads by using the
> > <webview> API. Hopefully, I didn't miss anything glaringly obvious.
> 
> Ok thanks Fady. I would have expected some tests to need updating though; so I
> would at least test (or make sure it's tested) in a web page, blessed, and
from
> within webui.
This CL alone is not ready for test yet. Though it allows <webview> to run in
unblessed context, <webview> is still limited to platform apps and platform apps
always run in its own blessed context, thus it is not possible to test the
unblessed flow (without adding lots of mock).

i have filed a bug crbug/295243 to add browser tests for the new use cases when
the feature is complete (when 24243007, 23653012 and 23530029 are all
submitted).

[meacer, 2013-09-24 00:08:22]

I did sort of a drive-by review upon Charlie's request. I don't know the
code very well and I only have a vague understanding of the overall change,
so here is a potentially naive question:

There are some chrome.webview.* methods that are not in webview_api.cc
(e.g. contentWindow.postMessage) and thus not guarded with process id
checks (WebViewGuest::From). Is it possible to put the same checks for
those APIs as well?

[Fady Samuel, 2013-09-24 20:25:24]

On 2013/09/24 00:08:22, Mustafa Emre Acer wrote:
> I did sort of a drive-by review upon Charlie's request. I don't know the
> code very well and I only have a vague understanding of the overall change,
> so here is a potentially naive question:
> 
> There are some chrome.webview.* methods that are not in webview_api.cc
> (e.g. contentWindow.postMessage) and thus not guarded with process id
> checks (WebViewGuest::From). Is it possible to put the same checks for
> those APIs as well?

Privileged <webview> APIs live in the Chrome layer. Content layer APIs in
BrowserPlugin are no more powerful than what iframes can do, and so I feel that
they don't need a guard? They're safe in any context? Charlie? Thoughts?

[Fady Samuel, 2013-09-24 20:25:24]

On 2013/09/24 00:08:22, Mustafa Emre Acer wrote:
> I did sort of a drive-by review upon Charlie's request. I don't know the
> code very well and I only have a vague understanding of the overall change,
> so here is a potentially naive question:
> 
> There are some chrome.webview.* methods that are not in webview_api.cc
> (e.g. contentWindow.postMessage) and thus not guarded with process id
> checks (WebViewGuest::From). Is it possible to put the same checks for
> those APIs as well?

Privileged <webview> APIs live in the Chrome layer. Content layer APIs in
BrowserPlugin are no more powerful than what iframes can do, and so I feel that
they don't need a guard? They're safe in any context? Charlie? Thoughts?

[Charlie Reis, 2013-09-24 20:57:49]

On 2013/09/24 20:25:24, Fady Samuel wrote:
> On 2013/09/24 00:08:22, Mustafa Emre Acer wrote:
> > I did sort of a drive-by review upon Charlie's request. I don't know the
> > code very well and I only have a vague understanding of the overall change,
> > so here is a potentially naive question:
> > 
> > There are some chrome.webview.* methods that are not in webview_api.cc
> > (e.g. contentWindow.postMessage) and thus not guarded with process id
> > checks (WebViewGuest::From). Is it possible to put the same checks for
> > those APIs as well?
> 
> Privileged <webview> APIs live in the Chrome layer. Content layer APIs in
> BrowserPlugin are no more powerful than what iframes can do, and so I feel
that
> they don't need a guard? They're safe in any context? Charlie? Thoughts?

Moving this discussion to the bug report so that we can more easily keep a
record of it.  See comment 6 of http://crbug.com/293724.

[Charlie Reis, 2013-09-24 22:19:20]

As mentioned in comment 1 of the bug, we'll want to add clear warnings to
webview_api and browser_plugin_guest_manager (perhaps in the .h file rather than
the .cc file?) that the webview API is unblessed, and thus any new APIs must
include checks to prevent abuse by normal renderer processes.  Hui, can you add
those in this CL?

[guohui, 2013-09-25 15:58:53]

On 2013/09/24 22:19:20, creis wrote:
> As mentioned in comment 1 of the bug, we'll want to add clear warnings to
> webview_api and browser_plugin_guest_manager (perhaps in the .h file rather
than
> the .cc file?) that the webview API is unblessed, and thus any new APIs must
> include checks to prevent abuse by normal renderer processes.  Hui, can you
add
> those in this CL?

Thanks everyone! Uploaded a new patch with comments added.

[not at google - send to devlin, 2013-09-25 17:14:02]

some more thoughts.

https://codereview.chromium.org/24243007/diff/28001/chrome/browser/extensions...
File chrome/browser/extensions/api/webview/webview_api.h (right):

https://codereview.chromium.org/24243007/diff/28001/chrome/browser/extensions...
chrome/browser/extensions/api/webview/webview_api.h:12: // renderer processes.
To enforce this at another level, consider having all of these functions extend
a WebviewExtensionFunction class which does the process ID check. You may be
able to utilise ExtensionFunction::HasPermission as a first layer of defense;
you may also need to do a bit more plumbing; I'm not totally sure what the
intent of this is.

https://codereview.chromium.org/24243007/diff/28001/chrome/renderer/extension...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/24243007/diff/28001/chrome/renderer/extension...
chrome/renderer/extensions/dispatcher.cc:1121: // extension in the current
process, in other words, if it is loaded in the
FWIW the comment is slightly misleading since there could be multiple extensions
active in a single process. "a" top frame might be more accurate.

https://codereview.chromium.org/24243007/diff/28001/chrome/renderer/extension...
chrome/renderer/extensions/dispatcher.cc:1122: // top frame. To support webview
in an iframed extension, we have to allow
note that this would include other non toplevel frames as well, such as
window.open(...).

[Charlie Reis, 2013-09-25 17:25:18]

Thanks!  Minor nit below, and just waiting to see if Mustafa has any further
thoughts.

https://codereview.chromium.org/24243007/diff/28001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest_manager.h (right):

https://codereview.chromium.org/24243007/diff/28001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest_manager.h:41: // WARNING:
BrowserPlugin could be loaded in an unblessed context, thus any new
This part isn't quite right-- BrowserPlugins can't be loaded in unblessed
contexts.  It's the webview API that's available (though I agree we shouldn't
talk about webviews in content/).

Maybe just leave out the first part:
WARNING: All APIs should be guarded with a process ID check like
CanEmbedderAccessInstanceIDMaybeKill, to prevent abuse by normal renderer
processes.

[guohui, 2013-09-25 22:43:51]

https://codereview.chromium.org/24243007/diff/28001/chrome/browser/extensions...
File chrome/browser/extensions/api/webview/webview_api.h (right):

https://codereview.chromium.org/24243007/diff/28001/chrome/browser/extensions...
chrome/browser/extensions/api/webview/webview_api.h:12: // renderer processes.
On 2013/09/25 17:14:03, kalman wrote:
> To enforce this at another level, consider having all of these functions
extend
> a WebviewExtensionFunction class which does the process ID check. You may be
> able to utilise ExtensionFunction::HasPermission as a first layer of defense;
> you may also need to do a bit more plumbing; I'm not totally sure what the
> intent of this is.

Done.

https://codereview.chromium.org/24243007/diff/28001/chrome/renderer/extension...
File chrome/renderer/extensions/dispatcher.cc (right):

https://codereview.chromium.org/24243007/diff/28001/chrome/renderer/extension...
chrome/renderer/extensions/dispatcher.cc:1121: // extension in the current
process, in other words, if it is loaded in the
On 2013/09/25 17:14:03, kalman wrote:
> FWIW the comment is slightly misleading since there could be multiple
extensions
> active in a single process. "a" top frame might be more accurate.

Done.

https://codereview.chromium.org/24243007/diff/28001/chrome/renderer/extension...
chrome/renderer/extensions/dispatcher.cc:1122: // top frame. To support webview
in an iframed extension, we have to allow
On 2013/09/25 17:14:03, kalman wrote:
> note that this would include other non toplevel frames as well, such as
> window.open(...).

Done.

https://codereview.chromium.org/24243007/diff/28001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest_manager.h (right):

https://codereview.chromium.org/24243007/diff/28001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest_manager.h:41: // WARNING:
BrowserPlugin could be loaded in an unblessed context, thus any new
On 2013/09/25 17:25:18, creis wrote:
> This part isn't quite right-- BrowserPlugins can't be loaded in unblessed
> contexts.  It's the webview API that's available (though I agree we shouldn't
> talk about webviews in content/).
> 
> Maybe just leave out the first part:
> WARNING: All APIs should be guarded with a process ID check like
> CanEmbedderAccessInstanceIDMaybeKill, to prevent abuse by normal renderer
> processes.

Done.

[Charlie Reis, 2013-09-25 22:48:43]

Thanks for abstracting out the check.  LGTM.

[not at google - send to devlin, 2013-09-25 23:46:46]

lgtm with that parting comment (not necessarily actionable)

https://codereview.chromium.org/24243007/diff/41001/chrome/browser/extensions...
File chrome/browser/extensions/api/webview/webview_api.h (right):

https://codereview.chromium.org/24243007/diff/41001/chrome/browser/extensions...
chrome/browser/extensions/api/webview/webview_api.h:12: // APIs must extend
WebviewExtensionFunction/WebviewExecuteCodeFunction which
FWIW, it's not totally clear to me (i.e. not clear just from this file) how
WebviewExecuteCodeFunction actually does this (how it's safe); overriding
GetScriptExecutor is a bit vague. A safer way to structure this would be to
delegate to ExecuteScriptFunction rather than extending it, and extend
WebviewExtensionFunction instead. But that would be a more significant change.

[guohui, 2013-09-26 13:57:59]

https://codereview.chromium.org/24243007/diff/41001/chrome/browser/extensions...
File chrome/browser/extensions/api/webview/webview_api.h (right):

https://codereview.chromium.org/24243007/diff/41001/chrome/browser/extensions...
chrome/browser/extensions/api/webview/webview_api.h:12: // APIs must extend
WebviewExtensionFunction/WebviewExecuteCodeFunction which
On 2013/09/25 23:46:46, kalman wrote:
> FWIW, it's not totally clear to me (i.e. not clear just from this file) how
> WebviewExecuteCodeFunction actually does this (how it's safe); overriding
> GetScriptExecutor is a bit vague. A safer way to structure this would be to
> delegate to ExecuteScriptFunction rather than extending it, and extend
> WebviewExtensionFunction instead. But that would be a more significant change.

TODO added.

[commit-bot: I haz the power, 2013-09-26 13:58:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/guohui@chromium.org/24243007/45001

[commit-bot: I haz the power, 2013-09-26 13:58:54]

Failed to apply patch for chrome/browser/extensions/api/webview/webview_api.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/api/webview/webview_api.h
  Hunk #7 FAILED at 154.
  Hunk #8 succeeded at 187 (offset 17 lines).
  Hunk #9 succeeded at 202 (offset 17 lines).
  1 out of 9 hunks FAILED -- saving rejects to file
chrome/browser/extensions/api/webview/webview_api.h.rej

Patch:       chrome/browser/extensions/api/webview/webview_api.h
Index: chrome/browser/extensions/api/webview/webview_api.h
diff --git a/chrome/browser/extensions/api/webview/webview_api.h
b/chrome/browser/extensions/api/webview/webview_api.h
index
59f9d975073894e00ac898bb040e93f78bc46583..cd5652e5e302a8a0d65d1b62ce15579a502a120c
100644
--- a/chrome/browser/extensions/api/webview/webview_api.h
+++ b/chrome/browser/extensions/api/webview/webview_api.h
@@ -6,10 +6,33 @@
 #define CHROME_BROWSER_EXTENSIONS_API_WEBVIEW_WEBVIEW_API_H_
 
 #include "chrome/browser/extensions/api/execute_code_function.h"
+#include "chrome/browser/guestview/webview/webview_guest.h"
 
+// WARNING: Webview could be loaded in an unblessed context, thus any new
+// APIs must extend WebviewExtensionFunction/WebviewExecuteCodeFunction which
+// do a process ID check to prevent abuse by normal renderer processes.
+// TODO(guohui): refactor WebviewExecuteCodeFunction to also extend
+// WebviewExtensionFunction.
 namespace extensions {
 
-class WebviewClearDataFunction : public AsyncExtensionFunction {
+// An abstract base class for async webview APIs. It does a process ID check
+// in RunImpl, and then calls RunImplSafe which must be overriden by all
+// subclasses.
+class WebviewExtensionFunction : public AsyncExtensionFunction {
+ public:
+   WebviewExtensionFunction() {}
+
+ protected:
+  virtual ~WebviewExtensionFunction() {}
+
+  // ExtensionFunction implementation.
+  virtual bool RunImpl() OVERRIDE FINAL;
+
+ private:
+  virtual bool RunImplSafe(WebViewGuest* guest) = 0;
+};
+
+class WebviewClearDataFunction : public WebviewExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("webview.clearData", WEBVIEW_CLEARDATA);
 
@@ -18,10 +41,9 @@ class WebviewClearDataFunction : public
AsyncExtensionFunction {
  protected:
   virtual ~WebviewClearDataFunction();
 
-  // ExtensionFunction implementation.
-  virtual bool RunImpl() OVERRIDE;
-
  private:
+  virtual bool RunImplSafe(WebViewGuest* guest);
+
   uint32 GetRemovalMask();
   void ClearDataDone();
 
@@ -46,7 +68,8 @@ class WebviewExecuteCodeFunction : public
extensions::ExecuteCodeFunction {
   virtual bool Init() OVERRIDE;
   virtual bool ShouldInsertCSS() const OVERRIDE;
   virtual bool CanExecuteScriptOnPage() OVERRIDE;
-  virtual extensions::ScriptExecutor* GetScriptExecutor() OVERRIDE;
+  // Guarded by a process ID check.
+  virtual extensions::ScriptExecutor* GetScriptExecutor() OVERRIDE FINAL;
   virtual bool IsWebView() const OVERRIDE;
 
  private:
@@ -92,7 +115,7 @@ class WebviewInsertCSSFunction : public
WebviewExecuteCodeFunction {
   DISALLOW_COPY_AND_ASSIGN(WebviewInsertCSSFunction);
 };
 
-class WebviewGoFunction : public AsyncExtensionFunction {
+class WebviewGoFunction : public WebviewExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("webview.go", WEBVIEW_GO);
 
@@ -101,14 +124,13 @@ class WebviewGoFunction : public AsyncExtensionFunction {
  protected:
   virtual ~WebviewGoFunction();
 
-  // ExtensionFunction implementation.
-  virtual bool RunImpl() OVERRIDE;
-
  private:
+  virtual bool RunImplSafe(WebViewGuest* guest);
+
   DISALLOW_COPY_AND_ASSIGN(WebviewGoFunction);
 };
 
-class WebviewReloadFunction : public AsyncExtensionFunction {
+class WebviewReloadFunction : public WebviewExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("webview.reload", WEBVIEW_RELOAD);
 
@@ -117,14 +139,13 @@ class WebviewReloadFunction : public
AsyncExtensionFunction {
  protected:
   virtual ~WebviewReloadFunction();
 
-  // ExtensionFunction implementation.
-  virtual bool RunImpl() OVERRIDE;
-
  private:
+  virtual bool RunImplSafe(WebViewGuest* guest);
+
   DISALLOW_COPY_AND_ASSIGN(WebviewReloadFunction);
 };
 
-class WebviewSetPermissionFunction : public AsyncExtensionFunction {
+class WebviewSetPermissionFunction : public WebviewExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("webview.setPermission", WEBVIEW_SETPERMISSION);
 
@@ -133,14 +154,13 @@ class WebviewSetPermissionFunction : public
AsyncExtensionFunction {
  protected:
   virtual ~WebviewSetPermissionFunction();
 
-  // ExtensionFunction implementation.
-  virtual bool RunImpl() OVERRIDE;
-
  private:
+  virtual bool RunImplSafe(WebViewGuest* guest);
+
   DISALLOW_COPY_AND_ASSIGN(WebviewSetPermissionFunction);
 };
 
-class WebviewStopFunction : public AsyncExtensionFunction {
+class WebviewStopFunction : public WebviewExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("webview.stop", WEBVIEW_STOP);
 
@@ -149,14 +169,13 @@ class WebviewStopFunction : public AsyncExtensionFunction
{
  protected:
   virtual ~WebviewStopFunction();
 
-  // ExtensionFunction implementation.
-  virtual bool RunImpl() OVERRIDE;
-
  private:
+  virtual bool RunImplSafe(WebViewGuest* guest);
+
   DISALLOW_COPY_AND_ASSIGN(WebviewStopFunction);
 };
 
-class WebviewTerminateFunction : public AsyncExtensionFunction {
+class WebviewTerminateFunction : public WebviewExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("webview.terminate", WEBVIEW_TERMINATE);
 
@@ -165,10 +184,9 @@ class WebviewTerminateFunction : public
AsyncExtensionFunction {
  protected:
   virtual ~WebviewTerminateFunction();
 
-  // ExtensionFunction implementation.
-  virtual bool RunImpl() OVERRIDE;
-
  private:
+  virtual bool RunImplSafe(WebViewGuest* guest);
+
   DISALLOW_COPY_AND_ASSIGN(WebviewTerminateFunction);
 };

[commit-bot: I haz the power, 2013-09-26 14:31:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/guohui@chromium.org/24243007/55001

[commit-bot: I haz the power, 2013-09-26 14:56:00]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-09-26 15:00:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/guohui@chromium.org/24243007/79001

[commit-bot: I haz the power, 2013-09-26 17:44:20]

Change committed as 225493