OOPIF: Fix remote frame navigations from extension pages.

OOPIF: Fix remote frame navigations from extension pages.

Navigating a remote frame currently ends up in
RenderFrameProxyHost::OnOpenURL, which initiates the navigation via
RenderFrameHostImpl::OpenURL and then NavigatorImpl::RequestOpenURL.
This is problematic in the case where an extension background page
navigates a remote frame, since in this case the plumbing will reach
WebContentsDelegate::OpenURLFromTab, which will drop the navigation on
the floor.  This is because the WebContents's delegate in this case
will be ExtensionHost which doesn't implement OpenURLFromTab, and the
default implementation in WebContentsDelegate::OpenURLFromTab has been
removed in https://codereview.chromium.org/1563593002.  See issues
562389 and 560510 for some more context.

This CL changes navigations via RenderFrameProxies to instead use
NavigatorImpl::RequestTransferURL to avoid hitting the path above.

BUG=588096
TEST=Added coverage to ExtensionApiTest.ContentScriptExtensionProcess, which should pass with --isolate-extensions.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/9ab970c98034692afa06d7f4bb1ff813d2ffd3f2
Cr-Commit-Position: refs/heads/master@{#377832}

[alexmos, 2016-02-20 01:52:16]

Description was changed from

==========
OOPIF: Fix remote frame navigations from extension pages.

Navigating a remote frame currently ends up in
RenderFrameProxyHost::OnOpenURL, which initiates the navigation via
RenderFrameHostImpl::OpenURL and then NavigatorImpl::RequestOpenURL.
This is problematic in the case where an extension background page
navigates a remote frame, since in this case the plumbing will reach
WebContentsDelegate::OpenURLFromTab, which will drop the navigation on
the floor.  This is because the WebContents's delegate in this case
will be ExtensionHost which doesn't implement OpenURLFromTab, and the
default implementation in WebContentsDelegate::OpenURLFromTab has been
removed in https://codereview.chromium.org/1563593002.  See issues
562389 and 560510 for some more context.

This CL changes navigations via RenderFrameProxies to instead use
NavigatorImpl::RequestTransferURL to avoid hitting the path above.

BUG=588096
TEST=Added coverage to ExtensionApiTest.ContentScriptExtensionProcess, which
should pass with --isolate-extensions.
==========

to

==========
OOPIF: Fix remote frame navigations from extension pages.

Navigating a remote frame currently ends up in
RenderFrameProxyHost::OnOpenURL, which initiates the navigation via
RenderFrameHostImpl::OpenURL and then NavigatorImpl::RequestOpenURL.
This is problematic in the case where an extension background page
navigates a remote frame, since in this case the plumbing will reach
WebContentsDelegate::OpenURLFromTab, which will drop the navigation on
the floor.  This is because the WebContents's delegate in this case
will be ExtensionHost which doesn't implement OpenURLFromTab, and the
default implementation in WebContentsDelegate::OpenURLFromTab has been
removed in https://codereview.chromium.org/1563593002.  See issues
562389 and 560510 for some more context.

This CL changes navigations via RenderFrameProxies to instead use
NavigatorImpl::RequestTransferURL to avoid hitting the path above.

BUG=588096
TEST=Added coverage to ExtensionApiTest.ContentScriptExtensionProcess, which
should pass with --isolate-extensions.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[alexmos, 2016-02-20 02:42:47]

alexmos@chromium.org changed reviewers:
+ creis@chromium.org

[alexmos, 2016-02-20 02:42:47]

Charlie, can you please take a look when you get a chance?

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (left):

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:596: // TODO(creis): Move this to
RenderFrameProxyHost::OpenURL.
I'm assuming this check will have to stay in addition to the one I added in
RFPH::OnOpenURL until we are ready to rip out swapped out everywhere?

[Charlie Reis, 2016-02-21 05:47:23]

Thanks!  LGTM, though it looks like there might be some layout test failures?

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (left):

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:596: // TODO(creis): Move this to
RenderFrameProxyHost::OpenURL.
On 2016/02/20 02:42:47, alexmos wrote:
> I'm assuming this check will have to stay in addition to the one I added in
> RFPH::OnOpenURL until we are ready to rip out swapped out everywhere?

Sure, we can remove it when Nasko removes the rest of swapped out.

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_proxy_host.cc (right):

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_proxy_host.cc:264:
params.should_replace_current_entry);
Looking at RenderFrameProxy::OpenURL, it looks like the only thing we don't pass
through here is params.user_gesture.  I'm not sure what impact that might have
when navigating another frame, but maybe we should add a TODO to figure it out
and possibly pass it through?

