GuestViews should be able to specify task manager entries

GuestViews should be able to specify task manager entries

BUG=406575
TBR=cpu@chromium.org for changes to *.grd

Committed: https://crrev.com/99492bedc3bd5ac3a26b12fbf7599a55086a998e
Cr-Commit-Position: refs/heads/master@{#292312}

[Fady Samuel, 2014-08-22 21:06:27]

creis@: please review chrome/browser/task_manager/guest_information.cc

lfg@: please review app_view changes
hanxi@: please review web_view changes
ericzeng@: please review extension_options

[Fady Samuel, 2014-08-22 21:08:27]

+cpu@ for chrome/app/generated_resources.grd

[Charlie Reis, 2014-08-22 21:11:43]

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
Who determines this value?  I'm skeptical about allowing apps to declare a
prefix in the task manager, since they could list "Browser," "Tab," or other
confusing values that claim to be more important than they are.  It also
wouldn't send the prefix through localization.

[ericzeng, 2014-08-22 21:15:22]

extension_options lgtm

[Fady Samuel, 2014-08-22 21:15:31]

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
On 2014/08/22 21:11:43, Charlie Reis wrote:
> Who determines this value?  I'm skeptical about allowing apps to declare a
> prefix in the task manager, since they could list "Browser," "Tab," or other
> confusing values that claim to be more important than they are.  It also
> wouldn't send the prefix through localization.

Apps don't: however, there are multiple GuestView elements now:

extensionoptions, appview, webview, and soon mimetypehandler, etc

We would like different strings for different GuestView types.
Internationalization is still possible as we still use IDS strings

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

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
On 2014/08/22 21:15:31, Fady Samuel wrote:
> On 2014/08/22 21:11:43, Charlie Reis wrote:
> > Who determines this value?  I'm skeptical about allowing apps to declare a
> > prefix in the task manager, since they could list "Browser," "Tab," or other
> > confusing values that claim to be more important than they are.  It also
> > wouldn't send the prefix through localization.
> 
> Apps don't: however, there are multiple GuestView elements now:
> 
> extensionoptions, appview, webview, and soon mimetypehandler, etc
> 
> We would like different strings for different GuestView types.
> Internationalization is still possible as we still use IDS strings

For example: Here is what appview does:
https://codereview.chromium.org/497843002/patch/1/10004

[Charlie Reis, 2014-08-22 21:23:20]

https://codereview.chromium.org/497843002/diff/1/chrome/browser/guest_view/we...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/guest_view/we...
chrome/browser/guest_view/web_view/web_view_guest.cc:196: return
base::string16();
Would this show an empty row in the task manager?  That would be bad.

Same in app_view_guest.cc.

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
On 2014/08/22 21:16:24, Fady Samuel wrote:
> On 2014/08/22 21:15:31, Fady Samuel wrote:
> > On 2014/08/22 21:11:43, Charlie Reis wrote:
> > > Who determines this value?  I'm skeptical about allowing apps to declare a
> > > prefix in the task manager, since they could list "Browser," "Tab," or
other
> > > confusing values that claim to be more important than they are.  It also
> > > wouldn't send the prefix through localization.
> > 
> > Apps don't: however, there are multiple GuestView elements now:
> > 
> > extensionoptions, appview, webview, and soon mimetypehandler, etc
> > 
> > We would like different strings for different GuestView types.
> > Internationalization is still possible as we still use IDS strings
> 
> For example: Here is what appview does:
> https://codereview.chromium.org/497843002/patch/1/10004

Ah, thanks.  That sounds fine.  I just didn't want app developers to be able to
specify it.

Does it ever have a different format than Prefix: title?  If so, maybe we should
just get |message_id| from the GuestViewBase rather than duplicating the
l10n_util call everywhere?  That would also ensure we have a consistent format
for each row.

[lfg, 2014-08-22 21:25:12]

extensions/browser/guest_view/app_view/* lgtm

[cpu_(ooo_6.6-7.5), 2014-08-22 21:29:19]

grd are to be TBR.

[Xi Han, 2014-08-22 21:36:58]

web_view lgtm.

[Fady Samuel, 2014-08-22 21:48:54]

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
On 2014/08/22 21:23:20, Charlie Reis wrote:
> On 2014/08/22 21:16:24, Fady Samuel wrote:
> > On 2014/08/22 21:15:31, Fady Samuel wrote:
> > > On 2014/08/22 21:11:43, Charlie Reis wrote:
> > > > Who determines this value?  I'm skeptical about allowing apps to declare
a
> > > > prefix in the task manager, since they could list "Browser," "Tab," or
> other
> > > > confusing values that claim to be more important than they are.  It also
> > > > wouldn't send the prefix through localization.
> > > 
> > > Apps don't: however, there are multiple GuestView elements now:
> > > 
> > > extensionoptions, appview, webview, and soon mimetypehandler, etc
> > > 
> > > We would like different strings for different GuestView types.
> > > Internationalization is still possible as we still use IDS strings
> > 
> > For example: Here is what appview does:
> > https://codereview.chromium.org/497843002/patch/1/10004
> 
> Ah, thanks.  That sounds fine.  I just didn't want app developers to be able
to
> specify it.
> 
> Does it ever have a different format than Prefix: title?  If so, maybe we
should
> just get |message_id| from the GuestViewBase rather than duplicating the
> l10n_util call everywhere?  That would also ensure we have a consistent format
> for each row.

MIMETypeHandler (in a separate CL) is used to load a chrome extension page that
can handle resources on the web (such as PDF), we probably want to specify the
URL of the resource rather than the extension loading the resource in that case.

[Fady Samuel, 2014-08-22 21:57:15]

PTAL Charlie: Derived types are now FORCED to come up with a task name.

[Charlie Reis, 2014-08-22 21:57:48]

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
On 2014/08/22 21:48:54, Fady Samuel wrote:
> On 2014/08/22 21:23:20, Charlie Reis wrote:
> > On 2014/08/22 21:16:24, Fady Samuel wrote:
> > > On 2014/08/22 21:15:31, Fady Samuel wrote:
> > > > On 2014/08/22 21:11:43, Charlie Reis wrote:
> > > > > Who determines this value?  I'm skeptical about allowing apps to
declare
> a
> > > > > prefix in the task manager, since they could list "Browser," "Tab," or
> > other
> > > > > confusing values that claim to be more important than they are.  It
also
> > > > > wouldn't send the prefix through localization.
> > > > 
> > > > Apps don't: however, there are multiple GuestView elements now:
> > > > 
> > > > extensionoptions, appview, webview, and soon mimetypehandler, etc
> > > > 
> > > > We would like different strings for different GuestView types.
> > > > Internationalization is still possible as we still use IDS strings
> > > 
> > > For example: Here is what appview does:
> > > https://codereview.chromium.org/497843002/patch/1/10004
> > 
> > Ah, thanks.  That sounds fine.  I just didn't want app developers to be able
> to
> > specify it.
> > 
> > Does it ever have a different format than Prefix: title?  If so, maybe we
> should
> > just get |message_id| from the GuestViewBase rather than duplicating the
> > l10n_util call everywhere?  That would also ensure we have a consistent
format
> > for each row.
> 
> MIMETypeHandler (in a separate CL) is used to load a chrome extension page
that
> can handle resources on the web (such as PDF), we probably want to specify the
> URL of the resource rather than the extension loading the resource in that
case.

Could we do that by setting the title of that WebContents to the URL?

We can make this free-form if it's necessary, but I'd really like to have it
follow the "prefix: title" pattern and avoid code duplication if possible.

[Charlie Reis, 2014-08-22 22:00:45]

On 2014/08/22 21:57:15, Fady Samuel wrote:
> PTAL Charlie: Derived types are now FORCED to come up with a task name.

Hmm, I'm not sure I understand.

https://codereview.chromium.org/497843002/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:196: return
base::string16();
It still looks possible to return an empty string here.

https://codereview.chromium.org/497843002/diff/20001/extensions/browser/guest...
File extensions/browser/guest_view/app_view/app_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/20001/extensions/browser/guest...
extensions/browser/guest_view/app_view/app_view_guest.cc:143: return
base::string16();
It still looks possible to return an empty string here.

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

Sorry, what I mean by that is GuestViewBase::GetTaskName is now pure virtual.

The derived type can still return an empty string but that's only in the case
when AppViewGuest or WebViewGuest don't have a delegate. They will always have a
delegate if compiling chrome. They will lack a delegate if compiling app_shell.

https://codereview.chromium.org/497843002/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:196: return
base::string16();
On 2014/08/22 22:00:45, Charlie Reis wrote:
> It still looks possible to return an empty string here.

WebViewGuest will be used by app_shell which doesn't have the chrome module. I
don't believe app_shell has a Task Manager?

https://codereview.chromium.org/497843002/diff/20001/extensions/browser/guest...
File extensions/browser/guest_view/app_view/app_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/20001/extensions/browser/guest...
extensions/browser/guest_view/app_view/app_view_guest.cc:143: return
base::string16();
On 2014/08/22 22:00:45, Charlie Reis wrote:
> It still looks possible to return an empty string here.

AppViewGuest will be used by app_shell which doesn't have the chrome module. I
don't believe app_shell has a Task Manager?

[Charlie Reis, 2014-08-22 22:40:02]

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/1/chrome/browser/task_manager/...
chrome/browser/task_manager/guest_information.cc:71: GetTaskName();
On 2014/08/22 21:57:47, Charlie Reis wrote:
> Could we do that by setting the title of that WebContents to the URL?
> 
> We can make this free-form if it's necessary, but I'd really like to have it
> follow the "prefix: title" pattern and avoid code duplication if possible.

Is this suggestion feasible?  (Not sure if you saw it when the other message
came in.)

https://codereview.chromium.org/497843002/diff/20001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/20001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:196: return
base::string16();
On 2014/08/22 22:12:44, Fady Samuel wrote:
> On 2014/08/22 22:00:45, Charlie Reis wrote:
> > It still looks possible to return an empty string here.
> 
> WebViewGuest will be used by app_shell which doesn't have the chrome module. I
> don't believe app_shell has a Task Manager?

That's kind of orthogonal, though.  You could imagine app_shell getting a Task
Manager, or other ways to hit this.

It seems like we should return something sensible (like Webview: title) rather
than an empty string (which should never be possible).  My earlier suggestion to
keep the "prefix: title" approach in the Task Manager means you could just
return IDS_TASK_MANAGER_WEBVIEW_TAG_PREFIX in this case.

[Fady Samuel, 2014-08-25 18:44:49]

PTAL Charlie. I've separated GetTaskPrefix and GetTaskTitle into two separate
virtual methods in GuestViewBase:

GetTaskPrefix is pure virtual and GetTaskTitle has a default implementation that
simply grabs the WebContents' title.

If GetTaskPrefix returns 0, then we simply use the "Webview:" prefix by default
in GuestInformation.

GuestViewBase::GetTitle is unfortunately essentially a copy-and-paste from
task_manager::util::GetTitleFromWebContents because that code lives in chrome
and we need working code in content.

Thoughts? Is this what you had in mind?

[Fady Samuel, 2014-08-25 18:58:18]

On 2014/08/25 18:44:49, Fady Samuel wrote:
> PTAL Charlie. I've separated GetTaskPrefix and GetTaskTitle into two separate
> virtual methods in GuestViewBase:
> 
> GetTaskPrefix is pure virtual and GetTaskTitle has a default implementation
that
> simply grabs the WebContents' title.
> 
> If GetTaskPrefix returns 0, then we simply use the "Webview:" prefix by
default
> in GuestInformation.
> 
> GuestViewBase::GetTitle is unfortunately essentially a copy-and-paste from
> task_manager::util::GetTitleFromWebContents because that code lives in chrome
> and we need working code in content.
> 
> Thoughts? Is this what you had in mind?

Sorry, I meant working code in extensions.

[Charlie Reis, 2014-08-25 20:25:41]

This is much closer, thanks!

I may be missing something, but I still don't understand why we need
GuestViewBase::GetTaskTitle.  It's only called from the task manager code and
appears to return the same thing that GetTitleFromWebContents would.

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:196: return 0;
Could we use IDS_TASK_MANAGER_WEBVIEW_TAG_PREFIX ("Webview:") instead of 0?  It
sounds like that might be a reasonable default for WebViewGuest, based on the
class name, and then we wouldn't need to worry about whether 0 is a valid task
prefix or not.

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/task_mana...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/task_mana...
chrome/browser/task_manager/guest_information.cc:75: guest->GetTaskTitle());
Why do we need GetTaskTitle?  It looks like that's identical to
util::GetTitleFromWebContents(web_contents), which would let us avoid the code
duplication.

https://codereview.chromium.org/497843002/diff/80001/extensions/browser/guest...
File extensions/browser/guest_view/app_view/app_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/80001/extensions/browser/guest...
extensions/browser/guest_view/app_view/app_view_guest.cc:143: return 0;
Maybe IDS_TASK_MANAGER_APPVIEW_TAG_PREFIX by default?

[Fady Samuel, 2014-08-25 23:05:52]

PTAL Charlie. I moved the IDS strings to the extensions module which
significantly reduces complexity. I still have code duplication for GetTitle
though because GuestViewBase needs to be independent of stuff in chrome.

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/guest_vie...
File chrome/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/guest_vie...
chrome/browser/guest_view/web_view/web_view_guest.cc:196: return 0;
On 2014/08/25 20:25:41, Charlie Reis wrote:
> Could we use IDS_TASK_MANAGER_WEBVIEW_TAG_PREFIX ("Webview:") instead of 0? 
It
> sounds like that might be a reasonable default for WebViewGuest, based on the
> class name, and then we wouldn't need to worry about whether 0 is a valid task
> prefix or not.

Done.

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/task_mana...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/task_mana...
chrome/browser/task_manager/guest_information.cc:75: guest->GetTaskTitle());
On 2014/08/25 20:25:41, Charlie Reis wrote:
> Why do we need GetTaskTitle?  It looks like that's identical to
> util::GetTitleFromWebContents(web_contents), which would let us avoid the code
> duplication.

That code lives in chrome, this code lives in extensions.

https://codereview.chromium.org/497843002/diff/80001/extensions/browser/guest...
File extensions/browser/guest_view/app_view/app_view_guest.cc (right):

https://codereview.chromium.org/497843002/diff/80001/extensions/browser/guest...
extensions/browser/guest_view/app_view/app_view_guest.cc:143: return 0;
On 2014/08/25 20:25:41, Charlie Reis wrote:
> Maybe IDS_TASK_MANAGER_APPVIEW_TAG_PREFIX by default?

Done.

[Charlie Reis, 2014-08-25 23:47:22]

Sorry, I must be missing something.

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/task_mana...
File chrome/browser/task_manager/guest_information.cc (right):

https://codereview.chromium.org/497843002/diff/80001/chrome/browser/task_mana...
chrome/browser/task_manager/guest_information.cc:75: guest->GetTaskTitle());
On 2014/08/25 23:05:52, Fady Samuel wrote:
> On 2014/08/25 20:25:41, Charlie Reis wrote:
> > Why do we need GetTaskTitle?  It looks like that's identical to
> > util::GetTitleFromWebContents(web_contents), which would let us avoid the
code
> > duplication.
> 
> That code lives in chrome, this code lives in extensions.

But the only caller of this is in chrome, right?  Why do we need this in
extensions at all?

[Fady Samuel, 2014-08-26 15:14:30]

PTAL Charlie. Got rid of GuestViewBase::GetTitle.

[Charlie Reis, 2014-08-26 16:37:42]

Great!  This LGTM.  You'll probably need an owner's approval, but I like where
the task manager behavior ended up.

https://codereview.chromium.org/497843002/diff/120001/extensions/browser/gues...
File extensions/browser/guest_view/guest_view_base.cc (right):

https://codereview.chromium.org/497843002/diff/120001/extensions/browser/gues...
extensions/browser/guest_view/guest_view_base.cc:7: #include "base/i18n/rtl.h"
Is this leftover?

[Fady Samuel, 2014-08-26 19:02:46]

fsamuel@chromium.org changed reviewers:
+ sky@chromium.org

[Fady Samuel, 2014-08-26 19:02:46]

+sky@ for chrome/browser/task_manager OWNER review.

https://codereview.chromium.org/497843002/diff/120001/extensions/browser/gues...
File extensions/browser/guest_view/guest_view_base.cc (right):

https://codereview.chromium.org/497843002/diff/120001/extensions/browser/gues...
extensions/browser/guest_view/guest_view_base.cc:7: #include "base/i18n/rtl.h"
On 2014/08/26 16:37:42, Charlie Reis wrote:
> Is this leftover?

Removed.

[sky, 2014-08-26 20:28:11]

LGTM

[Fady Samuel, 2014-08-26 20:28:52]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-26 20:30:35]

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

[commit-bot: I haz the power, 2014-08-26 22:06:16]

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/46554)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/51916)
  linux_chromium_chromeos_rel_swarming 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_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-26 22:16:05]

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

[commit-bot: I haz the power, 2014-08-26 22:16:06]

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-26 22:59:14]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-26 23:02:13]

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

[commit-bot: I haz the power, 2014-08-26 23:59:45]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/46630)
  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_...)

[commit-bot: I haz the power, 2014-08-27 00:04:19]

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

[commit-bot: I haz the power, 2014-08-27 00:04:20]

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

[Fady Samuel, 2014-08-27 00:09:02]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-27 00:11:53]

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

[commit-bot: I haz the power, 2014-08-27 00:26:59]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/46672)
  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_...)

[commit-bot: I haz the power, 2014-08-27 00:38:52]

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

[commit-bot: I haz the power, 2014-08-27 00:38:53]

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

[Fady Samuel, 2014-08-27 00:53:26]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-27 00:54:16]

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

[commit-bot: I haz the power, 2014-08-27 01:12:55]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/46693)
  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_...)

[commit-bot: I haz the power, 2014-08-27 01:17:55]

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

[commit-bot: I haz the power, 2014-08-27 01:17:56]

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

[Fady Samuel, 2014-08-27 21:15:49]

The CQ bit was checked by fsamuel@chromium.org

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

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

[Fady Samuel, 2014-08-27 23:45:34]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-27 23:46:34]

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

[Fady Samuel, 2014-08-28 00:18:27]

fsamuel@chromium.org changed reviewers:
+ rockot@chromium.org

[Fady Samuel, 2014-08-28 00:18:27]

Ken, CHECKDEPS is failing on iOS. Any thoughts on how to fix it?

[commit-bot: I haz the power, 2014-08-28 01:36:25]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  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_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_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-28 01:40:30]

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

[commit-bot: I haz the power, 2014-08-28 01:40:31]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Fady Samuel, 2014-08-28 01:58:50]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2014-08-28 01:59:57]

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

[commit-bot: I haz the power, 2014-08-28 02:56:07]

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

[commit-bot: I haz the power, 2014-08-28 03:50:34]

Committed patchset #13 (id:240001) as 182766a4e449ed580f20b0887146102688679d93

[commit-bot: I haz the power, 2014-09-10 02:57:12]

Patchset 13 (id:??) landed as
https://crrev.com/99492bedc3bd5ac3a26b12fbf7599a55086a998e
Cr-Commit-Position: refs/heads/master@{#292312}