Add test for showing confirmation dialog for unsecure signin

Followup of https://codereview.chromium.org/317093002/. This CL adds tests for showing confirmation dialog for unsecure signin.

As prerequisite, this CL also,

1. changes the signin continue url to a local page in the gaia auth extension.
2. adds LoginUiService::Observer events for showing signin confirmation dialog for unsecure signin.
3. fixes broken methods in FakeGaia

BUG=368905
Committed: https://crrev.com/4bdcf21dcc119bdf3bea37e4a97e6ef0c05517bb
Cr-Commit-Position: refs/heads/master@{#293572}

[guohui, 2014-07-24 16:04:11]

Hey,

could you please take a look at the CL?

rogerta@ for the overall CL
xiyuan@ for owner review of saml_browsertest.cc
creis@ and nasko@ for security review

And you are all welcome to review everything.

Thanks,

Hui

[Charlie Reis, 2014-07-24 18:14:39]

I'll defer to sign-in reviewers for most of the code.  Just a few questions
below.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (left):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:229: // Loading any
untrusted (e.g., HTTP) URLs in the privileged sign-in process
Looks like you're changing the actual fix.  How does that affect what would be
merged to other branches?

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:213: //
MockBrowserWindow mock_browser_window_;
nit: Remove commented out code.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:288: #if
!defined(OS_CHROMEOS)
Why doesn't this run on ChromeOS?  Do we show the prompt there?

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:290:
ConfirmationRequiredForNonsecureSignin) {
Which part of the test goes to an insecure page in the sign-in process?  I just
see the navigation to the SOURCE_START_PAGE promo URL.

[guohui, 2014-07-24 18:31:22]

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (left):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:229: // Loading any
untrusted (e.g., HTTP) URLs in the privileged sign-in process
This CL changes the continue URL (from https://www.google.com/something to
chrome-extension://mfff.../success.html), and the continue URL has to be
excluded from the untrusted-domain-check, thus the logic here needs to be
updated. Security wise, there should be no negative impact, since we just switch
from a blank google web page to a blank chrome local page.

I did the change here, because it prevents issues that Chrome signin flow could
be broken by unexpected URL changes of google servers (see crbug/333869), and
second it is easier to write tests since i dont need to set up a fake server for
google.com.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:288: #if
!defined(OS_CHROMEOS)
Because the confirmation dialog path in inline_login_handler_impl.cc is only
executed on desktop. I suspect CrOS has similar issues as Chrome desktop, but
their implementation is slightly different, thus I will delegate to CrOS team to
fix it.

+xiyuan@ to confirm.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:290:
ConfirmationRequiredForNonsecureSignin) {
Signin page has an embedded gaia iframe, which loads the gaia signin page. Above
I set the gaia URL to the embedded test server, which runs on HTTP by default.

[Charlie Reis, 2014-07-24 18:47:11]

Thanks.  Just waiting for xiyuan to review (and confirm there will be a fix for
ChromeOS).

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (left):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:229: // Loading any
untrusted (e.g., HTTP) URLs in the privileged sign-in process
On 2014/07/24 18:31:21, guohui wrote:
> This CL changes the continue URL (from https://www.google.com/something to
> chrome-extension://mfff.../success.html), and the continue URL has to be
> excluded from the untrusted-domain-check, thus the logic here needs to be
> updated. Security wise, there should be no negative impact, since we just
switch
> from a blank google web page to a blank chrome local page.
> 
> I did the change here, because it prevents issues that Chrome signin flow
could
> be broken by unexpected URL changes of google servers (see crbug/333869), and
> second it is easier to write tests since i dont need to set up a fake server
for
> http://google.com.
> 

Works for me, but please leave the explanatory comment in place.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:290:
ConfirmationRequiredForNonsecureSignin) {
On 2014/07/24 18:31:21, guohui wrote:
> Signin page has an embedded gaia iframe, which loads the gaia signin page.
Above
> I set the gaia URL to the embedded test server, which runs on HTTP by default.

Ah, ok. Please explain that in a comment here, since it's not easy to spot.

[guohui, 2014-07-24 19:37:18]

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (left):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:229: // Loading any
untrusted (e.g., HTTP) URLs in the privileged sign-in process
On 2014/07/24 18:47:11, Charlie Reis wrote:
> On 2014/07/24 18:31:21, guohui wrote:
> > This CL changes the continue URL (from https://www.google.com/something to
> > chrome-extension://mfff.../success.html), and the continue URL has to be
> > excluded from the untrusted-domain-check, thus the logic here needs to be
> > updated. Security wise, there should be no negative impact, since we just
> switch
> > from a blank google web page to a blank chrome local page.
> > 
> > I did the change here, because it prevents issues that Chrome signin flow
> could
> > be broken by unexpected URL changes of google servers (see crbug/333869),
and
> > second it is easier to write tests since i dont need to set up a fake server
> for
> > http://google.com.
> > 
> 
> Works for me, but please leave the explanatory comment in place.

Done.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:213: //
MockBrowserWindow mock_browser_window_;
On 2014/07/24 18:14:38, Charlie Reis wrote:
> nit: Remove commented out code.

Done.

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:290:
ConfirmationRequiredForNonsecureSignin) {
On 2014/07/24 18:47:11, Charlie Reis wrote:
> On 2014/07/24 18:31:21, guohui wrote:
> > Signin page has an embedded gaia iframe, which loads the gaia signin page.
> Above
> > I set the gaia URL to the embedded test server, which runs on HTTP by
default.
> 
> Ah, ok. Please explain that in a comment here, since it's not easy to spot.

Done.

[xiyuan, 2014-07-25 18:07:23]

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:288: #if
!defined(OS_CHROMEOS)
On 2014/07/24 18:31:21, guohui wrote:
> Because the confirmation dialog path in inline_login_handler_impl.cc is only
> executed on desktop. I suspect CrOS has similar issues as Chrome desktop, but
> their implementation is slightly different, thus I will delegate to CrOS team
to
> fix it.
> 
> +xiyuan@ to confirm.

Cros mirror uses inline_login_handler_chromeos.cc. It should probably be
consistent with Cros login and just block all insecure content.

hui@, do you mind adding "/CrOS/.test(navigator.userAgent)" check in initSAML_
in main.js to do that?

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:180:
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
We cannot do it like this. Check out the comments in embedded_test_server.h,
browser_tests part.

https://code.google.com/p/chromium/codesearch#chromium/src/net/test/embedded_...

https://codereview.chromium.org/418043002/diff/40001/google_apis/gaia/fake_ga...
File google_apis/gaia/fake_gaia.cc (right):

https://codereview.chromium.org/418043002/diff/40001/google_apis/gaia/fake_ga...
google_apis/gaia/fake_gaia.cc:147: FakeGaia::MergeSessionParams params;
nit: wrong indent

[Roger Tawa OOO till Jul 10th, 2014-07-28 14:27:50]

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/gaia_auth/background.js (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/resources...
chrome/browser/resources/gaia_auth/background.js:73: {urls: ['*://*/*',
this.CONTINUE_URL_BASE + '*'], types: ['sub_frame']},
Why append *?

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/signin/si...
File chrome/browser/signin/signin_promo.cc (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/signin/si...
chrome/browser/signin/signin_promo.cc:172:
"chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html";
Put this in header file?  Also used in oobe_ui.cc, enrollment_screen_handler.cc,
inline_login_handler_impl.cc

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/login_ui_service.h (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/login_ui_service.h:45: virtual void
UntrustedSigninUIShown() {}
Is this shown instead of or as well as OnLoginUIShown()?  Can you add
documentation?

Rename to UntrustedLoginUIShown() for consistency.

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/login_ui_service.h:72: void
UntrustedSigninUIShown();
Rename as well.

[Charlie Reis, 2014-08-05 18:41:01]

What's the status of this?  It's important to get
https://codereview.chromium.org/317093002/ landed.

[guohui, 2014-08-06 20:45:17]

sorry just back from vacation, will resume working on this tmrw at the latest.

[guohui, 2014-08-06 20:45:22]

sorry just back from vacation, will resume working on this tmrw at the latest.

[guohui, 2014-08-06 20:45:22]

sorry just back from vacation, will resume working on this tmrw at the latest.

[guohui, 2014-08-06 20:45:22]

sorry just back from vacation, will resume working on this tmrw at the latest.

[guohui, 2014-08-07 19:53:34]

Sorry for the long delay, could you please take another look?

Thanks!

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:288: #if
!defined(OS_CHROMEOS)
i think for CrOS there is more to clean up, currently CrOS is filtering unsecure
requests from JS, as suggested by nasko@, it is not secure to do it in JS, thus
all http-filtering related code in JS should be removed. Also desktop has
different treatment for unsecure signin than CrOS, instead of blocking the
content or showing some banner, desktop shows a native confirmation dialog
afterwards. Not sure what CrOS want to do here, thus I prefer to leave the code
as it is and fix CrOS implementation separately.

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/resources...
File chrome/browser/resources/gaia_auth/background.js (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/resources...
chrome/browser/resources/gaia_auth/background.js:73: {urls: ['*://*/*',
this.CONTINUE_URL_BASE + '*'], types: ['sub_frame']},
On 2014/07/28 14:27:50, Roger Tawa (build sheriff) wrote:
> Why append *?

we need * to match any trailing URL parameters.

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/signin/si...
File chrome/browser/signin/signin_promo.cc (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/signin/si...
chrome/browser/signin/signin_promo.cc:172:
"chrome-extension://mfffpogegjflfpflabcdkioaeobkgjik/success.html";
On 2014/07/28 14:27:50, Roger Tawa (build sheriff) wrote:
> Put this in header file?  Also used in oobe_ui.cc,
enrollment_screen_handler.cc,
> inline_login_handler_impl.cc

Done.

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:180:
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
On 2014/07/25 18:07:23, xiyuan wrote:
> We cannot do it like this. Check out the comments in embedded_test_server.h,
> browser_tests part.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/test/embedded_...

Done.

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/signin/login_ui_service.h (right):

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/login_ui_service.h:45: virtual void
UntrustedSigninUIShown() {}
On 2014/07/28 14:27:50, Roger Tawa (build sheriff) wrote:
> Is this shown instead of or as well as OnLoginUIShown()?  Can you add
> documentation?
> 
> Rename to UntrustedLoginUIShown() for consistency.

The documentation has been updated to cover all login related UIs, instead of
just login UIs.

Method renamed.

https://codereview.chromium.org/418043002/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/signin/login_ui_service.h:72: void
UntrustedSigninUIShown();
for consistency i think we should not add On here.

https://codereview.chromium.org/418043002/diff/40001/google_apis/gaia/fake_ga...
File google_apis/gaia/fake_gaia.cc (right):

https://codereview.chromium.org/418043002/diff/40001/google_apis/gaia/fake_ga...
google_apis/gaia/fake_gaia.cc:147: FakeGaia::MergeSessionParams params;
On 2014/07/25 18:07:23, xiyuan wrote:
> nit: wrong indent

Done.

[guohui, 2014-08-07 22:31:24]

https://codereview.chromium.org/418043002/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:184: //
embedded_test_server()->StopThread();
@xiyuan, your suggested change of splitting the test server startup into two
parts somehow broke all tests in this class that call WaitUntilUIReady. They
would timeout at "ASSERT_TRUE(message_queue.WaitForMessage(&message));" with
your suggested change, and they all pass without.

Any idea what is wrong?

[xiyuan, 2014-08-07 23:07:19]

https://codereview.chromium.org/418043002/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc (right):

https://codereview.chromium.org/418043002/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc:184: //
embedded_test_server()->StopThread();
On 2014/08/07 22:31:23, guohui wrote:
> @xiyuan, your suggested change of splitting the test server startup into two
> parts somehow broke all tests in this class that call WaitUntilUIReady. They
> would timeout at "ASSERT_TRUE(message_queue.WaitForMessage(&message));" with
> your suggested change, and they all pass without.
> 
> Any idea what is wrong?

Splitting into two parts is required, per oshima's CL
https://codereview.chromium.org/83633004. 

Not sure what is wrong. Test server should behave the same in either way. Can
you add some log in FakeGaia::HandleRequest and see if that actually gets hit?
If it is, then maybe add some console.log in the injected JS and see if
gaia_auth_host is properly loaded etc.

[Roger Tawa OOO till Jul 10th, 2014-08-08 16:23:36]

lgtm

[guohui, 2014-08-11 21:35:10]

It seems that splitting the embedded test server startup into two parts has
never worked on windows platforms.

@xiyuan, oshima, please review my fix in embedded_test_server.cc, it is hacky
but I can't think of a better way without changing the interface of the
low-level class StreamListenSocket.

[xiyuan, 2014-08-11 22:00:41]

https://codereview.chromium.org/418043002/diff/120001/net/test/embedded_test_...
File net/test/embedded_test_server/embedded_test_server.cc (right):

https://codereview.chromium.org/418043002/diff/120001/net/test/embedded_test_...
net/test/embedded_test_server/embedded_test_server.cc:212:
TCPListenSocket::CreateAndBind("127.0.0.1", port_);
Thank you for digging through the problem. 

Maybe we want to fix it in HttpListenSocket (this seems to be a test class
defined in this file) by calling or exposing WatchSocket. Rebinding to |port_|
would be flaky on test bots where many test servers are running and might steal
the |port_| once it is released.

[oshima, 2014-08-12 04:27:25]

https://codereview.chromium.org/418043002/diff/120001/net/test/embedded_test_...
File net/test/embedded_test_server/embedded_test_server.cc (right):

https://codereview.chromium.org/418043002/diff/120001/net/test/embedded_test_...
net/test/embedded_test_server/embedded_test_server.cc:212:
TCPListenSocket::CreateAndBind("127.0.0.1", port_);
On 2014/08/11 22:00:41, xiyuan wrote:
> Thank you for digging through the problem. 
> 
> Maybe we want to fix it in HttpListenSocket (this seems to be a test class
> defined in this file) by calling or exposing WatchSocket. Rebinding to |port_|
> would be flaky on test bots where many test servers are running and might
steal
> the |port_| once it is released.

sounds good to me.

If this was broken, I'm curious to know why tests were passing?

[guohui, 2014-08-12 15:55:19]

https://codereview.chromium.org/418043002/diff/120001/net/test/embedded_test_...
File net/test/embedded_test_server/embedded_test_server.cc (right):

https://codereview.chromium.org/418043002/diff/120001/net/test/embedded_test_...
net/test/embedded_test_server/embedded_test_server.cc:212:
TCPListenSocket::CreateAndBind("127.0.0.1", port_);
thanks xiyuan, missed that =)

i think it is because the approach of splitting into two parts has only been
implemented in CrOS tests so far.

[guohui, 2014-08-12 18:31:39]

+mtomasz for owner review of embedded_test_server

[xiyuan, 2014-08-12 18:35:54]

https://codereview.chromium.org/418043002/diff/220001/net/test/embedded_test_...
File net/test/embedded_test_server/embedded_test_server.cc (right):

https://codereview.chromium.org/418043002/diff/220001/net/test/embedded_test_...
net/test/embedded_test_server/embedded_test_server.cc:105: void
HttpListenSocket::ListenOnIOThread() {
This looks the same as HttpListenSocket::Listen. Is it needed?

[guohui, 2014-08-12 19:01:17]

https://codereview.chromium.org/418043002/diff/220001/net/test/embedded_test_...
File net/test/embedded_test_server/embedded_test_server.cc (right):

https://codereview.chromium.org/418043002/diff/220001/net/test/embedded_test_...
net/test/embedded_test_server/embedded_test_server.cc:105: void
HttpListenSocket::ListenOnIOThread() {
On 2014/08/12 18:35:54, xiyuan wrote:
> This looks the same as HttpListenSocket::Listen. Is it needed?

Sorry this patch is broken =(

the #if block should only be added to ListenOnIOThread, not Listen. Fixed in the
patch 8.

[mtomasz, 2014-08-13 01:37:51]

lgtm for embedded_test_server.*

[xiyuan, 2014-08-13 04:06:25]

lgtm

[Charlie Reis, 2014-08-13 14:42:46]

Rubber stamp LGTM.  Thanks for finishing it up!

[bezzakonnik, 2014-08-29 08:07:14]

bezzakonnik@gmail.com changed reviewers:
+ bezzakonnik@gmail.com

[Charlie Reis, 2014-08-29 16:04:36]

I'm not sure what this latest email was for, but can this test be landed now?

On 2014/08/29 08:07:14, bezzakonnik wrote:

[guohui, 2014-08-29 17:06:33]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-29 17:07:37]

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

[guohui, 2014-08-29 18:42:51]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-29 18:43:46]

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

[guohui, 2014-08-29 20:57:58]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-08-30 00:11:16]

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_...)
  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-30 00:49:56]

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

[commit-bot: I haz the power, 2014-08-30 00:49:58]

Try jobs failed on following builders:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[guohui, 2014-09-03 23:53:02]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-09-03 23:54:41]

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

[commit-bot: I haz the power, 2014-09-04 01:17:12]

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

[commit-bot: I haz the power, 2014-09-04 01:17:14]

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_...)

[guohui, 2014-09-04 15:20:54]

Patchset #12 (id:410001) has been deleted

[guohui, 2014-09-04 15:21:51]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-09-04 15:23:42]

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

[guohui, 2014-09-04 16:11:29]

Patchset #13 (id:450001) has been deleted

[guohui, 2014-09-04 16:11:36]

The CQ bit was unchecked by guohui@chromium.org

[guohui, 2014-09-04 16:11:37]

The CQ bit was checked by guohui@chromium.org

[guohui, 2014-09-04 16:11:38]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-09-04 16:16:23]

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

[commit-bot: I haz the power, 2014-09-04 17:10:37]

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

[commit-bot: I haz the power, 2014-09-04 17:10:39]

Try jobs failed on following builders:
  mac_gpu_retina_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu_retina_tr...)
  win_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)