[alexmos, 2016-02-23 00:32:11]

Thanks!  Digging into those layout test failures actually revealed a couple of
interesting separate issues:

1) We were skipping a Blink security check when navigating remote frames to
javascript: URLs.  I've put up a separate CL to fix those
(https://codereview.chromium.org/1718403002/), which I believe might also help
fix issue 582201.  I'll aim to land that fix before this one.

2) Despite getting 1) fixed, I still sometimes hit a DCHECK in
frame_navigation_entry for the same layout tests when running several of them
through run-webkit-tests with --site-per-process:

ERR: [14285:14285:0222/155054:2437811853934:FATAL:frame_navigation_entry.cc(81)]
Check failed: item_sequence_number_ == -1 || item_sequence_number_ ==
item_sequence_number. \n
ERR: #0 0x7feb2c22b12e base::debug::StackTrace::StackTrace()\n
ERR: #1 0x7feb2c28847f logging::LogMessage::~LogMessage()\n
ERR: #2 0x7feb2d79edfe
content::FrameNavigationEntry::set_item_sequence_number()\n
ERR: #3 0x7feb2d7c00f1
content::NavigationControllerImpl::RendererDidNavigateToNewPage()\n
ERR: #4 0x7feb2d7bebe0
content::NavigationControllerImpl::RendererDidNavigate()\n
ERR: #5 0x7feb2d7e4698 content::NavigatorImpl::DidNavigate()\n
ERR: #6 0x7feb2d7edf8f
content::RenderFrameHostImpl::OnDidCommitProvisionalLoad()\n

The simplest repro I found is to run
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttribute.html
twice (--repeat-each=2 to run-webkit-tests) with --site-per-process.  That test
creates an OOPIF and attempts to navigate it to four different javascript: URL
(all aborted), and then navigates it to a malformed URL (not found) which
commits.  The test runner then navigates the main page to about:blank after the
test is done.  After doing this twice, I get this DCHECK while committing the
final about:blank.  The mismatched item sequence numbers are off by 2 in this
case (e.g., item_sequence_number_=1456185046809476 and
item_sequence_number=1456185046809478).

Do you know if this is this a known issue?  Is it ok to file a separate bug for
this and not worry about it for this CL?  These tests already have Failure
expectations for site-per-process; I'd need to add Crash expectations to keep
everything green.

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
File content/browser/frame_host/render_frame_proxy_host.cc (right):

https://codereview.chromium.org/1715993002/diff/20001/content/browser/frame_h...
content/browser/frame_host/render_frame_proxy_host.cc:264:
params.should_replace_current_entry);
On 2016/02/21 05:47:23, Charlie Reis (slow til Mar 7) wrote:
> Looking at RenderFrameProxy::OpenURL, it looks like the only thing we don't
pass
> through here is params.user_gesture.  I'm not sure what impact that might have
> when navigating another frame, but maybe we should add a TODO to figure it out
> and possibly pass it through?

Done.

[Charlie Reis, 2016-02-25 00:27:37]

On 2016/02/23 00:32:11, alexmos wrote:
> Thanks!  Digging into those layout test failures actually revealed a couple of
> interesting separate issues:
> 
> 1) We were skipping a Blink security check when navigating remote frames to
> javascript: URLs.  I've put up a separate CL to fix those
> (https://codereview.chromium.org/1718403002/), which I believe might also help
> fix issue 582201.  I'll aim to land that fix before this one.

Great, glad that landed!

> 2) Despite getting 1) fixed, I still sometimes hit a DCHECK in
> frame_navigation_entry for the same layout tests when running several of them
> through run-webkit-tests with --site-per-process:
> 
> ERR:
[14285:14285:0222/155054:2437811853934:FATAL:frame_navigation_entry.cc(81)]

