Bug 86962 - [GTK][DRT] Wrong cache model is taken in LayoutTestController's setCacheModel()
Summary: [GTK][DRT] Wrong cache model is taken in LayoutTestController's setCacheModel()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-20 04:29 PDT by Mikhail Pozdnyakov
Modified: 2012-05-22 11:17 PDT (History)
2 users (show)

See Also:

Attachments
patch (1.47 KB, patch)
2012-05-20 04:37 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
patch v2 (fixed bug title changelog) (1.47 KB, patch)
2012-05-20 04:43 PDT, Mikhail Pozdnyakov 		gustavo: review-
gustavo: commit-queue-
Details | Formatted Diff | Diff
patch v3 (fixed case also) (1.48 KB, patch)
2012-05-22 00:27 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2012-05-20 04:29:57 PDT 
In order to be consistent with other Webkit ports GTK's LayoutTestController::setCacheModel(int cacheModel) should set WEBKIT_CACHE_MODEL_WEB_BROWSER if cacheModel is equal to 3, currently sets WEBKIT_CACHE_MODEL_DOCUMENT_BROWSER.
Comment 1 Mikhail Pozdnyakov 2012-05-20 04:37:22 PDT 
Created attachment 142897 [details]
patch
Comment 2 Mikhail Pozdnyakov 2012-05-20 04:43:14 PDT 
Created attachment 142898 [details]
patch v2 (fixed bug title changelog)
Comment 3 Gustavo Noronha (kov) 2012-05-21 11:51:50 PDT 
Comment on attachment 142898 [details]
patch v2 (fixed bug title changelog)

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@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.
Comment 4 Mikhail Pozdnyakov 2012-05-22 00:27:35 PDT 
Created attachment 143211 [details]
patch v3 (fixed case also)
Comment 5 Mikhail Pozdnyakov 2012-05-22 00:29:58 PDT 
(In reply to comment #3)
> Please fix the case too.
Wow! I did not noticed that issue.. Thanks for review.
Comment 6 WebKit Review Bot 2012-05-22 11:17:39 PDT 
Comment on attachment 143211 [details]
patch v3 (fixed case also)

Clearing flags on attachment: 143211

Committed r118007: <http://trac.webkit.org/changeset/118007>
Comment 7 WebKit Review Bot 2012-05-22 11:17:44 PDT 
All reviewed patches have been landed.  Closing bug.