Add support for distilling current WebContents

Add support for distilling current WebContents

This CL adds the utilities needed for using the current WebContents when
distilling web pages.

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

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

[nyquist, 2014-05-03 00:07:17]

yfriedman: PTAL for a first pass. Also, see UI hooks here:
https://codereview.chromium.org/269723009/

[Yaron, 2014-05-06 00:23:20]

https://codereview.chromium.org/266073003/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java
(right):

https://codereview.chromium.org/266073003/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java:27:
public static void distillCurrentPageAndView(Tab tab) {
Just pass WebContents directly. Comment seems out of date too. Can this be done
inside the component since it only deps on WC? Maybe even re-use the current JNI
rebinding and re-purpose as "DomDistillerService" api?

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
File components/dom_distiller/content/distiller_page_web_contents.cc (right):

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
components/dom_distiller/content/distiller_page_web_contents.cc:47:
reinterpret_cast<SourcePageHandleWebContents*>(handle.release()));
static_cast

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
components/dom_distiller/content/distiller_page_web_contents.cc:76: // executing
JavaScript.
In the attach case, I wonder if we should stop the WebContents. it's a little
risky because it's a viable view. I guess if we assume that the distilled
contents are in the same tab (i.e. you have to navigate to get back to the
pre-distilled contents) then it's ok.

[nyquist, 2014-05-06 03:52:26]

https://codereview.chromium.org/266073003/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java
(right):

https://codereview.chromium.org/266073003/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java:27:
public static void distillCurrentPageAndView(Tab tab) {
On 2014/05/06 00:23:20, Yaron wrote:
> Just pass WebContents directly. Comment seems out of date too. Can this be
done
> inside the component since it only deps on WC? Maybe even re-use the current
JNI
> rebinding and re-purpose as "DomDistillerService" api?

DomDistillerService should not depend on WebContents, since it's from //content.

Then for just putting it in //components/dom_distiller/content/: The
implementation of this in C++ needs to have access to
//chrome/browser/ui/tab_contents/core_tab_helper[|_delegate].[cc|h] and
chrome/browser/dom_distiller/dom_distiller_service_factory.h.

The latter we could remove by just passing in DomDistillerService from the
caller, but the CoreTabHelper stuff seems hard to fix.

https://codereview.chromium.org/266073003/diff/1/chrome/browser/dom_distiller...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/266073003/diff/1/chrome/browser/dom_distiller...
chrome/browser/dom_distiller/tab_utils.cc:9: #include
"chrome/browser/ui/android/window_android_helper.h"
Note to self: Remove.

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
File components/dom_distiller/content/distiller_page_web_contents.cc (right):

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
components/dom_distiller/content/distiller_page_web_contents.cc:47:
reinterpret_cast<SourcePageHandleWebContents*>(handle.release()));
On 2014/05/06 00:23:20, Yaron wrote:
> static_cast

Sadly, no. This cast makes the type more specific, i.e. from SourcePageHandle to
SourcePageHandleWebContents.
We could've done this cast to a specific type with a dynamic_cast I think, but
it seems like Chrome is compiled with -nortti, so there is no real time type
information.

Maybe I should use templates or something similar instead? Or maybe design this
differently?

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
components/dom_distiller/content/distiller_page_web_contents.cc:76: // executing
JavaScript.
On 2014/05/06 00:23:20, Yaron wrote:
> In the attach case, I wonder if we should stop the WebContents. it's a little
> risky because it's a viable view. I guess if we assume that the distilled
> contents are in the same tab (i.e. you have to navigate to get back to the
> pre-distilled contents) then it's ok.

Yeah, I think we could stop the WebContents here.

[nyquist, 2014-05-15 10:15:35]

Addressed all comments, and changed to use WebContentsUserData, registered
together with other tab helpers.

yfriedman: PTAL. Still missing tests.

https://codereview.chromium.org/266073003/diff/1/chrome/android/java/src/org/...
File
chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java
(right):

