Bug 92474 - DragController should ascertain dragClient to be non-null
Summary: DragController should ascertain dragClient to be non-null
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vivek Galatage
URL:
Keywords:
Depends on:
Blocks: 90675
  Show dependency tree / graph
 
Reported: 2012-07-27 02:10 PDT by Vivek Galatage
Modified: 2012-07-30 20:26 PDT (History)
6 users (show)

See Also:

Attachments
Patch (1.43 KB, patch)
2012-07-27 02:13 PDT, Vivek Galatage 		no flags Details | Formatted Diff | Diff
Patch (1.24 KB, patch)
2012-07-27 12:45 PDT, Vivek Galatage 		no flags Details | Formatted Diff | Diff
Patch (1.24 KB, patch)
2012-07-27 12:54 PDT, Vivek Galatage 		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 Vivek Galatage 2012-07-27 02:10:27 PDT 
Page object should ascertain pageClients.dragClient to be non-null
Comment 1 Vivek Galatage 2012-07-27 02:13:32 PDT 
Created attachment 154882 [details]
Patch
Comment 2 Vivek Galatage 2012-07-27 02:23:08 PDT 
When enabled(DRAG_SUPPORT), the page object should ascertain pageClients.dragClient to be non-null. Page object goes on creating dragController with the given dragClient. The dragController assumes the dragClient being always available hence all the calls on dragClient are not checking for null value of m_client. So there are two options:

1. If the dragClient is still optional when ENABLE(DRAG_SUPPORT), then we should migrate the creation of dragController from Page::Page() constructor's initialization list inside the constructor body to create the controller only when a non-null dragClient is sent. a-la

if (pageClients.dragClient)
   m_dragController = DragController::create(this, pageClients.dragClient);

2. If this is mandatory when ENABLE(DRAG_SUPPORT), then we can simply put an ASSERT statement in Page::Page().

Please let me know your thoughts on the same.
Comment 3 Vivek Galatage 2012-07-27 12:45:42 PDT 
Created attachment 155026 [details]
Patch
Comment 4 Tony Chang 2012-07-27 12:48:15 PDT 
Comment on attachment 155026 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=155026&action=review

> Source/WebCore/ChangeLog:3
> +        Page object should ascertain pageClients.dragClient to be non-null

This description is no longer correct.  Otherwise, this change seems fine.
Comment 5 Vivek Galatage 2012-07-27 12:54:52 PDT 
Created attachment 155029 [details]
Patch
Comment 6 WebKit Review Bot 2012-07-27 15:38:00 PDT 
Comment on attachment 155029 [details]
Patch

Clearing flags on attachment: 155029

Committed r123927: <http://trac.webkit.org/changeset/123927>
Comment 7 WebKit Review Bot 2012-07-27 15:38:04 PDT 
All reviewed patches have been landed.  Closing bug.