An internal intents dispatcher useful for initiating an intent from the browser process.

An internal intents dispatcher useful for initiating an intent from the browser process.

R=jhawkins@chromium.org,jam@chromium.org
BUG=105732
TEST=InternalWebIntentsDispatcherTest


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

[James Hawkins, 2012-03-12 23:19:10]

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
File content/browser/intents/internal_web_intents_dispatcher.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.cc:33: WebContents*
destination_tab) {
WebIntentsDispatcher doesn't document this, but must |destination_tab| be
non-NULL?  If so, add that documentation to the interface and DCHECK
|destination_tab| here.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.cc:53: delete this;
What happens if SendReplyMessage is never called?

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:9: #include
"base/callback.h"
nit: Blank line in between std includes and src includes.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:16: // This class
implements a web intents coordinator object which originates
We use coordinator in the comment and dispatcher in the code: should we
reconcile?

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:18: // It will
terminate callbacks by notifying its delegate rather than returning
We should probably expand on some of the terminology here, as it's confusing
w/out knowing all of the Web Intents context, e.g.,

"...terminate callbacks sent from the active service..."

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:18: // It will
terminate callbacks by notifying its delegate rather than returning
Optional nit: "It" is less ambiguous in class-level documentation, but please
try to remove the pronoun (since pronouns in comments are ambiguous).

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:30:
base::Callback<void(webkit_glue::WebIntentReplyType,
typedef this.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:45:
webkit_glue::WebIntentData intent_;
Document member variable.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
File content/browser/intents/internal_web_intents_dispatcher_unittest.cc
(right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher_unittest.cc:38:
TEST_F(InternalWebIntentsDispatcherTest, SendIntent) {
Document what the test is doing.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
content/renderer/web_intents_host.cc:52: CHECK(!ssv.isNull());
These should likely be DCHECKs.

https://chromiumcodereview.appspot.com/9692017/diff/1/webkit/glue/web_intent_...
File webkit/glue/web_intent_data.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/webkit/glue/web_intent_...
webkit/glue/web_intent_data.h:34: int data_type;
Can we not use a named enum for this?

[Greg Billock, 2012-03-12 23:52:22]

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
File content/browser/intents/internal_web_intents_dispatcher.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.cc:33: WebContents*
destination_tab) {
On 2012/03/12 23:19:11, James Hawkins wrote:
> WebIntentsDispatcher doesn't document this, but must |destination_tab| be
> non-NULL?  If so, add that documentation to the interface and DCHECK
> |destination_tab| here.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.cc:53: delete this;
On 2012/03/12 23:19:11, James Hawkins wrote:
> What happens if SendReplyMessage is never called?

The dispatcher lasts as long as the intent is active. It is the responsibility
of the embedder to send a reply message when the intent becomes inactive. I
clarified the header language to make this more clear.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:9: #include
"base/callback.h"
On 2012/03/12 23:19:11, James Hawkins wrote:
> nit: Blank line in between std includes and src includes.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:16: // This class
implements a web intents coordinator object which originates
On 2012/03/12 23:19:11, James Hawkins wrote:
> We use coordinator in the comment and dispatcher in the code: should we
> reconcile?

Reworded a bit.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:18: // It will
terminate callbacks by notifying its delegate rather than returning
On 2012/03/12 23:19:11, James Hawkins wrote:
> Optional nit: "It" is less ambiguous in class-level documentation, but please
> try to remove the pronoun (since pronouns in comments are ambiguous).

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:18: // It will
terminate callbacks by notifying its delegate rather than returning
On 2012/03/12 23:19:11, James Hawkins wrote:
> We should probably expand on some of the terminology here, as it's confusing
> w/out knowing all of the Web Intents context, e.g.,
> 
> "...terminate callbacks sent from the active service..."

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:30:
base::Callback<void(webkit_glue::WebIntentReplyType,
On 2012/03/12 23:19:11, James Hawkins wrote:
> typedef this.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher.h:45:
webkit_glue::WebIntentData intent_;
On 2012/03/12 23:19:11, James Hawkins wrote:
> Document member variable.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
File content/browser/intents/internal_web_intents_dispatcher_unittest.cc
(right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/browser/intents...
content/browser/intents/internal_web_intents_dispatcher_unittest.cc:38:
TEST_F(InternalWebIntentsDispatcherTest, SendIntent) {
On 2012/03/12 23:19:11, James Hawkins wrote:
> Document what the test is doing.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
content/renderer/web_intents_host.cc:52: CHECK(!ssv.isNull());
On 2012/03/12 23:19:11, James Hawkins wrote:
> These should likely be DCHECKs.

We're parsing something we ourselves serialized. If there's an error, it
probably means the data is corrupted. IOW, a serious error.

https://chromiumcodereview.appspot.com/9692017/diff/1/webkit/glue/web_intent_...
File webkit/glue/web_intent_data.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/webkit/glue/web_intent_...
webkit/glue/web_intent_data.h:34: int data_type;
On 2012/03/12 23:19:11, James Hawkins wrote:
> Can we not use a named enum for this?

Done.

[James Hawkins, 2012-03-13 00:59:02]

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
content/renderer/web_intents_host.cc:52: CHECK(!ssv.isNull());
On 2012/03/12 23:52:22, Greg Billock wrote:
> On 2012/03/12 23:19:11, James Hawkins wrote:
> > These should likely be DCHECKs.
> 
> We're parsing something we ourselves serialized. If there's an error, it
> probably means the data is corrupted. IOW, a serious error.

DCHECK vs CHECK is not about severity.  CHECK should only be used to debug
crashes in the wild that cannot be repro'ed locally.

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:22: class
InternalWebIntentsDispatcher : public content::WebIntentsDispatcher {
I'm just curious, and jam will likely know better, but if this is specific to
the browser, should it be in chrome/browser/?

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:58:
std::vector<base::Callback<void(webkit_glue::WebIntentReplyType)> >
typedef this callback as well.

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/public/brows...
File content/public/browser/web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/public/brows...
content/public/browser/web_intents_dispatcher.h:44: // |web_contents| cannot be
null.
s/cannot/must not/

It actually *can* be null ;-)

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/public/brows...
content/public/browser/web_intents_dispatcher.h:44: // |web_contents| cannot be
null.
s/null/NULL/

https://chromiumcodereview.appspot.com/9692017/diff/3003/webkit/glue/web_inte...
File webkit/glue/web_intent_data.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/webkit/glue/web_inte...
webkit/glue/web_intent_data.h:31: SERIALIZED = 0,
Document DataType and the values.

[Greg Billock, 2012-03-13 17:42:38]

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
content/renderer/web_intents_host.cc:52: CHECK(!ssv.isNull());
DCHECK here will immediately mean a harder-to-diagnose crash in the next line.

On 2012/03/13 00:59:02, James Hawkins wrote:
> On 2012/03/12 23:52:22, Greg Billock wrote:
> > On 2012/03/12 23:19:11, James Hawkins wrote:
> > > These should likely be DCHECKs.
> > 
> > We're parsing something we ourselves serialized. If there's an error, it
> > probably means the data is corrupted. IOW, a serious error.
> 
> DCHECK vs CHECK is not about severity.  CHECK should only be used to debug
> crashes in the wild that cannot be repro'ed locally.

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:22: class
InternalWebIntentsDispatcher : public content::WebIntentsDispatcher {
On 2012/03/13 00:59:02, James Hawkins wrote:
> I'm just curious, and jam will likely know better, but if this is specific to
> the browser, should it be in chrome/browser/?

Good question. I'm thinking of it as a content-side API, but of course it
wouldn't have to be here.

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:58:
std::vector<base::Callback<void(webkit_glue::WebIntentReplyType)> >
On 2012/03/13 00:59:02, James Hawkins wrote:
> typedef this callback as well.

This type is really from the base class.

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/public/brows...
File content/public/browser/web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/public/brows...
content/public/browser/web_intents_dispatcher.h:44: // |web_contents| cannot be
null.
On 2012/03/13 00:59:02, James Hawkins wrote:
> s/cannot/must not/
> 
> It actually *can* be null ;-)

Done.

https://chromiumcodereview.appspot.com/9692017/diff/3003/webkit/glue/web_inte...
File webkit/glue/web_intent_data.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/webkit/glue/web_inte...
webkit/glue/web_intent_data.h:31: SERIALIZED = 0,
On 2012/03/13 00:59:02, James Hawkins wrote:
> Document DataType and the values.

Done.

[James Hawkins, 2012-03-13 18:22:03]

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
content/renderer/web_intents_host.cc:52: CHECK(!ssv.isNull());
How is that?  They're the exact same except DCHECK is for debug-only builds.

We write DCHECKs in our code, make sure test coverage is high.  If we get
crashes in the wild, we add CHECKs to debug.  Having this as a CHECK while in
development isn't going to help anything and it's against the style guide for
the reasons I mentioned:

http://dev.chromium.org/developers/coding-style#TOC-CHECK-vs-DCHECK-and-NOTRE...

On 2012/03/13 17:42:38, Greg Billock wrote:
> DCHECK here will immediately mean a harder-to-diagnose crash in the next line.
> 
> On 2012/03/13 00:59:02, James Hawkins wrote:
> > On 2012/03/12 23:52:22, Greg Billock wrote:
> > > On 2012/03/12 23:19:11, James Hawkins wrote:
> > > > These should likely be DCHECKs.
> > > 
> > > We're parsing something we ourselves serialized. If there's an error, it
> > > probably means the data is corrupted. IOW, a serious error.
> > 
> > DCHECK vs CHECK is not about severity.  CHECK should only be used to debug
> > crashes in the wild that cannot be repro'ed locally.
>

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:58:
std::vector<base::Callback<void(webkit_glue::WebIntentReplyType)> >
On 2012/03/13 17:42:38, Greg Billock wrote:
> On 2012/03/13 00:59:02, James Hawkins wrote:
> > typedef this callback as well.
> 
> This type is really from the base class.

typedef it in the base class.

[Greg Billock, 2012-03-13 19:26:50]

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/1/content/renderer/web_in...
content/renderer/web_intents_host.cc:52: CHECK(!ssv.isNull());
On 2012/03/13 18:22:03, James Hawkins wrote:
> How is that?  They're the exact same except DCHECK is for debug-only builds.

If control continues when ssv.isNull, ssv.deserialize will crash. I'll leave the
DCHECK here to make that easier to spot in debug. It's pretty unexpected,
though.


> We write DCHECKs in our code, make sure test coverage is high.  If we get
> crashes in the wild, we add CHECKs to debug.  Having this as a CHECK while in
> development isn't going to help anything and it's against the style guide for
> the reasons I mentioned:
> 
>
http://dev.chromium.org/developers/coding-style#TOC-CHECK-vs-DCHECK-and-NOTRE...
> 
> On 2012/03/13 17:42:38, Greg Billock wrote:
> > DCHECK here will immediately mean a harder-to-diagnose crash in the next
line.
> > 
> > On 2012/03/13 00:59:02, James Hawkins wrote:
> > > On 2012/03/12 23:52:22, Greg Billock wrote:
> > > > On 2012/03/12 23:19:11, James Hawkins wrote:
> > > > > These should likely be DCHECKs.
> > > > 
> > > > We're parsing something we ourselves serialized. If there's an error, it
> > > > probably means the data is corrupted. IOW, a serious error.
> > > 
> > > DCHECK vs CHECK is not about severity.  CHECK should only be used to debug
> > > crashes in the wild that cannot be repro'ed locally.
> > 
>

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/3003/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:58:
std::vector<base::Callback<void(webkit_glue::WebIntentReplyType)> >
On 2012/03/13 18:22:03, James Hawkins wrote:
> On 2012/03/13 17:42:38, Greg Billock wrote:
> > On 2012/03/13 00:59:02, James Hawkins wrote:
> > > typedef this callback as well.
> > 
> > This type is really from the base class.
> 
> typedef it in the base class.

Done.

[James Hawkins, 2012-03-13 20:15:44]

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:25: const string16&)>
ReplyCallback;
Document the callback and parameters.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:35: ReplyCallback
reply_callback);
const&

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:51: // Weak pointer to
the internal object which provides the intent to the
When I read "provides" I thought it meant "provides to this class".  This may be
clearer as "injects the intent into the newly-created tab contents."

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/public/brows...
File content/public/browser/web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/public/brows...
content/public/browser/web_intents_dispatcher.h:38: typedef
base::Callback<void(webkit_glue::WebIntentReplyType)>
Document the callback and the parameter.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/public/brows...
content/public/browser/web_intents_dispatcher.h:47: // |web_contents| should not
be NULL.
s/should/must/ right?

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/renderer/web...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/renderer/web...
content/renderer/web_intents_host.cc:52: CHECK(intent.data_type ==
webkit_glue::WebIntentData::UNSERIALIZED);
DCHECK

[Greg Billock, 2012-03-13 20:34:42]

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
File content/browser/intents/internal_web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:25: const string16&)>
ReplyCallback;
On 2012/03/13 20:15:44, James Hawkins wrote:
> Document the callback and parameters.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:35: ReplyCallback
reply_callback);
On 2012/03/13 20:15:44, James Hawkins wrote:
> const&

Done.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/browser/inte...
content/browser/intents/internal_web_intents_dispatcher.h:51: // Weak pointer to
the internal object which provides the intent to the
On 2012/03/13 20:15:44, James Hawkins wrote:
> When I read "provides" I thought it meant "provides to this class".  This may
be
> clearer as "injects the intent into the newly-created tab contents."

Done.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/public/brows...
File content/public/browser/web_intents_dispatcher.h (right):

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/public/brows...
content/public/browser/web_intents_dispatcher.h:38: typedef
base::Callback<void(webkit_glue::WebIntentReplyType)>
On 2012/03/13 20:15:44, James Hawkins wrote:
> Document the callback and the parameter.

Done.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/public/brows...
content/public/browser/web_intents_dispatcher.h:47: // |web_contents| should not
be NULL.
On 2012/03/13 20:15:44, James Hawkins wrote:
> s/should/must/ right?

Done.

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/renderer/web...
File content/renderer/web_intents_host.cc (right):

https://chromiumcodereview.appspot.com/9692017/diff/6002/content/renderer/web...
content/renderer/web_intents_host.cc:52: CHECK(intent.data_type ==
webkit_glue::WebIntentData::UNSERIALIZED);
On 2012/03/13 20:15:44, James Hawkins wrote:
> DCHECK

Done.

[James Hawkins, 2012-03-13 20:35:08]

LGTM

[Greg Billock, 2012-03-14 16:03:37]

Need love from owners.

[jam, 2012-03-14 19:05:56]

On 2012/03/14 16:03:37, Greg Billock wrote:
> Need love from owners.

I defer to darin since he knows more about webintents than me

[darin (slow to review), 2012-03-15 23:12:26]

LGTM, but I think it is time to move this binding to WebCore!

[Greg Billock, 2012-03-15 23:36:30]

On 2012/03/15 23:12:26, darin wrote:
> LGTM, but I think it is time to move this binding to WebCore!

Agreed! :-) It needs to happen to support transferables, which I'm working on
concurrently.

[commit-bot: I haz the power, 2012-03-15 23:58:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/9692017/1014

[commit-bot: I haz the power, 2012-03-16 01:33:08]

Failed to request the patch to try. Please note that binary filesare still
unsupported at the moment, this is being worked on.

Thanks for your patience.

HTTP Error 500: 
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>500 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered an error and could not complete your request.<p>If
the problem persists, please <A
HREF="http://code.google.com/appengine/community.html">report</A> your problem
and mention this error message and the query that caused it.</h2>
<h2></h2>
</body></html>

[commit-bot: I haz the power, 2012-03-16 01:34:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/9692017/10003

[commit-bot: I haz the power, 2012-03-16 03:34:14]

Try job failure for 9692017-10003 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-03-16 04:46:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/9692017/15004

[commit-bot: I haz the power, 2012-03-16 05:52:05]

Try job failure for 9692017-15004 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-03-16 21:06:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/9692017/15006

[commit-bot: I haz the power, 2012-03-16 22:35:49]

Change committed as 127275