Extract image URLs in srcset as well

Extract image URLs in srcset as well

BUG=625621
R=mdjones@chromium.org

Committed: 8d8063a210a3d4b9ba36584bcef8ae9d66ed8e1b

[wychen, 2016-08-01 20:48:57]

wychen@chromium.org changed reviewers:
+ mdjones@chromium.org

[wychen, 2016-08-01 20:48:58]

wychen@chromium.org changed reviewers:
+ mdjones@chromium.org

[wychen, 2016-08-01 20:48:58]

PTAL

[wychen, 2016-08-01 20:48:58]

PTAL

[wychen, 2016-08-01 20:59:20]

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
File java/org/chromium/distiller/webdocument/WebImage.java (right):

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
java/org/chromium/distiller/webdocument/WebImage.java:104:
generateOutput(false);
This feels entangled. :(

[mdjones, 2016-08-01 21:14:35]

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
File java/org/chromium/distiller/DomUtil.java (right):

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
java/org/chromium/distiller/DomUtil.java:354: if (srcset == "") {
srcset.isEmpty() or .equals please.

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
File java/org/chromium/distiller/webdocument/WebImage.java (right):

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
java/org/chromium/distiller/webdocument/WebImage.java:104:
generateOutput(false);
On 2016/08/01 20:59:20, wychen wrote:
> This feels entangled. :(

Can we instead cut the top part of generateOutput and put it into a function
called something like cloneAndProcessNode? We could then call that function
lazily in multiple places. Leaving this as-is will cause a lot of confusion
later on.

[wychen, 2016-08-01 21:35:15]

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
File java/org/chromium/distiller/DomUtil.java (right):

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
java/org/chromium/distiller/DomUtil.java:354: if (srcset == "") {
On 2016/08/01 21:14:35, mdjones wrote:
> srcset.isEmpty() or .equals please.

Done.

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
File java/org/chromium/distiller/webdocument/WebImage.java (right):

https://codereview.chromium.org/2203563002/diff/20001/java/org/chromium/disti...
java/org/chromium/distiller/webdocument/WebImage.java:104:
generateOutput(false);
On 2016/08/01 21:14:35, mdjones wrote:
> On 2016/08/01 20:59:20, wychen wrote:
> > This feels entangled. :(
> 
> Can we instead cut the top part of generateOutput and put it into a function
> called something like cloneAndProcessNode? We could then call that function
> lazily in multiple places. Leaving this as-is will cause a lot of confusion
> later on.

Done.

[mdjones, 2016-08-01 21:49:32]

lgtm

[wychen, 2016-08-02 01:34:51]

Description was changed from

==========
Extract image URLs in srcset as well

BUG=625621
==========

to

==========
Extract image URLs in srcset as well

BUG=625621
R=mdjones@chromium.org

Committed: 8d8063a210a3d4b9ba36584bcef8ae9d66ed8e1b
==========

[wychen, 2016-08-02 01:34:53]

Committed patchset #4 (id:60001) manually as
8d8063a210a3d4b9ba36584bcef8ae9d66ed8e1b (presubmit successful).