Uploading patch just expose the srcset attribute to javascripts.
there is whatwg discussion thread on "srcset"
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-May/035746.html
ToDo>>
1)It needs to parse the given string for srcset attr.
like we need to keep the parse values of urls and its descriptors in some datastructure(maybe map)
2)user-agents/ports needs to read and compare these values to compare with current viewport settings
to select appropriate image source in "ImageLoader"
Please let me know if any concerns/suggestions to proceed further.

Created attachment 189817[details]
wip_patch_002
Adding some "srcset" string parsing mechanism and storing values in map.
Idea is to parse the url and descriptors values and compare those with
current viewport values to select appropriate image "src" before
ImageLoader requestImage().

Comment on attachment 190345[details]
updated_patch
View in context: https://bugs.webkit.org/attachment.cgi?id=190345&action=review> Source/WebCore/loader/ImageLoader.cpp:216
> + int keyBegin, keyEnd, i = 0;
I don't think we usually do this.
> Source/WebCore/loader/ImageLoader.h:44
> + Valuedensity = 1
Not sure what's going on here but I think it's a capitalization problem.
> Source/WebCore/loader/ImageLoader.h:48
> + int density;
Density? Looking at the spec, I guess this is "pixel density". Still a bit odd.
> Source/WebCore/page/Chrome.h:169
> + AtomicString getImageSource(const HashMap<String, ImageSourceDescriptors>&) const;
If you have to use this long type in lots of places you could typedef it (or not, I'm never clear exactly what our stance on that is).

Comment on attachment 190345[details]
updated_patch
View in context: https://bugs.webkit.org/attachment.cgi?id=190345&action=review
Where are we selecting the correct image?
> Source/WebCore/ChangeLog:9
> + Spec : http://www.whatwg.org/specs/web-apps/current-work/multipage/embedded-content-1.html#the-img-element
Could you refer to the specific version of the spec you used?
> Source/WebCore/dom/Element.cpp:1132
> +const QualifiedName& Element::imageSourceSetAttributeName() const
> +{
> + return srcsetAttr;
> +}
Why do we need this function? Since there is no alternative name to srcsetAttr we should just hardcode it.
> Source/WebCore/html/HTMLImageElement.cpp:308
> + return document()->completeURL(getAttribute(srcsetAttr));
How does this work? completeURL doesn't support srcset syntax. r- because of this.
>> Source/WebCore/loader/ImageLoader.cpp:205
>> + }
>
> This parsing code is messy and way too complicated for a simple format like the srcset descriptor list. For starters, why did you write all those 5-line while loops instead of using String::find?
I completely agree with Maciej here. r- because of this as well.
In addition, this code should probably live in HTMLImageElement instead.
ImageLoader should only be concerned about loading images, not a particular syntax used in an element that loads an image.
> Source/WebCore/loader/ImageLoader.cpp:214
> +void ImageLoader::processImageSourceAttributes(String& urlString, String& descString)
I would also put this on HTMLImageElement.
> Source/WebCore/loader/ImageLoader.h:47
> + int width;
> + int height;
Can we ever have negative width & height?

FWIW, I think it would be better to tackle a subset of the srcset="" microsyntax and feature set, at least initially. The x specifier has a clear and immediate value (HiDPI), whereas it's less clear if the w and h specifiers are the right solution to the problems they set out to solve.

(In reply to comment #12)
> FWIW, I think it would be better to tackle a subset of the srcset="" microsyntax and feature set, at least initially. The x specifier has a clear and immediate value (HiDPI), whereas it's less clear if the w and h specifiers are the right solution to the problems they set out to solve.
Agreed. It would still be extremely useful and on par feature-wise with CSS -image-set

See a first version of my patch in attachement. For now it only supports the x specifier (as suggested by edward and dean). It includes tests and expected files.
Changes are documented in the corresponding ChangeLogs. Feel free to give me your feedbacks, all suggestions are welcome :)