Hmm, did you have a modified copy of frame_navigation_entry.cc?  That check
looks like it's on line 61 on trunk, and the file doesn't seem to have changed
recently.  Guessing that's not relevant, but just curious.

> Check failed: item_sequence_number_ == -1 || item_sequence_number_ ==
> item_sequence_number. \n
> ERR: #0 0x7feb2c22b12e base::debug::StackTrace::StackTrace()\n
> ERR: #1 0x7feb2c28847f logging::LogMessage::~LogMessage()\n
> ERR: #2 0x7feb2d79edfe
> content::FrameNavigationEntry::set_item_sequence_number()\n
> ERR: #3 0x7feb2d7c00f1
> content::NavigationControllerImpl::RendererDidNavigateToNewPage()\n
> ERR: #4 0x7feb2d7bebe0
> content::NavigationControllerImpl::RendererDidNavigate()\n
> ERR: #5 0x7feb2d7e4698 content::NavigatorImpl::DidNavigate()\n
> ERR: #6 0x7feb2d7edf8f
> content::RenderFrameHostImpl::OnDidCommitProvisionalLoad()\n

The premise of the check is that the ISN and DSN shouldn't change on a
FrameNavigationEntry once they've been assigned, which means that navigations
with replacement will sometimes need to replace the FNE rather than update it. 
This case is a bit surprising to me, since it's RendererDidNavigateToNewPage. 
That should create a new entry, optionally replacing the current entry.  How did
it already have an ISN?  Was it set during DidStartProvisionalLoad?


> 
> The simplest repro I found is to run
>
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttribute.html
> twice (--repeat-each=2 to run-webkit-tests) with --site-per-process.  That
test
> creates an OOPIF and attempts to navigate it to four different javascript: URL
> (all aborted), and then navigates it to a malformed URL (not found) which
> commits.  The test runner then navigates the main page to about:blank after
the
> test is done.  After doing this twice, I get this DCHECK while committing the
> final about:blank.  The mismatched item sequence numbers are off by 2 in this
> case (e.g., item_sequence_number_=1456185046809476 and
> item_sequence_number=1456185046809478).

Yeah, when they differ it tends to be by 2, since the ISN and DSN get
incremented back-to-back in most cases (and come from the same counter).  I'm
not sure what's actually going on here-- we should probably look more closely in
a debugger.

> Do you know if this is this a known issue?

Hmm, no, I haven't seen that DCHECK fail before.

>  Is it ok to file a separate bug for
> this and not worry about it for this CL?  These tests already have Failure
> expectations for site-per-process; I'd need to add Crash expectations to keep
> everything green.

Sure.  This CL seems worth landing and we can follow up on those test
failures/crashes.  Thanks!

[alexmos, 2016-02-25 23:07:40]

On 2016/02/25 00:27:37, Charlie Reis (slow til Mar 7) wrote:
> On 2016/02/23 00:32:11, alexmos wrote:
> > Thanks!  Digging into those layout test failures actually revealed a couple
of
> > interesting separate issues:
> > 
> > 1) We were skipping a Blink security check when navigating remote frames to
> > javascript: URLs.  I've put up a separate CL to fix those
> > (https://codereview.chromium.org/1718403002/), which I believe might also
help
> > fix issue 582201.  I'll aim to land that fix before this one.
> 
> Great, glad that landed!
> 
> > 2) Despite getting 1) fixed, I still sometimes hit a DCHECK in
> > frame_navigation_entry for the same layout tests when running several of
them
> > through run-webkit-tests with --site-per-process:
> > 
> > ERR:
> [14285:14285:0222/155054:2437811853934:FATAL:frame_navigation_entry.cc(81)]
> 
> Hmm, did you have a modified copy of frame_navigation_entry.cc?  That check
> looks like it's on line 61 on trunk, and the file doesn't seem to have changed
> recently.  Guessing that's not relevant, but just curious.

Indeed, I had some debugging info in frame_navigation_entry.cc, which changed
the line.  It should be line 61.

