Add HTTP status code to navigation data structures

Add HTTP status code to navigation data structures

This is a reboot of https://codereview.chromium.org/11054025/

BUG=153936, 260276
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219954

[pval...(no longer on Chromium), 2013-08-15 01:58:40]

R=zea for now as a sanity check. I'll add the other reviewers on the next pass.

[Nicolas Zea, 2013-08-15 18:25:40]

Some nits. Could you update proto_value_conversions.cc as well with the new
proto field serialization?

https://codereview.chromium.org/22837005/diff/10003/chrome/browser/sessions/s...
File chrome/browser/sessions/session_restore_browsertest.cc (right):

https://codereview.chromium.org/22837005/diff/10003/chrome/browser/sessions/s...
chrome/browser/sessions/session_restore_browsertest.cc:312: GURL
url2("about:version");
Any reason you needed to change this? what status code does title2.html return?
It seems like you could just have the test make sure it restores whatever status
code title2.html returned?

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
File content/browser/web_contents/navigation_controller_impl_unittest.cc
(right):

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
content/browser/web_contents/navigation_controller_impl_unittest.cc:358: //
Neither the timestamp not the status code hould not have been set yet.
"nor the status code should have been"

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
File content/browser/web_contents/navigation_entry_impl.h (right):

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
content/browser/web_contents/navigation_entry_impl.h:87: 
remove newline

[pval...(no longer on Chromium), 2013-08-20 15:28:10]

proto_value_conversions.cc updated. Please take a look.

https://codereview.chromium.org/22837005/diff/10003/chrome/browser/sessions/s...
File chrome/browser/sessions/session_restore_browsertest.cc (right):

https://codereview.chromium.org/22837005/diff/10003/chrome/browser/sessions/s...
chrome/browser/sessions/session_restore_browsertest.cc:312: GURL
url2("about:version");
On 2013/08/15 18:25:40, Nicolas Zea wrote:
> Any reason you needed to change this? what status code does title2.html
return?
> It seems like you could just have the test make sure it restores whatever
status
> code title2.html returned?

The rationale for this is that title1.html returns a 0 status code (since it's a
local file?) and that is also the default http_status_code for the
NavigationEntry. So expecting a 0 status code from the NavigationEntry doesn't
really test that the correct value is preserved.

Thoughts on going forward?

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
File content/browser/web_contents/navigation_controller_impl_unittest.cc
(right):

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
content/browser/web_contents/navigation_controller_impl_unittest.cc:358: //
Neither the timestamp not the status code hould not have been set yet.
On 2013/08/15 18:25:40, Nicolas Zea wrote:
> "nor the status code should have been"

yikes. thanks for catching my error here.

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
File content/browser/web_contents/navigation_entry_impl.h (right):

https://codereview.chromium.org/22837005/diff/10003/content/browser/web_conte...
content/browser/web_contents/navigation_entry_impl.h:87: 
On 2013/08/15 18:25:40, Nicolas Zea wrote:
> remove newline

Done.

[Nicolas Zea, 2013-08-20 18:33:43]

LGTM

https://codereview.chromium.org/22837005/diff/10003/chrome/browser/sessions/s...
File chrome/browser/sessions/session_restore_browsertest.cc (right):

https://codereview.chromium.org/22837005/diff/10003/chrome/browser/sessions/s...
chrome/browser/sessions/session_restore_browsertest.cc:312: GURL
url2("about:version");
On 2013/08/20 15:28:10, pvalenzuela wrote:
> On 2013/08/15 18:25:40, Nicolas Zea wrote:
> > Any reason you needed to change this? what status code does title2.html
> return?
> > It seems like you could just have the test make sure it restores whatever
> status
> > code title2.html returned?
> 
> The rationale for this is that title1.html returns a 0 status code (since it's
a
> local file?) and that is also the default http_status_code for the
> NavigationEntry. So expecting a 0 status code from the NavigationEntry doesn't
> really test that the correct value is preserved.
> 
> Thoughts on going forward?

Ah, makes sense.

[pval...(no longer on Chromium), 2013-08-20 20:16:37]

Adding owners for other dirs:

android_webview: mkosiba@
sessions: marja@
content: avi@

[Avi (use Gerrit), 2013-08-20 20:26:58]

On 2013/08/20 20:16:37, pvalenzuela wrote:
> Adding owners for other dirs:
> 
> android_webview: mkosiba@
> sessions: marja@
> content: avi@

Content LGTM

[mkosiba (inactive), 2013-08-21 08:53:18]

LGTM for android_webview, +CC FYI boliu who wrote the code

[marja, 2013-08-21 12:05:43]

sessions/ lgtm

[commit-bot: I haz the power, 2013-08-28 00:04:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pvalenzuela@chromium.org/22837005/66001

[commit-bot: I haz the power, 2013-08-28 00:38:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pvalenzuela@chromium.org/22837005/66001

[commit-bot: I haz the power, 2013-08-28 01:41:53]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[commit-bot: I haz the power, 2013-08-28 05:02:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pvalenzuela@chromium.org/22837005/66001

[commit-bot: I haz the power, 2013-08-28 07:51:49]

Change committed as 219954