Added GetFormattedUrlFromOriginalDistillerUrl to DomDistillerTabUtils

Added GetFormattedUrlFromOriginalDistillerUrl to DomDistillerTabUtils

These changes create support for GetFormattedUrlFromOriginalDistillerUrl,
which is necessary for reformatting the original distiller URL for
display in the Location Bar.

BUG=383630
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289166

[nyquist, 2014-08-08 22:21:54]

https://codereview.chromium.org/454063003/diff/20001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/20001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:31: jstring
GetFormattedUrlFromOriginalDistillerUrl(JNIEnv* env,
Could you instead use ScopedJavaLocalRef<jstring>?

[sunangel, 2014-08-09 01:34:09]

https://codereview.chromium.org/454063003/diff/20001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/20001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:31: jstring
GetFormattedUrlFromOriginalDistillerUrl(JNIEnv* env,
On 2014/08/08 22:21:54, nyquist wrote:
> Could you instead use ScopedJavaLocalRef<jstring>?

Done.

[robliao, 2014-08-10 02:16:50]

https://codereview.chromium.org/454063003/diff/40001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/40001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:43: // Note that we can't
unescape spaces here, because if the user copies this
The comment should immediately precede the discussed line.

[sunangel, 2014-08-11 15:28:48]

https://codereview.chromium.org/454063003/diff/40001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/40001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:43: // Note that we can't
unescape spaces here, because if the user copies this
On 2014/08/10 02:16:50, robliao wrote:
> The comment should immediately precede the discussed line. 

Done.

[nyquist, 2014-08-11 20:35:48]

https://codereview.chromium.org/454063003/diff/60001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/60001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:41: // Note that we can't
unescape spaces here, because if the user copies this
I believe this is about net::FormatUrl, not the part about length. Could you
move the comment down to where Robert mentioned, immediately preceding the
discussed line? (ie. just above FormatUrl)?

https://codereview.chromium.org/454063003/diff/60001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:54:
base::android::ScopedJavaLocalRef<jstring> j_formatted_url =
Is it possible to just
return base::android::ConverTUT...().Release()
here?

[sunangel, 2014-08-12 01:54:39]

https://codereview.chromium.org/454063003/diff/60001/chrome/browser/dom_disti...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/60001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:41: // Note that we can't
unescape spaces here, because if the user copies this
On 2014/08/11 20:35:48, nyquist wrote:
> I believe this is about net::FormatUrl, not the part about length. Could you
> move the comment down to where Robert mentioned, immediately preceding the
> discussed line? (ie. just above FormatUrl)?

Done.

https://codereview.chromium.org/454063003/diff/60001/chrome/browser/dom_disti...
chrome/browser/dom_distiller/tab_utils_android.cc:54:
base::android::ScopedJavaLocalRef<jstring> j_formatted_url =
On 2014/08/11 20:35:48, nyquist wrote:
> Is it possible to just
> return base::android::ConverTUT...().Release()
> here?

Done.

[nyquist, 2014-08-12 21:57:50]

lgtm with one nit

https://codereview.chromium.org/454063003/diff/100001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/100001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/tab_utils_android.cc:16: #include
"content/public/common/content_constants.h"
Nit: What is this include for?

[sunangel, 2014-08-12 22:00:01]

https://codereview.chromium.org/454063003/diff/100001/chrome/browser/dom_dist...
File chrome/browser/dom_distiller/tab_utils_android.cc (right):

https://codereview.chromium.org/454063003/diff/100001/chrome/browser/dom_dist...
chrome/browser/dom_distiller/tab_utils_android.cc:16: #include
"content/public/common/content_constants.h"
content::kMaxURLDisplayChars
On 2014/08/12 21:57:50, nyquist wrote:
> Nit: What is this include for?

[sunangel, 2014-08-12 22:34:04]

The CQ bit was checked by sunangel@chromium.org

[commit-bot: I haz the power, 2014-08-12 22:45:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sunangel@chromium.org/454063003/100001

[commit-bot: I haz the power, 2014-08-13 03:04:03]

Change committed as 289166