> 
> > Check failed: item_sequence_number_ == -1 || item_sequence_number_ ==
> > item_sequence_number. \n
> > ERR: #0 0x7feb2c22b12e base::debug::StackTrace::StackTrace()\n
> > ERR: #1 0x7feb2c28847f logging::LogMessage::~LogMessage()\n
> > ERR: #2 0x7feb2d79edfe
> > content::FrameNavigationEntry::set_item_sequence_number()\n
> > ERR: #3 0x7feb2d7c00f1
> > content::NavigationControllerImpl::RendererDidNavigateToNewPage()\n
> > ERR: #4 0x7feb2d7bebe0
> > content::NavigationControllerImpl::RendererDidNavigate()\n
> > ERR: #5 0x7feb2d7e4698 content::NavigatorImpl::DidNavigate()\n
> > ERR: #6 0x7feb2d7edf8f
> > content::RenderFrameHostImpl::OnDidCommitProvisionalLoad()\n
> 
> The premise of the check is that the ISN and DSN shouldn't change on a
> FrameNavigationEntry once they've been assigned, which means that navigations
> with replacement will sometimes need to replace the FNE rather than update it.

> This case is a bit surprising to me, since it's RendererDidNavigateToNewPage. 
> That should create a new entry, optionally replacing the current entry.  How
did
> it already have an ISN?  Was it set during DidStartProvisionalLoad?
> 
> 
> > 
> > The simplest repro I found is to run
> >
>
http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttribute.html
> > twice (--repeat-each=2 to run-webkit-tests) with --site-per-process.  That
> test
> > creates an OOPIF and attempts to navigate it to four different javascript:
URL
> > (all aborted), and then navigates it to a malformed URL (not found) which
> > commits.  The test runner then navigates the main page to about:blank after
> the
> > test is done.  After doing this twice, I get this DCHECK while committing
the
> > final about:blank.  The mismatched item sequence numbers are off by 2 in
this
> > case (e.g., item_sequence_number_=1456185046809476 and
> > item_sequence_number=1456185046809478).
> 
> Yeah, when they differ it tends to be by 2, since the ISN and DSN get
> incremented back-to-back in most cases (and come from the same counter).  I'm
> not sure what's actually going on here-- we should probably look more closely
in
> a debugger.
> 
> > Do you know if this is this a known issue?
> 
> Hmm, no, I haven't seen that DCHECK fail before.
> 
> >  Is it ok to file a separate bug for
> > this and not worry about it for this CL?  These tests already have Failure
> > expectations for site-per-process; I'd need to add Crash expectations to
keep
> > everything green.
> 
> Sure.  This CL seems worth landing and we can follow up on those test
> failures/crashes.  Thanks!

