RESOLVED WONTFIX 53776
Device element patch 1: adding a basic skeleton of the HTMLDeviceElement and the Stream objects. 
https://bugs.webkit.org/show_bug.cgi?id=53776
Summary Device element patch 1: adding a basic skeleton of the HTMLDeviceElement and ...
Leandro Graciá Gil
Reported 2011-02-04 09:30:28 PST
This patch creates the basic skeleton for the asynchronous client-based implementation of the device element that we proposed. This is designed to be the second of a set of small patches. Please see https://bugs.webkit.org/show_bug.cgi?id=53595 for the first one. All functionality and specific details will be included in later patches.
Attachments
Patch (50.35 KB, patch)
2011-02-04 10:18 PST, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Patch (125.46 KB, patch)
2011-03-09 11:22 PST, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Patch (125.44 KB, patch)
2011-03-09 11:26 PST, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Patch (122.48 KB, patch)
2011-03-11 09:17 PST, Leandro Graciá Gil 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Leandro Graciá Gil
Comment 1 2011-02-04 10:18:50 PST
Created attachment 81242 [details] Patch
Gustavo Noronha (kov)
Comment 2 2011-02-04 11:07:38 PST
Attachment 81242 [details] did not build on gtk: Build output: http://queues.webkit.org/results/7701071
Build Bot
Comment 3 2011-02-04 11:31:40 PST
Attachment 81242 [details] did not build on win: Build output: http://queues.webkit.org/results/7698578
Eric Seidel (no email)
Comment 4 2011-02-05 01:55:05 PST
Comment on attachment 81242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81242&action=review build failures. > Source/WebCore/dom/DeviceType.cpp:43 > +// Avoid using static Strings. Code should not run on WebKit load. > +const char* DeviceType::s_names[] = > +{ > + "invalid", > + "media" > +}; We've historically done this with AtomicStrings and an init() method. But in this case for only 2 types, I'm not sure having an array actually buys us anything besides complexity. > Source/WebCore/dom/Stream.h:49 > + static PassRefPtr<Stream> create(HTMLDeviceElement*, const String& url); Should these urls be strings or KURLs? > Source/WebCore/html/HTMLDeviceElement.cpp:87 > + HTMLElement::defaultEventHandler(event); > + return; This could just be one line.
Leandro Graciá Gil
Comment 5 2011-03-09 11:22:12 PST
Created attachment 85202 [details] Patch
Leandro Graciá Gil
Comment 6 2011-03-09 11:26:38 PST
Created attachment 85203 [details] Patch
Leandro Graciá Gil
Comment 7 2011-03-09 11:27:36 PST
(In reply to comment #4) > (From update of attachment 81242 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=81242&action=review > > build failures. > > > Source/WebCore/dom/DeviceType.cpp:43 > > +// Avoid using static Strings. Code should not run on WebKit load. > > +const char* DeviceType::s_names[] = > > +{ > > + "invalid", > > + "media" > > +}; > > We've historically done this with AtomicStrings and an init() method. But in this case for only 2 types, I'm not sure having an array actually buys us anything besides complexity. > > > Source/WebCore/dom/Stream.h:49 > > + static PassRefPtr<Stream> create(HTMLDeviceElement*, const String& url); > > Should these urls be strings or KURLs? > > > Source/WebCore/html/HTMLDeviceElement.cpp:87 > > + HTMLElement::defaultEventHandler(event); > > + return; > > This could just be one line. Fixed. Adding also the session manager code to make or proposal clear.
Leandro Graciá Gil
Comment 8 2011-03-11 09:17:00 PST
Created attachment 85481 [details] Patch
Leandro Graciá Gil
Comment 9 2011-03-11 09:52:17 PST
Decoupled the DeviceSession to the API calls to make clear that the object is managed in WebCore, not by the embedder. The object itself has been also considerably simplified by removing specific information that was not required. Also deleted some code not being currently used.
Eric Seidel (no email)
Comment 10 2011-04-18 09:12:37 PDT
Comment on attachment 85481 [details] Patch Cleared review? from attachment 85481 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Note You need to log in before you can comment on or make changes to this bug.