Remove BrowserPlugin's -internal-attach method

Remove BrowserPlugin's -internal-attach method

This CL exposes a much simpler content/public method:
void RenderView::AttachToBrowserPlugin(blink::WebNode&)

This removes BrowserPlugin's usage of NPAPI method bindings.
This will hopefully help us move towards using OOPIFs.

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

[Fady Samuel, 2014-08-08 22:33:44]

Hi Xi,

This is my internal-attach patch which is based on Istiaque's patch. It finally
works! Let's chat about it on Monday! I'll write down a short design doc for
Monday. 

Thanks,

Fady

[Fady Samuel, 2014-08-11 21:10:00]

https://codereview.chromium.org/444813002/diff/80001/chrome/renderer/extensio...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.cc (right):

https://codereview.chromium.org/444813002/diff/80001/chrome/renderer/extensio...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:50: 
extra line

[Xi Han, 2014-08-12 18:17:55]

lgtm with nit.

https://codereview.chromium.org/444813002/diff/60001/chrome/browser/guest_vie...
File chrome/browser/guest_view/guest_view_base.h (right):

https://codereview.chromium.org/444813002/diff/60001/chrome/browser/guest_vie...
chrome/browser/guest_view/guest_view_base.h:307:
scoped_ptr<base::DictionaryValue> attach_params_;
Please update the first line of the description: "extra parameters"->"attach
parameters".

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
File content/renderer/browser_plugin/browser_plugin.cc (right):

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
content/renderer/browser_plugin/browser_plugin.cc:518: case
BrowserPluginMsg_Attach_ACK::ID:
Please sort it alphabetically, i.e., move to after "case
BrowserPluginMsg_AdvanceFocus::ID".

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
File content/renderer/browser_plugin/browser_plugin_bindings.cc (right):

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
content/renderer/browser_plugin/browser_plugin_bindings.cc:139: NULL,           
                    &BrowserPluginBindingsEnumerate,
Should we change this format back?

https://codereview.chromium.org/444813002/diff/80001/content/renderer/render_...
File content/renderer/render_view_impl.h (right):

https://codereview.chromium.org/444813002/diff/80001/content/renderer/render_...
content/renderer/render_view_impl.h:486: 
Unnecessary change, please revert.

[Fady Samuel, 2014-08-13 22:24:40]

Addressed nits:

jochen@:

chrome/browser
chrome/renderer (not extensions)

jam@: content/public


