Bug 98503 - [WK2][WKTR] Avoid duplication of UIClient callbacks for main page and other pages
Summary: [WK2][WKTR] Avoid duplication of UIClient callbacks for main page and other p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks: 98256
  Show dependency treegraph
 
Reported: 2012-10-05 02:37 PDT by Chris Dumez
Modified: 2012-10-05 09:16 PDT (History)
7 users (show)

See Also:


Attachments
Patch (7.41 KB, patch)
2012-10-05 03:46 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (5.16 KB, patch)
2012-10-05 05:25 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2012-10-05 02:37:27 PDT
We currently often have to use different UIClient callbacks for the main page and for other pages. The reason for this is that the callbacks for the main page are passed the Controller as clientInfo, while the other pages callbacks are passed the PlatformWebView as clientInfo.

This causes code duplication and it is error prone. Someone may not pay attention to this detail and use the same callback for both the main view and the other views, and therefore not properly cast the clientInfo callback argument.

As a matter of fact, the existing runModal() callback is wrong: it is used for both the Main page UIClient and the other pages UIClient. However, the callback unconditionally casts clientInfo to a PlatformWebView (which is wrong for the main page).

I propose that we have a TestController as clientInfo for other pages UIClient callbacks, for consistency and to avoid useless code duplication. We just need to introduce a static HashTable so that we can retrieve the PlatformWebView corresponding to a given page in those callbacks.
Comment 1 Chris Dumez 2012-10-05 03:46:11 PDT
Created attachment 167302 [details]
Patch
Comment 2 Chris Dumez 2012-10-05 03:54:20 PDT
Comment on attachment 167302 [details]
Patch

I will try the pass the main view for the main page callbacks instead.
Comment 3 Chris Dumez 2012-10-05 05:25:43 PDT
Created attachment 167314 [details]
Patch

Much simpler proposal.
Comment 4 WebKit Review Bot 2012-10-05 09:16:47 PDT
Comment on attachment 167314 [details]
Patch

Clearing flags on attachment: 167314

Committed r130515: <http://trac.webkit.org/changeset/130515>
Comment 5 WebKit Review Bot 2012-10-05 09:16:52 PDT
All reviewed patches have been landed.  Closing bug.