[webkit-reviews] review denied: [Bug 86962] [GTK][DRT] Wrong cache model is taken in LayoutTestController's setCacheModel() : [Attachment 142898] patch v2 (fixed bug title changelog)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 21 11:52:45 PDT 2012


Gustavo Noronha (kov) <gns at gnome.org> has denied Mikhail Pozdnyakov
<mikhail.pozdnyakov at intel.com>'s request for review:
Bug 86962: [GTK][DRT] Wrong cache model is taken in LayoutTestController's
setCacheModel()
https://bugs.webkit.org/show_bug.cgi?id=86962

Attachment 142898: patch v2 (fixed bug title changelog)
https://bugs.webkit.org/attachment.cgi?id=142898&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=142898&action=review


> Tools/DumpRenderTree/gtk/LayoutTestControllerGtk.cpp:655
>      case 3:
> -	   webkit_set_cache_model(WEBKIT_CACHE_MODEL_DOCUMENT_BROWSER);
> +	   webkit_set_cache_model(WEBKIT_CACHE_MODEL_WEB_BROWSER);

Your change looks right to me, but... that case should be 2 not 3, and 2 should
be falling down to the ASSERT_NOT_REACHED(), so I'm wondering if this is used
at all. Hah, it's not used indeed:

kov at goiaba ~/s/W/LayoutTests> git grep setCacheModel
fast/dom/HTMLScriptElement/nested-execution.html:		
layoutTestController.setCacheModel(0); // WebCacheModelDocumentViewer
platform/chromium/test_expectations.txt:// This test requires
LayoutTestController.setCacheModel, which we don't
platform/efl/Skipped:# EFL's LayoutTestController does not implement
setCacheModel
platform/wk2/Skipped:# WebKitTestRunner needs
layoutTestController.setCacheModel

Please fix the case too.


More information about the webkit-reviews mailing list