kalman@: extension/renderer/*

https://codereview.chromium.org/444813002/diff/60001/chrome/browser/guest_vie...
File chrome/browser/guest_view/guest_view_base.h (right):

https://codereview.chromium.org/444813002/diff/60001/chrome/browser/guest_vie...
chrome/browser/guest_view/guest_view_base.h:307:
scoped_ptr<base::DictionaryValue> attach_params_;
On 2014/08/12 18:17:54, hanxi wrote:
> Please update the first line of the description: "extra parameters"->"attach
> parameters".

Done.

https://codereview.chromium.org/444813002/diff/80001/chrome/renderer/extensio...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.cc (right):

https://codereview.chromium.org/444813002/diff/80001/chrome/renderer/extensio...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:50: 
On 2014/08/11 21:10:00, Fady Samuel wrote:
> extra line

Done.

https://codereview.chromium.org/444813002/diff/80001/chrome/renderer/resource...
File chrome/renderer/resources/extensions/web_view.js (right):

https://codereview.chromium.org/444813002/diff/80001/chrome/renderer/resource...
chrome/renderer/resources/extensions/web_view.js:885: this.setAttribute('id',
'browser-plugin-' + IdGenerator.GetNextId());
Note to self: Remove this.

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
File content/renderer/browser_plugin/browser_plugin.cc (right):

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
content/renderer/browser_plugin/browser_plugin.cc:518: case
BrowserPluginMsg_Attach_ACK::ID:
On 2014/08/12 18:17:55, hanxi wrote:
> Please sort it alphabetically, i.e., move to after "case
> BrowserPluginMsg_AdvanceFocus::ID".  

Done.

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
File content/renderer/browser_plugin/browser_plugin_bindings.cc (right):

https://codereview.chromium.org/444813002/diff/80001/content/renderer/browser...
content/renderer/browser_plugin/browser_plugin_bindings.cc:139: NULL,           
                    &BrowserPluginBindingsEnumerate,
On 2014/08/12 18:17:55, hanxi wrote:
> Should we change this format back?

Done.

[not at google - send to devlin, 2014-08-13 23:40:17]

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.cc (right):

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:32:
content::RenderFrame* render_frame = context()->GetRenderFrame();
Nit: do this after argument validation.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:36: if
(args.Length() < 3 || !args[0]->IsInt32() || !args[1]->IsInt32() ||
!=3. Be explicit.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:37:
!args[2]->IsObject()) {
Add NOTREACHED() here as well. Or even CHECK. Best not to silently swallow
errors, it's cost me much time in the past debugging JS.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:46: if
(base::Value::TYPE_DICTIONARY != params_value->GetType()) {
If it's not a dictionary then V8ValueConverter is having issues, since you
checked it was an Object. Maybe NOTREACHED() this.

Also, don't use a Yoda condition, use params_value->IsType(DICTIONARY).

Final nit: it's nice not to leave dangling temporary variables like
|params_value| hanging around, asking to be incorrectly used. It's nice to scope
them locally, like

scoped_ptr<DictionaryValue> params;
{
  scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
  scoped_ptr<Value> params_as_value(
      converter->FromV8Value(args[2], context()->v8_context());
  CHECK(params_as_value->IsType(DICTIONARY));
  params = params_as_value.PassAs<DictionaryValue>();
}

Oops wrote more code there than I intended. You get the gist.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:52:
static_cast<base::DictionaryValue*>(params_value.release()));
Nit: use params_value.PassAs()?

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:63:
args.GetReturnValue().Set(v8::Boolean::New(isolate, true));
Nit: inline |isolate|. You could also use context()->isolate() but whatevs, same
diff.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.h (right):

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.h:13: //
Implements custom bindings for the pageActions API.
s/pageActions/guestViewInternal/

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.h:19: void
AttachGuest(const v8::FunctionCallbackInfo<v8::Value>& args);
Document AttachGuest and its arguments.

https://codereview.chromium.org/444813002/diff/120001/extensions/renderer/scr...
File extensions/renderer/script_context.cc (right):

https://codereview.chromium.org/444813002/diff/120001/extensions/renderer/scr...
extensions/renderer/script_context.cc:81: return NULL;
No else after return. Mind fixing line 74 as well?

[Fady Samuel, 2014-08-14 19:41:18]

Addressed kalman@'s comments

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.cc (right):

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:32:
content::RenderFrame* render_frame = context()->GetRenderFrame();
On 2014/08/13 23:40:16, kalman wrote:
> Nit: do this after argument validation.

Done.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:36: if
(args.Length() < 3 || !args[0]->IsInt32() || !args[1]->IsInt32() ||
On 2014/08/13 23:40:16, kalman wrote:
> !=3. Be explicit.

Done.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:37:
!args[2]->IsObject()) {
On 2014/08/13 23:40:16, kalman wrote:
> Add NOTREACHED() here as well. Or even CHECK. Best not to silently swallow
> errors, it's cost me much time in the past debugging JS.

Done.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:46: if
(base::Value::TYPE_DICTIONARY != params_value->GetType()) {
On 2014/08/13 23:40:16, kalman wrote:
> If it's not a dictionary then V8ValueConverter is having issues, since you
> checked it was an Object. Maybe NOTREACHED() this.
> 
> Also, don't use a Yoda condition, use params_value->IsType(DICTIONARY).
> 
> Final nit: it's nice not to leave dangling temporary variables like
> |params_value| hanging around, asking to be incorrectly used. It's nice to
scope
> them locally, like
> 
> scoped_ptr<DictionaryValue> params;
> {
>   scoped_ptr<V8ValueConverter> converter(V8ValueConverter::create());
>   scoped_ptr<Value> params_as_value(
>       converter->FromV8Value(args[2], context()->v8_context());
>   CHECK(params_as_value->IsType(DICTIONARY));
>   params = params_as_value.PassAs<DictionaryValue>();
> }
> 
> Oops wrote more code there than I intended. You get the gist.

Done.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:52:
static_cast<base::DictionaryValue*>(params_value.release()));
On 2014/08/13 23:40:16, kalman wrote:
> Nit: use params_value.PassAs()?

Can't. PassAs can only be used for upcasting and not downcasting.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.cc:63:
args.GetReturnValue().Set(v8::Boolean::New(isolate, true));
On 2014/08/13 23:40:16, kalman wrote:
> Nit: inline |isolate|. You could also use context()->isolate() but whatevs,
same
> diff.

Done.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.h (right):

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.h:13: //
Implements custom bindings for the pageActions API.
On 2014/08/13 23:40:16, kalman wrote:
> s/pageActions/guestViewInternal/

Done.

https://codereview.chromium.org/444813002/diff/120001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.h:19: void
AttachGuest(const v8::FunctionCallbackInfo<v8::Value>& args);
On 2014/08/13 23:40:16, kalman wrote:
> Document AttachGuest and its arguments.

Done.

https://codereview.chromium.org/444813002/diff/120001/extensions/renderer/scr...
File extensions/renderer/script_context.cc (right):

https://codereview.chromium.org/444813002/diff/120001/extensions/renderer/scr...
extensions/renderer/script_context.cc:81: return NULL;
On 2014/08/13 23:40:16, kalman wrote:
> No else after return. Mind fixing line 74 as well?

Done.

[not at google - send to devlin, 2014-08-14 19:51:22]

lgtm

https://codereview.chromium.org/444813002/diff/140001/chrome/renderer/extensi...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.h (right):

https://codereview.chromium.org/444813002/diff/140001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.h:28: // at the
time of attachment.
Could you phrase these arguments as a list, rather than a block of text?

[Fady Samuel, 2014-08-14 20:02:56]

Addressed nit.

https://codereview.chromium.org/444813002/diff/140001/chrome/renderer/extensi...
File chrome/renderer/extensions/guest_view_internal_custom_bindings.h (right):

https://codereview.chromium.org/444813002/diff/140001/chrome/renderer/extensi...
chrome/renderer/extensions/guest_view_internal_custom_bindings.h:28: // at the
time of attachment.
On 2014/08/14 19:51:22, kalman wrote:
> Could you phrase these arguments as a list, rather than a block of text?

Done.

[jam, 2014-08-15 17:41:04]

https://codereview.chromium.org/444813002/diff/160001/content/public/browser/...
File content/public/browser/browser_plugin_guest_manager.h (right):

https://codereview.chromium.org/444813002/diff/160001/content/public/browser/...
content/public/browser/browser_plugin_guest_manager.h:45: // content/ vs chrome/
IDs, remove this.
this is breaking http://www.chromium.org/developers/content-module/content-api

"methods in the API should be there because either content is calling out to its
embedder, or the embedder is calling to content. There shouldn't be any methods
which are used to call from the embedder to the embedder."

https://codereview.chromium.org/444813002/diff/160001/content/renderer/render...
File content/renderer/render_view_impl.h (right):

https://codereview.chromium.org/444813002/diff/160001/content/renderer/render...
content/renderer/render_view_impl.h:486: 
?

[Fady Samuel, 2014-08-16 23:24:30]

Hi John,

I've removed the content API that does the ID translation. This CL now has a
separate instance ID space in content from chrome. The content "BPID" is known
in chrome but not the opposite. Could I please get your thoughts on the content
API now?

Thanks,

Fady

https://codereview.chromium.org/444813002/diff/160001/content/public/browser/...
File content/public/browser/browser_plugin_guest_manager.h (right):

https://codereview.chromium.org/444813002/diff/160001/content/public/browser/...
content/public/browser/browser_plugin_guest_manager.h:45: // content/ vs chrome/
IDs, remove this.
On 2014/08/15 17:41:04, jam wrote:
> this is breaking http://www.chromium.org/developers/content-module/content-api
> 
> "methods in the API should be there because either content is calling out to
its
> embedder, or the embedder is calling to content. There shouldn't be any
methods
> which are used to call from the embedder to the embedder."

Removed.

https://codereview.chromium.org/444813002/diff/160001/content/public/browser/...
content/public/browser/browser_plugin_guest_manager.h:45: // content/ vs chrome/
IDs, remove this.
On 2014/08/15 17:41:04, jam wrote:
> this is breaking http://www.chromium.org/developers/content-module/content-api
> 
> "methods in the API should be there because either content is calling out to
its
> embedder, or the embedder is calling to content. There shouldn't be any
methods
> which are used to call from the embedder to the embedder."

Removed.

https://codereview.chromium.org/444813002/diff/160001/content/renderer/render...
File content/renderer/render_view_impl.h (right):

https://codereview.chromium.org/444813002/diff/160001/content/renderer/render...
content/renderer/render_view_impl.h:486: 
On 2014/08/15 17:41:04, jam wrote:
> ?

Removed.

[jam, 2014-08-18 15:53:58]

On 2014/08/16 23:24:30, Fady Samuel wrote:
> Hi John,
> 
> I've removed the content API that does the ID translation. This CL now has a
> separate instance ID space in content from chrome. The content "BPID" is known
> in chrome but not the opposite. Could I please get your thoughts on the
content
> API now?

I thought that the plan was that each ID namespace would be used in each model
only? i.e. content's IDs wouldn't be known outside? if Chrome knows content's
IDs, what's the point of it having its own IDs as wel?

[Fady Samuel, 2014-08-18 17:10:43]

On 2014/08/18 15:53:58, jam wrote:
> On 2014/08/16 23:24:30, Fady Samuel wrote:
> > Hi John,
> > 
> > I've removed the content API that does the ID translation. This CL now has a
> > separate instance ID space in content from chrome. The content "BPID" is
known
> > in chrome but not the opposite. Could I please get your thoughts on the
> content
> > API now?
> 
> I thought that the plan was that each ID namespace would be used in each model
> only? i.e. content's IDs wouldn't be known outside? if Chrome knows content's
> IDs, what's the point of it having its own IDs as wel?

This is tricky but (I think) not impossible. Content knows about BrowserPlugin
(in the embedder) and BrowserPluginGuest (in the guest), chrome knows about
GuestViews.

A GuestView is at a high-level simply a WebContentsDelegate, and
WebContentsObserver of a guest WebContents.

BrowserPlugin has a BrowserPluginGuest that is a WebContentsObserver on the same
guest (to handle input events and graphics, and IME and other content things).

There is a internal chrome JavaScript API by which guests are created:

GuestViewInternal.createGuest.

This CL introduces another internal chrome API:

GuestViewInternal.AttachGuest(browserPluginInstanceId, guestInstanceId,
attachParams)

Chrome is being made aware of this browser plugin ID because we have no clean
way to take a Javascript v8::Value and get its WebNode to speak directly to
BrowserPlugin.

Adam Barth was concerned with this approach here:
https://codereview.chromium.org/449683002/

At some point we need to attach a chrome-created guest to a content
BrowserPlugin. That means some kind of association (mapping) needs to happen.
Hence, chrome knows about content IDs.

I'm still actively thinking about if it's possible to do this without an
awareness of content IDs in chrome. Perhaps a VC chat might be helpful?

[Fady Samuel, 2014-08-18 23:16:01]

PTAL hanxi@: We can chat offline about what's going on.
+kenrb@ for _messages.h

[kenrb, 2014-08-19 15:37:35]

ipc lgtm

[Fady Samuel, 2014-08-20 15:34:24]

PTAL hanxi@: Let's chat these new changes offline.

jam@: Please review content API changes.

[jam, 2014-08-20 23:29:20]

lgtm

[Fady Samuel, 2014-08-20 23:33:15]

Xi: I'm CQ'ing this patch now because it's blocking progress on the PDF viewer.
We can sync up tomorrow offline to chat about this since I want to make sure
more there's more than one person that understands what's going in BrowserPlugin
land.

[Fady Samuel, 2014-08-20 23:33:21]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-20 23:33:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/444813002/260001

[commit-bot: I haz the power, 2014-08-21 00:44:46]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/44467)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/49643)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-21 00:47:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-21 00:47:18]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/44497)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2014-08-21 16:47:12]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-21 16:49:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/444813002/280001

[Fady Samuel, 2014-08-21 17:59:42]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-21 18:01:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/444813002/320001

[commit-bot: I haz the power, 2014-08-21 19:47:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-21 19:50:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-21 19:50:38]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Fady Samuel, 2014-08-21 20:01:35]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-21 20:05:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/444813002/340001

[Fady Samuel, 2014-08-21 21:16:08]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-21 21:27:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fsamuel@chromium.org/444813002/360001

[commit-bot: I haz the power, 2014-08-21 23:39:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-22 00:16:35]

Committed patchset #18 (360001) as 291261