[guohui, 2014-09-04 17:18:02]

Patchset #12 (id:430001) has been deleted

[guohui, 2014-09-04 17:20:28]

guohui@chromium.org changed reviewers:
+ maruel@chromium.org

[guohui, 2014-09-04 17:21:14]

Fixed broken tests on CQ due to missing dependency on google_apis/test.

Add maruel@ for owner review of browser_tests.isolate.

[guohui, 2014-09-05 12:50:15]

Patchset #12 (id:470001) has been deleted

[M-A Ruel, 2014-09-05 17:54:48]

lgtm

[guohui, 2014-09-05 17:58:44]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-09-05 18:00:44]

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

[commit-bot: I haz the power, 2014-09-05 18:53:45]

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

[commit-bot: I haz the power, 2014-09-05 18:53:46]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[guohui, 2014-09-05 19:43:44]

The CQ bit was checked by guohui@chromium.org

[commit-bot: I haz the power, 2014-09-05 19:44:48]

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

[commit-bot: I haz the power, 2014-09-05 20:11:55]

Committed patchset #12 (id:490001) as 01479d4cad7c529336c31009b9c63ce49849c5ef

[commit-bot: I haz the power, 2014-09-10 03:41:18]

Patchset 12 (id:??) landed as
https://crrev.com/4bdcf21dcc119bdf3bea37e4a97e6ef0c05517bb
Cr-Commit-Position: refs/heads/master@{#293572}