Do not save offline copy for error page

Do not save offline copy for error page

BUG=549381
Committed: https://crrev.com/fbc603e471a410f3020b660508c70b2ac01f5535
Cr-Commit-Position: refs/heads/master@{#357488}

[jianli, 2015-10-30 00:51:18]

jianli@chromium.org changed reviewers:
+ fgorski@chromium.org, kkimlabs@chromium.org

[fgorski, 2015-10-30 13:29:54]

lgtm with nit.

https://codereview.chromium.org/1418613016/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java
(right):

https://codereview.chromium.org/1418613016/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java:86:
tab.getUrl(), tab.getWebContents(), tab.isShowingErrorPage(),
nit: I know this may sound crazy, but did you consider passing a tab into
addBookmarkAsync? :)

[jianli, 2015-10-30 20:31:01]

https://codereview.chromium.org/1418613016/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java
(right):

https://codereview.chromium.org/1418613016/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/enhancedbookmarks/EnhancedBookmarkUtils.java:86:
tab.getUrl(), tab.getWebContents(), tab.isShowingErrorPage(),
On 2015/10/30 13:29:54, fgorski wrote:
> nit: I know this may sound crazy, but did you consider passing a tab into
> addBookmarkAsync? :)

I thought about this but unfortunately the test code could not pass a tab.

[jianli, 2015-10-30 21:36:40]

Description was changed from

==========
Do not save offline copty for error page

BUG=549381
==========

to

==========
Do not save offline copy for error page

BUG=549381
==========

[Kibeom Kim (inactive), 2015-10-30 22:07:29]

lgtm

[jianli, 2015-10-30 22:14:44]

The CQ bit was checked by jianli@chromium.org

[jianli, 2015-10-30 22:14:44]

The patchset sent to the CQ was uploaded after l-g-t-m from fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/1418613016/#ps20001
(title: "Fix trybot")

[commit-bot: I haz the power, 2015-10-30 22:16:39]

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

[commit-bot: I haz the power, 2015-10-30 22:29:46]

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

[commit-bot: I haz the power, 2015-10-30 22:29:46]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jianli, 2015-11-02 22:11:45]

jianli@chromium.org changed reviewers:
+ newt@chromium.org

[jianli, 2015-11-02 22:11:46]

Could you please review and approve Java changes? Thanks.

[newt (away), 2015-11-02 23:53:37]

lgtm

[jianli, 2015-11-03 00:02:57]

The CQ bit was checked by jianli@chromium.org

[commit-bot: I haz the power, 2015-11-03 00:04:46]

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

[commit-bot: I haz the power, 2015-11-03 00:41:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-03 00:42:31]

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