Creating contents state byte array before opening stream

Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815
Committed: https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc
Cr-Commit-Position: refs/heads/master@{#405299}

[andrhung, 2016-07-08 22:26:18]

Description was changed from

==========
Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815
==========

to

==========
Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815
==========

[andrhung, 2016-07-08 22:26:18]

andrhung@amazon.com changed reviewers:
+ dfalcantara@chromium.org, dtrainor@google.com, wnwen@chromium.org

[gone, 2016-07-11 17:50:57]

Was this ready to review?  Saw this in my review queue but didn't get an email
about it (e.g. Publish+Mail Comments with the new reviewers).

[andrhung, 2016-07-11 17:58:01]

This change is to fix the issue we discussed regarding saving TabStates with
mapped byte buffers.

[andrhung, 2016-07-11 17:58:26]

On 2016/07/11 17:50:57, dfalcantara wrote:
> Was this ready to review?  Saw this in my review queue but didn't get an email
> about it (e.g. Publish+Mail Comments with the new reviewers).

Yeah it's ready. Just published.

[Peter Wen, 2016-07-11 18:03:52]

I'll take a look at this today then.

Somehow publish email went into spam. :/

[Peter Wen, 2016-07-12 13:57:16]

lgtm % nits

This looks really good. At first I was concerned that we always read and flush
the same bytes if it was a mapped file buffer, but since right now that just
crashes, this is strictly better. Thanks for fixing this and adding the test! It
passes locally for me.

https://codereview.chromium.org/2138503002/diff/1/chrome/android/junit/src/or...
File
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java
(right):

https://codereview.chromium.org/2138503002/diff/1/chrome/android/junit/src/or...
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java:5:
package src.org.chromium.chrome.browser.tabstate;
org.chromium.chrome.browser.tabstate

https://codereview.chromium.org/2138503002/diff/1/chrome/android/junit/src/or...
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java:52:
FileInputStream fis = null;
nit: rename to inputStream or stream

https://codereview.chromium.org/2138503002/diff/1/chrome/android/junit/src/or...
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java:91:
FileOutputStream fos = null;
ditto for more descriptive names

[Peter Wen, 2016-07-12 13:59:17]

Also, you'll need to fix presubmit and do this:

andrhung@amazon.com is not in AUTHORS file. If you are a new contributor, please
visit
http://www.chromium.org/developers/contributing-code and read the "Legal"
section

chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java:25:
Public classes and interfaces require JavaDoc comments.

[andrhung, 2016-07-12 20:21:32]

On 2016/07/12 13:59:17, Peter Wen wrote:
> Also, you'll need to fix presubmit and do this:
> 
> mailto:andrhung@amazon.com is not in AUTHORS file. If you are a new
contributor, please
> visit
> http://www.chromium.org/developers/contributing-code and read the "Legal"
> section
> 
>
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java:25:
> Public classes and interfaces require JavaDoc comments.

Thanks for the feedback. I've updated the code and published the changes.

[gone, 2016-07-13 17:03:08]

lgtm

https://chromiumcodereview.appspot.com/2138503002/diff/20001/chrome/android/j...
File
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java
(right):

https://chromiumcodereview.appspot.com/2138503002/diff/20001/chrome/android/j...
chrome/android/junit/src/org/chromium/chrome/browser/tabstate/TabStateUnitTest.java:60:
FileChannel.MapMode.READ_ONLY, fileInputStream.getChannel().position(),
nit: indentation is slightly off here

[andrhung, 2016-07-13 17:19:00]

The CQ bit was checked by andrhung@amazon.com

[andrhung, 2016-07-13 17:19:01]

The patchset sent to the CQ was uploaded after l-g-t-m from wnwen@chromium.org
Link to the patchset: https://codereview.chromium.org/2138503002/#ps20001
(title: "renamed variables, added javadoc, updated AUTHORS")

[commit-bot: I haz the power, 2016-07-13 17:19:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 18:46:10]

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

[commit-bot: I haz the power, 2016-07-13 18:46:11]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[andrhung, 2016-07-13 19:45:49]

The CQ bit was checked by andrhung@amazon.com

[commit-bot: I haz the power, 2016-07-13 19:48:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 21:39:11]

Description was changed from

==========
Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815
==========

to

==========
Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815
==========

[commit-bot: I haz the power, 2016-07-13 21:39:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-13 21:39:39]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 21:41:27]

Description was changed from

==========
Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815
==========

to

==========
Creating contents state byte array before opening stream

Changed TabState.saveState to be able to safely write
mapped byte buffers by opening FileOutputStream after
retrieving buffer contents. Removed unnecessary call to
FileChannel.write which is not guaranteed to write all
bytes.

BUG=626815

Committed: https://crrev.com/b6f7a7a894bbced31fb74e75f14364998a610bdc
Cr-Commit-Position: refs/heads/master@{#405299}
==========

[commit-bot: I haz the power, 2016-07-13 21:41:28]

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