Created attachment 206775[details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Comment on attachment 206861[details]
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=206861&action=review
Great work! That is quite something for a first patch :)
Sorry, I did not get the time to do a proper review today. Some quick comments bellow.
I think we should also add a Feature Flag (http://trac.webkit.org/wiki/AddingFeatures) if we want to mature this feature.
> Source/WebCore/html/HTMLImageElement.cpp:136
> + const String& string = static_cast<String>(value);
static_cast<const String&>
> Source/WebCore/html/HTMLImageElement.cpp:138
> + ImageWithScale image;
I would declare this where is it used instead.
> Source/WebCore/html/HTMLImageElement.cpp:141
> + string.split(",", srcSet);
"," -> ','
I would move the declaration of srcSet just above this line to reduce the span declaration<->use.
> Source/WebCore/html/HTMLImageElement.cpp:142
> + for (size_t i = 0; i < srcSet.size(); i++) {
i++ -> ++i
> Source/WebCore/html/HTMLImageElement.cpp:146
> + srcSet[i].stripWhiteSpace().split(" ", data);
" " -> ' '
> Source/WebCore/html/HTMLImageElement.cpp:148
> + if (data[1].endsWith("x")) {
"x" -> 'x'
> Source/WebCore/html/HTMLImageElement.cpp:165
> + std::sort(m_imageSet.begin(), m_imageSet.end(), compareByScaleFactor);
Shouldn't the sort be stable?
What if the srcset gives several input for the same scale factor? The one we chose should not depends on the sort algorithm.
> Source/WebCore/html/HTMLImageElement.cpp:184
> + updateImageSet(value);
> + updateBestImageForScaleFactor();
Shouldn't this also be run when srcAttr changes?
> Source/WebCore/html/HTMLImageElement.h:117
> + AtomicString imageURL;
Does this really need to be Atomic?
> Source/WebCore/html/HTMLImageElement.h:130
> + Vector<ImageWithScale> m_imageSet;
The naming is not ideal, because m_imageSet is not a Set :)
> LayoutTests/ChangeLog:16
> + * platform/mac/fast/hidpi/image-srcset-simple-expected.png:
> + * platform/mac/fast/hidpi/image-srcset-simple-expected.txt:
> + * platform/mac/fast/hidpi/image-srcset-src-selection-expected.png:
> + * platform/mac/fast/hidpi/image-srcset-src-selection-expected.txt:
I would have more test coverage. We should try to cover as much as possible. e.g.:
-Various invalid input.
-Valid src, invalid input on the srcset.
-If you have a src (so scaleFactor = 1) and srcset also defines something for scaleFactor = 1, which image is selected.
-Changing the attributes dynamically from JavaScript.
-Removing the src attribute from JavaScript.
> LayoutTests/fast/hidpi/image-srcset-src-selection.html:34
> + <div>This test passes if the div below is a blue 100px square when the deviceScaleFactor is 1. It simply ensures that the src attribute is taken into account by the selection algorithm when this one is processing the images candidates</div>
Missing period.

(In reply to comment #32)
> (From update of attachment 206861[details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206861&action=review
>
> Great work! That is quite something for a first patch :)
> Sorry, I did not get the time to do a proper review today. Some quick comments bellow.
>
> I think we should also add a Feature Flag (http://trac.webkit.org/wiki/AddingFeatures) if we want to mature this feature.
I will look at it
> I would have more test coverage. We should try to cover as much as possible. e.g.:
> -Various invalid input.
> -Valid src, invalid input on the srcset.
> -If you have a src (so scaleFactor = 1) and srcset also defines something for scaleFactor = 1, which image is selected.
> -Changing the attributes dynamically from JavaScript.
> -Removing the src attribute from JavaScript.
>
> > LayoutTests/fast/hidpi/image-srcset-src-selection.html:34
> > + <div>This test passes if the div below is a blue 100px square when the deviceScaleFactor is 1. It simply ensures that the src attribute is taken into account by the selection algorithm when this one is processing the images candidates</div>
>
> Missing period.
I agree, working on it
Except these two points above, all others issues are fixed.

Created attachment 207007[details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3

Created attachment 207027[details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08 Port: mac-mountainlion Platform: Mac OS X 10.8.3

Does it make sense for you to test that images exist ? I mean , we cannot apply the selection algorithm on a set of images if one of them does not exist. It does not make sense, imho. However nothing is specified in the spec about this part.

(In reply to comment #41)
> <img src> doesn't test for existence; you simply get a broken image if the image isn't there. <img srcset> is specced to behave in the same way. Please don't make up different behavior.
Ok, it's noted. thanks

Created attachment 207531[details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.3

AFAICT, this patch doesn't add srcset support to the PreloadScanner, most probably leading to double download in cases where a blocking script is present at the page's head.
Would it be best to add PreloadScanner support as a patch on this bug, or on a separate bug?

Created attachment 207611[details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4

I disagree. I think any updates to the preloader should be done in a separate patch. I've filed
https://bugs.webkit.org/show_bug.cgi?id=119360
to cover the changes (and will upload a patch in a moment).
Meanwhile this patch breaks src="data:.." urls, as demonstrated by the test results above. I'm trying to fix this.

(In reply to comment #53)
> Meanwhile this patch breaks src="data:.." urls, as demonstrated by the test results above. I'm trying to fix this.
This is due to a minor flaw in the algorithm, and will require a specification update. I now suggest we land this patch as is (great work Romain) and then some followup patches:
- the attribute splitting needs to take into account more than just spaces. The values might be separated by tabs or newlines.
- in order not to break data: urls that may (often!) have embedded COMMA characters, change the algorithm to require at least one space in the run of characters that make up an image candidate string. The spec requires at least one of the candidate selectors to be present, so there must be a space.
- update the HTMLPreloadScanner to handle srcset
Ben mentioned he also has some optimisations in mind.
Once I have patches ready for the above I'll formally review this patch and land it, then followup. I think it is mandatory that we don't land just this patch because all images with data: src attributes will break.