Bug 109061

Summary:

Product:

WebKit

Reporter:

Ruslan Abdikeev <aruslan>

Component:

DOM

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, andersca, beidson, buildbot, ddkilzer, dglazkov, esprehn+autocc, fishd, jamesr, japhet, leviw, mathias, ojan.autocc, philn, rniwa, syoichi, tkent, tkent+wkapi, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

111266

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Ruslan Abdikeev

Reported 2013-02-06 09:24:40 PST

No apple-touch-icons are fetched (and, hence, used) for CNN.con, workflowy.com etc. The same holds for apple-touch-icon-precomposed. Test steps: Load http://aruslan.me/t1237621jsa/t.html <html><head> <link href="http://www.cnn.com/favicon.ie9.ico" rel="Shortcut Icon" type="image/x-icon" /> <link href="http://i.cdn.turner.com/cnn/.e/img/3.0/global/misc/apple-touch-icon.png" rel="apple-touch-icon" type="image/png" /> <title>Title</title> </head><body>Body.</body></html> Observed results: 1. favicon.ie9.ico is fetched and shown correctly. 2. apple-touch-icon is NOT fetched; instead, the request is made to the website to fetch "default" fallback apple-touch-icon.png. Expected results: 1. favicon.ie9.ico is fetched and shown (no changes). 2. apple-touch-icon from CDN is fetched and shown.

Attachments
Patch (1.43 KB, patch) 2013-02-06 09:40 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (4.99 KB, patch) 2013-02-06 11:47 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (7.11 KB, patch) 2013-02-06 14:55 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (14.23 KB, patch) 2013-02-06 18:42 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (16.32 KB, patch) 2013-02-11 17:55 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (16.18 KB, patch) 2013-02-11 18:36 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (18.60 KB, patch) 2013-02-13 10:03 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (18.45 KB, patch) 2013-02-13 20:02 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (18.44 KB, patch) 2013-02-19 18:08 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (18.50 KB, patch) 2013-02-22 14:48 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (22.44 KB, patch) 2013-02-26 12:32 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (22.07 KB, patch) 2013-03-01 08:50 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (22.14 KB, patch) 2013-03-01 18:48 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Patch (22.11 KB, patch) 2013-03-04 10:09 PST, Ruslan Abdikeev	no flags	Details Formatted Diff Diff
Show Obsolete (13) View All Add attachment proposed patch, testcase, etc.

Ruslan Abdikeev

Comment 1 2013-02-06 09:40:36 PST

Created attachment 186872 [details] Patch

Ruslan Abdikeev

Comment 2 2013-02-06 11:47:18 PST

Created attachment 186887 [details] Patch

WebKit Review Bot

Comment 3 2013-02-06 12:51:01 PST

Comment on attachment 186887 [details] Patch Attachment 186887 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16388449 New failing tests: fast/dom/icon-url-list-apple-touch.html

Build Bot

Comment 4 2013-02-06 12:51:50 PST

Comment on attachment 186887 [details] Patch Attachment 186887 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16393364

Ruslan Abdikeev

Comment 5 2013-02-06 14:55:52 PST

Created attachment 186926 [details] Patch

Brady Eidson

Comment 6 2013-02-06 16:31:34 PST

Comment on attachment 186926 [details] Patch Why are we making a change that causes all desktop webkit clients to download a non-standard icon that was meant for mobile devices?

Ruslan Abdikeev

Comment 7 2013-02-06 16:36:31 PST