https://codereview.chromium.org/266073003/diff/1/chrome/android/java/src/org/...
chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerTabUtils.java:27:
public static void distillCurrentPageAndView(Tab tab) {
On 2014/05/06 03:52:27, nyquist wrote:
> On 2014/05/06 00:23:20, Yaron wrote:
> > Just pass WebContents directly. Comment seems out of date too. Can this be
> done
> > inside the component since it only deps on WC? Maybe even re-use the current
> JNI
> > rebinding and re-purpose as "DomDistillerService" api?
> 
> DomDistillerService should not depend on WebContents, since it's from
//content.
> 
> Then for just putting it in //components/dom_distiller/content/: The
> implementation of this in C++ needs to have access to
> //chrome/browser/ui/tab_contents/core_tab_helper[|_delegate].[cc|h] and
> chrome/browser/dom_distiller/dom_distiller_service_factory.h.
> 
> The latter we could remove by just passing in DomDistillerService from the
> caller, but the CoreTabHelper stuff seems hard to fix.

Changed to take WebContents here.

https://codereview.chromium.org/266073003/diff/1/chrome/browser/dom_distiller...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/266073003/diff/1/chrome/browser/dom_distiller...
chrome/browser/dom_distiller/tab_utils.cc:9: #include
"chrome/browser/ui/android/window_android_helper.h"
On 2014/05/06 03:52:27, nyquist wrote:
> Note to self: Remove.

Done.

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
File components/dom_distiller/content/distiller_page_web_contents.cc (right):

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
components/dom_distiller/content/distiller_page_web_contents.cc:47:
reinterpret_cast<SourcePageHandleWebContents*>(handle.release()));
On 2014/05/06 03:52:27, nyquist wrote:
> On 2014/05/06 00:23:20, Yaron wrote:
> > static_cast
> 
> Sadly, no. This cast makes the type more specific, i.e. from SourcePageHandle
to
> SourcePageHandleWebContents.
> We could've done this cast to a specific type with a dynamic_cast I think, but
> it seems like Chrome is compiled with -nortti, so there is no real time type
> information.
> 
> Maybe I should use templates or something similar instead? Or maybe design
this
> differently?

Done.

https://codereview.chromium.org/266073003/diff/1/components/dom_distiller/con...
components/dom_distiller/content/distiller_page_web_contents.cc:76: // executing
JavaScript.
On 2014/05/06 03:52:27, nyquist wrote:
> On 2014/05/06 00:23:20, Yaron wrote:
> > In the attach case, I wonder if we should stop the WebContents. it's a
little
> > risky because it's a viable view. I guess if we assume that the distilled
> > contents are in the same tab (i.e. you have to navigate to get back to the
> > pre-distilled contents) then it's ok.
> 
> Yeah, I think we could stop the WebContents here.

Done.

[Yaron, 2014-05-16 02:02:49]