Great, I've filed https://crbug.com/590011 and can follow up there with more
details and answers to your questions.  Meanwhile, I'll land this, and I've
changed expectations for these tests to Crash in the latest PS (they shouldn't
fail any more now that I've fixed issue 582201).

[Charlie Reis, 2016-02-25 23:17:32]

Thanks, LGTM.

[alexmos, 2016-02-26 00:48:15]

The CQ bit was checked by alexmos@chromium.org

[commit-bot: I haz the power, 2016-02-26 00:54:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1715993002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1715993002/140001

[commit-bot: I haz the power, 2016-02-26 01:24:07]

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

[commit-bot: I haz the power, 2016-02-26 01:24:08]

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

[alexmos, 2016-02-26 01:24:36]

The CQ bit was checked by alexmos@chromium.org

[commit-bot: I haz the power, 2016-02-26 01:31:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1715993002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1715993002/140001

[commit-bot: I haz the power, 2016-02-26 03:37:11]

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

[commit-bot: I haz the power, 2016-02-26 03:37:13]

Failed to apply patch for
third_party/WebKit/LayoutTests/FlagExpectations/site-per-process:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/LayoutTests/FlagExpectations/site-per-process:46
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/LayoutTests/FlagExpectations/site-per-process' with
conflicts.
  U third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Patch:       third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
Index: third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
b/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
index
def48670400ca4ee24117f19ab160e9d18a37f89..70b648fbb7a6627be2d891ba47a5ecbc06cad9d4
100644
--- a/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
+++ b/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
@@ -46,16 +46,16 @@ http/tests/security/cross-frame-access-call.html [ Failure ]
 http/tests/security/cross-frame-access-dispatchEvent.html [ Failure ]
 
 # https://crbug.com/582201 - missing console message about blocked XSS.
-http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-getAttribute-value.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-htmldom.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttribute.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNode.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNS.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-getAttribute-value.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-htmldom.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttribute.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNode.html
[ Failure ]
-http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNS.html
[ Failure ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-getAttribute-value.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-htmldom.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttribute.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNode.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-frame-src-setAttributeNS.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-getAttribute-value.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-htmldom.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttribute.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNode.html
[ Crash ]
+http/tests/security/javascriptURL/javascriptURL-execution-context-iframe-src-setAttributeNS.html
[ Crash ]
 http/tests/security/xss-DENIED-iframe-src-alias.html [ Crash Failure ]
 http/tests/security/xss-DENIED-javascript-with-spaces.html [ Failure ]

[commit-bot: I haz the power, 2016-02-26 03:40:50]

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

[alexmos, 2016-02-26 04:52:00]

The CQ bit was checked by alexmos@chromium.org

[alexmos, 2016-02-26 04:52:00]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/1715993002/#ps160001
(title: "Resolve mid-air collision in FlagExpectations/site-per-process")

[commit-bot: I haz the power, 2016-02-26 04:53:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1715993002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1715993002/160001

[commit-bot: I haz the power, 2016-02-26 06:40:58]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-02-26 06:42:08]

Description was changed from

==========
OOPIF: Fix remote frame navigations from extension pages.

Navigating a remote frame currently ends up in
RenderFrameProxyHost::OnOpenURL, which initiates the navigation via
RenderFrameHostImpl::OpenURL and then NavigatorImpl::RequestOpenURL.
This is problematic in the case where an extension background page
navigates a remote frame, since in this case the plumbing will reach
WebContentsDelegate::OpenURLFromTab, which will drop the navigation on
the floor.  This is because the WebContents's delegate in this case
will be ExtensionHost which doesn't implement OpenURLFromTab, and the
default implementation in WebContentsDelegate::OpenURLFromTab has been
removed in https://codereview.chromium.org/1563593002.  See issues
562389 and 560510 for some more context.

This CL changes navigations via RenderFrameProxies to instead use
NavigatorImpl::RequestTransferURL to avoid hitting the path above.

BUG=588096
TEST=Added coverage to ExtensionApiTest.ContentScriptExtensionProcess, which
should pass with --isolate-extensions.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
OOPIF: Fix remote frame navigations from extension pages.

Navigating a remote frame currently ends up in
RenderFrameProxyHost::OnOpenURL, which initiates the navigation via
RenderFrameHostImpl::OpenURL and then NavigatorImpl::RequestOpenURL.
This is problematic in the case where an extension background page
navigates a remote frame, since in this case the plumbing will reach
WebContentsDelegate::OpenURLFromTab, which will drop the navigation on
the floor.  This is because the WebContents's delegate in this case
will be ExtensionHost which doesn't implement OpenURLFromTab, and the
default implementation in WebContentsDelegate::OpenURLFromTab has been
removed in https://codereview.chromium.org/1563593002.  See issues
562389 and 560510 for some more context.

This CL changes navigations via RenderFrameProxies to instead use
NavigatorImpl::RequestTransferURL to avoid hitting the path above.

BUG=588096
TEST=Added coverage to ExtensionApiTest.ContentScriptExtensionProcess, which
should pass with --isolate-extensions.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/9ab970c98034692afa06d7f4bb1ff813d2ffd3f2
Cr-Commit-Position: refs/heads/master@{#377832}
==========

[commit-bot: I haz the power, 2016-02-26 06:42:09]

Patchset 9 (id:??) landed as
https://crrev.com/9ab970c98034692afa06d7f4bb1ff813d2ffd3f2
Cr-Commit-Position: refs/heads/master@{#377832}