(In reply to comment #6) > (From update of attachment 186926 [details]) > Why are we making a change that causes all desktop webkit clients to download a non-standard icon that was meant for mobile devices? Desktop clients shouldn't define ENABLE(TOUCH_ICON_LOADING). When TOUCH_ICON_LOADING is not enabled, LinkRelAttribute ignores <link rel> for non-standard icons. As the result, Document.cpp doesn't have them anyway. This patch is to fix the error that causes all TOUCH_ICON_LOADING-enabled clients to never get touch icons.

Brady Eidson

Comment 8 2013-02-06 17:16:09 PST

Good point, but... There is a comment inside Document::ironURLs() that says: // Include any icons where type = link, rel = "shortcut icon". Admittedly this is an example of why comments aren't good enforcement over more obvious naming in code. But that point aside, this patch changes that behavior. This method is about "shortcut icons" that should be considered in the course of normal favicon loading whereas apple-touch-icons were always meant to be loaded out of band from normal page loading. We should rename iconURLs to shortcutIconURLs and leave it's behavior as-is. If you need to get at the apple-touch-icons for whatever reason, they can be in a different accessor. You could also refactor the current accessor to take an IconType parameter and leverage that for code reuse between the current behavior and the new behavior you hope to add.

Ruslan Abdikeev

Comment 9 2013-02-06 17:56:02 PST

(In reply to comment #8) > We should rename iconURLs to shortcutIconURLs and leave it's behavior as-is. If you need to get at the apple-touch-icons for whatever reason, they can be in a different accessor. > You could also refactor the current accessor to take an IconType parameter and leverage that for code reuse between the current behavior and the new behavior you hope to add. Agreed; I'm going to rename existing iconURLs() to shortcutIconURLs() keeping its semantics and add iconURLs(int iconTypes) that accepts a bitmask of IconTypes. Additionally, I'll extend Internals.idl, fix tests and surrounding call-sites. Are these names and parameters fine by you?

Brady Eidson

Comment 10 2013-02-06 18:09:53 PST

Sounds reasonable.

Ruslan Abdikeev

Comment 11 2013-02-06 18:42:59 PST

Created attachment 186976 [details] Patch

Ruslan Abdikeev

Comment 12 2013-02-06 18:45:26 PST

(In reply to comment #10) > Sounds reasonable. Please take a look, thanks!

Build Bot

Comment 13 2013-02-06 20:06:45 PST

Comment on attachment 186976 [details] Patch Attachment 186976 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16417050

Build Bot

Comment 14 2013-02-06 22:08:29 PST

Comment on attachment 186976 [details] Patch Attachment 186976 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16409082

Build Bot

Comment 15 2013-02-06 23:05:24 PST

Comment on attachment 186976 [details] Patch Attachment 186976 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16414112

Build Bot

Comment 16 2013-02-06 23:10:10 PST

Comment on attachment 186976 [details] Patch Attachment 186976 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16403099

Brady Eidson

Comment 17 2013-02-07 08:57:31 PST

I'll hold off on review until ews is cleared up

Ruslan Abdikeev

Comment 18 2013-02-11 08:14:06 PST

(In reply to comment #17) > I'll hold off on review until ews is cleared up It fails to build on gtk/mac/mac-wk2/win with Undefined symbols for architecture x86_64: "__ZN7WebCore8Document8iconURLsEv" Strangely, this means that something refers to (now gone) iconURLs(void), but only at the link time. Otherwise it would have failed to compile the call to iconURLs(). Do we use precompiled libraries on gtk/mac/win? How could I trigger a clobber build or something?

Ruslan Abdikeev

Comment 19 2013-02-11 09:52:17 PST

gtk-ews fails with the reverse: ./.libs/libWebCoreInternals.a(./.libs/../Source/WebCore/testing/.libs/libWebCoreInternals_la-Internals.o):Internals.cpp:function WebCore::Internals::iconURLs(WebCore::Document*, int) const: error: undefined reference to 'WebCore::Document::iconURLs(int)' Looks like it uses correct headers, but links against a stale library.

Kent Tamura

Comment 20 2013-02-11 15:37:24 PST

(In reply to comment #18) > (In reply to comment #17) > > I'll hold off on review until ews is cleared up > > It fails to build on gtk/mac/mac-wk2/win with > > Undefined symbols for architecture x86_64: > "__ZN7WebCore8Document8iconURLsEv" WebCore in these ports export only specific symbols. See http://trac.webkit.org/wiki/ExportingSymbols .

Ruslan Abdikeev

Comment 21 2013-02-11 17:55:17 PST

Created attachment 187737 [details] Patch

Levi Weintraub

Comment 22 2013-02-11 18:04:15 PST

(In reply to comment #21) > Created an attachment (id=187737) [details] > Patch Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :(

Ruslan Abdikeev

Comment 23 2013-02-11 18:36:25 PST

Created attachment 187746 [details] Patch

Ruslan Abdikeev

Comment 24 2013-02-11 18:42:45 PST

(In reply to comment #22) > (In reply to comment #21) > > Created an attachment (id=187737) [details] [details] > > Patch > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( Thanks, Levi, rebase FTW.

Levi Weintraub

Comment 25 2013-02-11 22:39:42 PST

(In reply to comment #24) > (In reply to comment #22) > > (In reply to comment #21) > > > Created an attachment (id=187737) [details] [details] [details] > > > Patch > > > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( > > Thanks, Levi, rebase FTW. Woo, green!

Ruslan Abdikeev

Comment 26 2013-02-12 15:15:19 PST

(In reply to comment #25) > (In reply to comment #24) > > (In reply to comment #22) > > > (In reply to comment #21) > > > > Created an attachment (id=187737) [details] [details] [details] [details] > > > > Patch > > > > > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( > > > > Thanks, Levi, rebase FTW. > > Woo, green! What are my next steps? On Mac it says "pass" and then it fails (?), on Mac-WK2 it fails as well.

Build Bot

Comment 27 2013-02-13 04:15:29 PST

Comment on attachment 187746 [details] Patch Attachment 187746 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/16454806

Ruslan Abdikeev

Comment 28 2013-02-13 10:03:06 PST

Created attachment 188107 [details] Patch

Ruslan Abdikeev

Comment 29 2013-02-13 13:20:47 PST

Why I don't see Win in the EWS? Green is good. Green is healthy.

Ruslan Abdikeev

Comment 30 2013-02-13 20:02:48 PST

Created attachment 188252 [details] Patch

Ruslan Abdikeev

Comment 31 2013-02-13 20:04:21 PST

(In reply to comment #25) > (In reply to comment #24) > > (In reply to comment #22) > > > (In reply to comment #21) > > > > Created an attachment (id=187737) [details] [details] [details] [details] > > > > Patch > > > > > > Heads up in case you didn't notice, but this patch doesn't end up applying, so EWS gave up :( > > > > Thanks, Levi, rebase FTW. > > Woo, green! Rebased and re-submitted (the previous patch is green on all platforms, but for some reason EWS never put it to Win). Assuming Win is green this time, this should be a final version.

Ruslan Abdikeev

Comment 32 2013-02-19 18:08:39 PST

Created attachment 189218 [details] Patch

Levi Weintraub

Comment 33 2013-02-21 12:10:56 PST

Brady, would you take another look?

Ruslan Abdikeev

Comment 34 2013-02-22 14:48:16 PST

Created attachment 189837 [details] Patch

Ruslan Abdikeev

Comment 35 2013-02-22 14:51:26 PST

Brady -- would you take another look please? Rebase. The last 4 patches are exactly the same, but first the "win" EWS bot had unrelated problems (eventually became "green"), and now "cr-android" EWS bot is failing and "mac" build had some problems apparently.

Brady Eidson

Comment 36 2013-02-25 10:59:57 PST

Comment on attachment 189837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189837&action=review > Source/WebCore/dom/Document.cpp:4466 > +const Vector<IconURL>& Document::shortcutIconURLs() > +{ > + // Include any icons where type = link, rel = "shortcut icon". > + return iconURLs(Favicon); > +} Where does the "Favicon" symbol come from and... > Source/WebCore/dom/Document.cpp:4468 > +const Vector<IconURL>& Document::iconURLs(int iconTypes) ...why is this not an enum? This should be an enum

Ruslan Abdikeev

Comment 37 2013-02-25 11:26:45 PST

Comment on attachment 189837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189837&action=review >> Source/WebCore/dom/Document.cpp:4466 >> +} > > Where does the "Favicon" symbol come from and... from WebCore::IconType enum (defined in Source/WebCore/dom/IconURL.h). >> Source/WebCore/dom/Document.cpp:4468 >> +const Vector<IconURL>& Document::iconURLs(int iconTypes) > > ...why is this not an enum? > > This should be an enum It's a bitmask of requested icon types (this matches IconController::urlsForTypes(int iconTypes) API).

Ruslan Abdikeev

Comment 38 2013-02-26 09:46:18 PST

(In reply to comment #36) > (From update of attachment 189837 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189837&action=review > > > Source/WebCore/dom/Document.cpp:4466 > > +const Vector<IconURL>& Document::shortcutIconURLs() > > +{ > > + // Include any icons where type = link, rel = "shortcut icon". > > + return iconURLs(Favicon); > > +} > > Where does the "Favicon" symbol come from and... > > > Source/WebCore/dom/Document.cpp:4468 > > +const Vector<IconURL>& Document::iconURLs(int iconTypes) > > ...why is this not an enum? > > This should be an enum Hi Brady, "Favicon" symbol comes from WebCore::IconType enum (defined in Source/WebCore/dom/IconURL.h), and >> +const Vector<IconURL>& Document::iconURLs(int iconTypes) isn't enum because it's a bitmask of requested icon types. This matches IconController::urlsForTypes(int iconTypes) API. Please let me know if anything should be adjusted here. Otherwise, I'd appreciate if you could approve this patch. Yours, -- Ruslan.

Brady Eidson

Comment 39 2013-02-26 11:15:00 PST

(In reply to comment #38) > (In reply to comment #36 > > Hi Brady, > "Favicon" symbol comes from WebCore::IconType enum (defined in Source/WebCore/dom/IconURL.h), and > >> +const Vector<IconURL>& Document::iconURLs(int iconTypes) > isn't enum because it's a bitmask of requested icon types. > This matches IconController::urlsForTypes(int iconTypes) API. Then please call it iconTypesMask, and no harm in updating the old one also. r+ with that change.

Brady Eidson

Comment 40 2013-02-26 11:15:17 PST

Comment on attachment 189837 [details] Patch r+ with the question naming change.

Ruslan Abdikeev

Comment 41 2013-02-26 12:32:28 PST

Created attachment 190340 [details] Patch

WebKit Review Bot

Comment 42 2013-02-26 12:34:41 PST

Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

Ruslan Abdikeev

Comment 43 2013-02-26 12:36:56 PST

(In reply to comment #40) > (From update of attachment 189837 [details]) > r+ with the question naming change. Renamed iconTypes to iconTypesMask in Document, Internals, IconController and Chromium WebFrame.

Adam Barth

Comment 44 2013-02-26 14:50:01 PST

Comment on attachment 190340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190340&action=review > LayoutTests/fast/dom/icon-url-list-apple-touch.html:17 > + var supportsTouchIcons = window.internals.isTouchIconsLoadingSupported(); This is sort of unusual. Typically we just have different expected.txt files when compile-time options change rather than having the test automagically adapt.

Adam Barth

Comment 45 2013-02-26 15:11:22 PST

Comment on attachment 190340 [details] Patch API change LGTM.

Ruslan Abdikeev

Comment 46 2013-03-01 08:50:14 PST

Created attachment 190974 [details] Patch

Ruslan Abdikeev

Comment 47 2013-03-01 14:10:39 PST

(In reply to comment #44) > (From update of attachment 190340 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=190340&action=review > > > LayoutTests/fast/dom/icon-url-list-apple-touch.html:17 > > + var supportsTouchIcons = window.internals.isTouchIconsLoadingSupported(); > > This is sort of unusual. Typically we just have different expected.txt files when compile-time options change rather than having the test automagically adapt. Hi Adam and Brady, PTAL: I removed test auto-adaptation to apple-touch-icon loading and created two -expected -- one in the cross-platform with favicon only, and one in chromium with favicon and apple-touch-icon. Yours, -- Ruslan.

WebKit Review Bot

Comment 48 2013-03-01 15:25:01 PST

Comment on attachment 190974 [details] Patch Attachment 190974 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16877059 New failing tests: fast/dom/icon-url-list-apple-touch.html

WebKit Review Bot

Comment 49 2013-03-01 16:17:05 PST

Comment on attachment 190974 [details] Patch Attachment 190974 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16796239 New failing tests: fast/dom/icon-url-list-apple-touch.html

Ruslan Abdikeev

Comment 50 2013-03-01 18:48:48 PST

Created attachment 191088 [details] Patch

Ruslan Abdikeev

Comment 51 2013-03-01 18:54:28 PST

Moved the expectations from chromium/ to chromium-android/ due to chromium-linux failing because it doesn't ENABLE TOUCH_ICON_LOADING. I believe some of the chromium-mac targets could be enabling touch icons loading, in which case the expectations should be copied there from chromium-android. To summarize, I believe the auto-adapting test was a simpler design and it didn't require heavy maintenance.

Adam Barth

Comment 52 2013-03-02 23:32:51 PST

Comment on attachment 191088 [details] Patch This patch looks good. Thank you for taking the expected.txt route. I can understand why it seems like the adapting test might be easier when writing the test, but this approach is consistent with how we manage other platform differences and is easier to maintain in the long run.

WebKit Review Bot

Comment 53 2013-03-03 01:56:08 PST

Comment on attachment 191088 [details] Patch Clearing flags on attachment: 191088 Committed r144567: <http://trac.webkit.org/changeset/144567>

WebKit Review Bot

Comment 54 2013-03-03 01:56:16 PST

All reviewed patches have been landed. Closing bug.

WebKit Review Bot

Comment 55 2013-03-03 02:29:49 PST

Re-opened since this is blocked by bug 111266

Ryosuke Niwa

Comment 56 2013-03-03 04:01:13 PST

(In reply to comment #55) > Re-opened since this is blocked by bug 111266 The patch caused Apple Windows build failures: e.g. 19>LINK : C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\bin\DumpRenderTree.dll not found or not built by the last incremental link; performing full link 19> Creating library C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\DumpRenderTree.lib and object C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\lib\DumpRenderTree.exp 19>WebCoreTestSupport.lib(JSInternals.obj) : error LNK2019: unresolved external symbol "class JSC::JSValue __cdecl WebCore::toJS(class JSC::ExecState *,class WebCore::JSDOMGlobalObject *,class WebCore::CSSStyleDeclaration *)" (?toJS@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@PAVJSDOMGlobalObject@1@PAVCSSStyleDeclaration@1@@Z) referenced in function "__int64 __fastcall WebCore::jsInternalsPrototypeFunctionComputedStyleIncludingVisitedInfo(class JSC::ExecState *)" (?jsInternalsPrototypeFunctionComputedStyleIncludingVisitedInfo@WebCore@@YI_JPAVExecState@JSC@@@Z) http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio It wasn’t a simple missing symbol error. It looked like something wasn’t compliling.

Ruslan Abdikeev

Comment 57 2013-03-04 08:28:45 PST

(In reply to comment #56) > The patch caused Apple Windows build failures: http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio > > It wasn’t a simple missing symbol error. It looked like something wasn’t compliling. Thank you for jumping in on the issue! Given that the breakage wasn't caused by this patch (reverting this patch didn't help, and the build was fixed subsequently with http://trac.webkit.org/changeset/144573), I'm going to re-land it Objections?

Ruslan Abdikeev

Comment 58 2013-03-04 08:58:16 PST

(In reply to comment #57) > (In reply to comment #56) > > The patch caused Apple Windows build failures: > http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio > > > > It wasn’t a simple missing symbol error. It looked like something wasn’t compliling. > > Thank you for jumping in on the issue! > > Given that the breakage wasn't caused by this patch (reverting this patch didn't help, and the build was fixed subsequently with http://trac.webkit.org/changeset/144573), I'm going to re-land it > > Objections? P.S. The revert (http://trac.webkit.org/changeset/144572) failed with the same link errors as this patch: http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63457 The real fix was submitted in a subsequent patch (http://trac.webkit.org/changeset/144573) and made the tree green: http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63458 That is, unless I'm reading it wrong.

Ruslan Abdikeev

Comment 59 2013-03-04 10:09:42 PST

Created attachment 191267 [details] Patch

Ruslan Abdikeev

Comment 60 2013-03-04 10:37:12 PST

(In reply to comment #59) > Created an attachment (id=191267) [details] > Patch This is a rebased patch. Previously the patch failed to compile on apple-win, because WebKitBuild\Debug\obj\WebCore\DerivedSources\JSInternals.cpp had to be regenerated, but there was an unrelated issue preventing this from happening. The build just before the revert successfully compiled everything related to this patch, but was blocked on the other issue: http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63456/steps/compile-webkit/logs/stdio Reverting the patch didn't help, as the unrelated issue remained. Asking for permission to re-land the patch.

Adam Barth

Comment 61 2013-03-04 13:06:51 PST

> Asking for permission to re-land the patch. The http://build.webkit.org/waterfall?show=Apple%20Win%20Debug%20(Build) bot is current not compiling. I'd prefer to re-land your patch when the bot is compiling so that we can verify that it does compile...

Ruslan Abdikeev

Comment 62 2013-03-04 13:08:02 PST

(In reply to comment #61) > > Asking for permission to re-land the patch. > > The http://build.webkit.org/waterfall?show=Apple%20Win%20Debug%20(Build) bot is current not compiling. I'd prefer to re-land your patch when the bot is compiling so that we can verify that it does compile... Sure thing. Thanks, Adam!

Adam Barth

Comment 63 2013-03-04 16:18:54 PST

Comment on attachment 191267 [details] Patch Windows is building again. Let's try landing this patch again.

WebKit Review Bot

Comment 64 2013-03-04 16:44:36 PST

Comment on attachment 191267 [details] Patch Clearing flags on attachment: 191267 Committed r144696: <http://trac.webkit.org/changeset/144696>

WebKit Review Bot

Comment 65 2013-03-04 16:44:44 PST

All reviewed patches have been landed. Closing bug.

Adam Barth

Comment 66 2013-03-04 17:17:39 PST

http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/63524 confirms that this patch builds correct for apple-win.

Note You need to log in before you can comment on or make changes to this bug.