https://codereview.chromium.org/266073003/diff/80001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/266073003/diff/80001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:19: void
DistillCurrentPageAndViewJava(JNIEnv* env,
nit: remove "Java" from the name

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
File components/dom_distiller/content/distiller_page_web_contents.cc (right):

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
components/dom_distiller/content/distiller_page_web_contents.cc:86:
web_contents_->Stop();
You don't want to stop here, right? You're waiting for it to finish loading and
then ExecuteJavaScript will stop it.

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
File components/dom_distiller/content/web_contents_main_frame_observer.h
(right):

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
components/dom_distiller/content/web_contents_main_frame_observer.h:5: #ifndef
COMPONENTS_DOM_DISTILLER_CONTENT_INJECTION_WEB_CONTENTS_OBSERVER_H_
nit: update

[nyquist, 2014-05-20 19:58:36]

yfriedman: PTAL

https://codereview.chromium.org/266073003/diff/80001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/266073003/diff/80001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:19: void
DistillCurrentPageAndViewJava(JNIEnv* env,
On 2014/05/16 02:02:49, Yaron wrote:
> nit: remove "Java" from the name

Done.

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
File components/dom_distiller/content/distiller_page_web_contents.cc (right):

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
components/dom_distiller/content/distiller_page_web_contents.cc:86:
web_contents_->Stop();
On 2014/05/16 02:02:49, Yaron wrote:
> You don't want to stop here, right? You're waiting for it to finish loading
and
> then ExecuteJavaScript will stop it.

Done.

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
File components/dom_distiller/content/web_contents_main_frame_observer.h
(right):

https://codereview.chromium.org/266073003/diff/80001/components/dom_distiller...
components/dom_distiller/content/web_contents_main_frame_observer.h:5: #ifndef
COMPONENTS_DOM_DISTILLER_CONTENT_INJECTION_WEB_CONTENTS_OBSERVER_H_
On 2014/05/16 02:02:49, Yaron wrote:
> nit: update

Done.

[nyquist, 2014-05-20 20:08:10]

sky: PTAL chrome/browser/ui

[sky, 2014-05-20 20:59:44]

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
File chrome/browser/ui/tab_helpers.cc (right):

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
chrome/browser/ui/tab_helpers.cc:197:
dom_distiller::WebContentsMainFrameObserver::CreateForWebContents(
Why do we need to do this here? Can't you install whatever is needed when the
menu item is clicked?

[nyquist, 2014-05-20 21:03:27]

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
File chrome/browser/ui/tab_helpers.cc (right):

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
chrome/browser/ui/tab_helpers.cc:197:
dom_distiller::WebContentsMainFrameObserver::CreateForWebContents(
On 2014/05/20 20:59:45, sky wrote:
> Why do we need to do this here? Can't you install whatever is needed when the
> menu item is clicked?

The WebContentsMainFrameObserver is only there to track whether the
WebContentsObservers of the given |web_contents| have gotten a call to
DocumentLoadedInFrame for the main frame. Adding it at the time of when you
click the menu item would be too late, as the event has most probably already
been triggered. Say you go to a page, click the menu item, and then add the
observer; chances are the page has already (at least mostly) loaded.

After talking to jam@ I realized that putting this state tracking in //content
was not encouraged, if it was possible to do it in //chrome.

[sky, 2014-05-20 23:25:20]

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
File chrome/browser/ui/tab_helpers.cc (right):

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
chrome/browser/ui/tab_helpers.cc:197:
dom_distiller::WebContentsMainFrameObserver::CreateForWebContents(
On 2014/05/20 21:03:28, nyquist wrote:
> On 2014/05/20 20:59:45, sky wrote:
> > Why do we need to do this here? Can't you install whatever is needed when
the
> > menu item is clicked?
> 
> The WebContentsMainFrameObserver is only there to track whether the
> WebContentsObservers of the given |web_contents| have gotten a call to
> DocumentLoadedInFrame for the main frame. Adding it at the time of when you
> click the menu item would be too late, as the event has most probably already
> been triggered. Say you go to a page, click the menu item, and then add the
> observer; chances are the page has already (at least mostly) loaded.
> 
> After talking to jam@ I realized that putting this state tracking in //content
> was not encouraged, if it was possible to do it in //chrome.

Can't you determine what you need at the time you need it rather than making
everything get more expensive in anticipation of using something?

[nyquist, 2014-05-20 23:28:10]

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
File chrome/browser/ui/tab_helpers.cc (right):

https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
chrome/browser/ui/tab_helpers.cc:197:
dom_distiller::WebContentsMainFrameObserver::CreateForWebContents(
On 2014/05/20 23:25:20, sky wrote:
> On 2014/05/20 21:03:28, nyquist wrote:
> > On 2014/05/20 20:59:45, sky wrote:
> > > Why do we need to do this here? Can't you install whatever is needed when
> the
> > > menu item is clicked?
> > 
> > The WebContentsMainFrameObserver is only there to track whether the
> > WebContentsObservers of the given |web_contents| have gotten a call to
> > DocumentLoadedInFrame for the main frame. Adding it at the time of when you
> > click the menu item would be too late, as the event has most probably
already
> > been triggered. Say you go to a page, click the menu item, and then add the
> > observer; chances are the page has already (at least mostly) loaded.
> > 
> > After talking to jam@ I realized that putting this state tracking in
//content
> > was not encouraged, if it was possible to do it in //chrome.
> 
> Can't you determine what you need at the time you need it rather than making
> everything get more expensive in anticipation of using something?

That would be great, but I could not find any place where this state is already
stored. In WebContents / WebContentsImpl, the events are just forwarded to the
observers. Any thoughts as to where this could already be?

[sky, 2014-05-21 21:39:46]

On Tue, May 20, 2014 at 4:28 PM,  <nyquist@chromium.org> wrote:
>
>
https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
> File chrome/browser/ui/tab_helpers.cc (right):
>
>
https://codereview.chromium.org/266073003/diff/100001/chrome/browser/ui/tab_h...
> chrome/browser/ui/tab_helpers.cc:197:
> dom_distiller::WebContentsMainFrameObserver::CreateForWebContents(
> On 2014/05/20 23:25:20, sky wrote:
>>
>> On 2014/05/20 21:03:28, nyquist wrote:
>> > On 2014/05/20 20:59:45, sky wrote:
>> > > Why do we need to do this here? Can't you install whatever is
>
> needed when
>>
>> the
>> > > menu item is clicked?
>> >
>> > The WebContentsMainFrameObserver is only there to track whether the
>> > WebContentsObservers of the given |web_contents| have gotten a call
>
> to
>>
>> > DocumentLoadedInFrame for the main frame. Adding it at the time of
>
> when you
>>
>> > click the menu item would be too late, as the event has most
>
> probably already
>>
>> > been triggered. Say you go to a page, click the menu item, and then
>
> add the
>>
>> > observer; chances are the page has already (at least mostly) loaded.
>> >
>> > After talking to jam@ I realized that putting this state tracking in
>
> //content
>>
>> > was not encouraged, if it was possible to do it in //chrome.
>
>
>> Can't you determine what you need at the time you need it rather than
>
> making
>>
>> everything get more expensive in anticipation of using something?
>
>
> That would be great, but I could not find any place where this state is
> already stored. In WebContents / WebContentsImpl, the events are just
> forwarded to the observers. Any thoughts as to where this could already
> be?

I'm not sure what the right way is to determine this. I know the
renderer has it, but I'm not sure if its tracked in the browser. I'm
adding Darin as he likely knows.

+darin - What is the best way from the browser side to determine if
the main frame's document has loaded?

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Yaron, 2014-05-22 18:25:01]

dom_distiller aspects lgtm

sky: Tommy had originally talked to jam about adding that state to
WebContentsImpl. It seems like the clearest place to know that browser-side but
jam said to have feature-code track what it needs instead

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/tab_utils.cc:104:
content::NavigationController::LoadURLParams params(viewer_url);
I think this should have PAGE_TRANSITION_AUTO_BOOKMARK

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
File components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
(right):

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:98:
EXPECT_EQ(true, use_new_web_contents_);
ASSERT_EQ? I think a lot's going to go south if you miss this assertion.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:100:
if (use_new_web_contents_) {
remove this

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:295:
bool use_new_web_contents,
nit: expect_new_web_contents throughout as this is expectation not how to run.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:304:
scoped_ptr<WebContentsMainFrameHelper> main_frame_loaded(
why scoped_ptr?

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
File components/dom_distiller/content/web_contents_main_frame_observer.cc
(right):

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/web_contents_main_frame_observer.cc:26: }
Call CleanUp to be safe? (i.e. if you're deleted, don't continue to hang off the
WebContents)

[sky, 2014-05-22 19:50:40]

Darin said this is the best way now:( So, LGTM

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/tab_utils.h (right):

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/tab_utils.h:12: namespace dom_distiller {
Generally we don't use namespaces in chrome code like this. I would nuke both of
these.

[nyquist, 2014-05-22 23:00:54]

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/tab_utils.cc (right):

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/tab_utils.cc:104:
content::NavigationController::LoadURLParams params(viewer_url);
On 2014/05/22 18:25:02, Yaron wrote:
> I think this should have PAGE_TRANSITION_AUTO_BOOKMARK

Done.

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/tab_utils.h (right):

https://codereview.chromium.org/266073003/diff/120001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/tab_utils.h:12: namespace dom_distiller {
On 2014/05/22 19:50:41, sky wrote:
> Generally we don't use namespaces in chrome code like this. I would nuke both
of
> these.

Done.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
File components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
(right):

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:98:
EXPECT_EQ(true, use_new_web_contents_);
On 2014/05/22 18:25:02, Yaron wrote:
> ASSERT_EQ? I think a lot's going to go south if you miss this assertion.

Done.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:100:
if (use_new_web_contents_) {
On 2014/05/22 18:25:02, Yaron wrote:
> remove this

Done.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:295:
bool use_new_web_contents,
On 2014/05/22 18:25:02, Yaron wrote:
> nit: expect_new_web_contents throughout as this is expectation not how to run.

Done.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/distiller_page_web_contents_browsertest.cc:304:
scoped_ptr<WebContentsMainFrameHelper> main_frame_loaded(
On 2014/05/22 18:25:02, Yaron wrote:
> why scoped_ptr?

Done.

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
File components/dom_distiller/content/web_contents_main_frame_observer.cc
(right):

https://codereview.chromium.org/266073003/diff/120001/components/dom_distille...
components/dom_distiller/content/web_contents_main_frame_observer.cc:26: }
On 2014/05/22 18:25:02, Yaron wrote:
> Call CleanUp to be safe? (i.e. if you're deleted, don't continue to hang off
the
> WebContents)

Done.

[nyquist, 2014-05-22 23:01:06]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2014-05-22 23:02:31]

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

[nyquist, 2014-05-22 23:26:10]

The CQ bit was unchecked by nyquist@chromium.org

[nyquist, 2014-05-23 06:52:43]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2014-05-23 06:54:27]

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

[commit-bot: I haz the power, 2014-05-23 11:43:00]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 14:05:37]

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

[commit-bot: I haz the power, 2014-05-23 14:05:37]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[nyquist, 2014-05-23 15:13:49]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2014-05-23 15:14:10]

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

[commit-bot: I haz the power, 2014-05-23 20:05:07]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 22:11:35]

Change committed as 272611

[sadrul, 2014-05-24 05:44:10]

Hi. This CL has been reverted in r272704 because the test added this patch is
failing in some mac bots. The link to the failures are linked to from the revert
CL (https://codereview.chromium.org/297163003)

[nyquist, 2014-05-27 20:38:40]

The CQ bit was checked by nyquist@chromium.org

[commit-bot: I haz the power, 2014-05-27 20:39:39]

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

[commit-bot: I haz the power, 2014-05-27 23:51:02]

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

[commit-bot: I haz the power, 2014-05-28 01:24:17]

Change committed as 273101