17640 – eliminate WebCoreFrameBridge

RESOLVED FIXED 17640

eliminate WebCoreFrameBridge

https://bugs.webkit.org/show_bug.cgi?id=17640

Summary eliminate WebCoreFrameBridge

Darin Adler

Reported 2008-03-02 12:42:40 PST

We need to get rid of the bridge.

Attachments
eliminate the WebCore -> WebKit half of the bridge (148.63 KB, patch) 2008-03-02 13:18 PST, Darin Adler	no flags	Details Formatted Diff Diff
smaller piece for Adam to review (32.52 KB, patch) 2008-03-03 10:38 PST, Darin Adler	no flags	Details Formatted Diff Diff
work in progress (ready to land, but needs to be broken in pieces for review) (121.16 KB, patch) 2008-03-03 10:48 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for just issuePasteCommand (7.08 KB, patch) 2008-03-04 03:25 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for just dashboardRegionsChanged (10.92 KB, patch) 2008-03-04 03:59 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch moving code from the bridge to frame loader client on the WebKit side (42.01 KB, patch) 2008-03-04 04:11 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for just window method (6.72 KB, patch) 2008-03-04 04:16 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for just installInFrame method (12.08 KB, patch) 2008-03-04 04:33 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for scrollOverflow method (7.39 KB, patch) 2008-03-04 04:42 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for createFrameView method (4.80 KB, patch) 2008-03-04 04:52 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for reapplyStyles method (5.56 KB, patch) 2008-03-04 05:01 PST, Darin Adler	no flags	Details Formatted Diff Diff
work in progress (69.94 KB, patch) 2008-03-04 07:41 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for runOpenPanel (15.13 KB, patch) 2008-03-04 08:54 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch for custom highlight calls (17.02 KB, patch) 2008-03-07 19:41 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch that moves Java applet loading off the bridge (14.20 KB, patch) 2008-03-08 07:17 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch that moves keyboard mode off the bridge (15.86 KB, patch) 2008-03-08 07:29 PST, Darin Adler	no flags	Details Formatted Diff Diff
patch that removes the rest of the WebKit -> WebCore bridge (28.25 KB, patch) 2008-03-10 22:13 PDT, Darin Adler	no flags	Details Formatted Diff Diff
patch that removes use of the bridge to go from an NSView to a WebCore::Frame (73.65 KB, patch) 2008-03-11 12:34 PDT, Darin Adler	no flags	Details Formatted Diff Diff
revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame (74.15 KB, patch) 2008-03-11 14:34 PDT, Darin Adler	no flags	Details Formatted Diff Diff
patch that removes every last use of the bridge in WebCore (24.10 KB, patch) 2008-03-11 20:35 PDT, Darin Adler	no flags	Details Formatted Diff Diff
patch that drives a stake through the heart of the bridge (249.76 KB, patch) 2008-03-12 10:03 PDT, Darin Adler	andersca: review+	Details Formatted Diff Diff
Show Obsolete (20) View All Add attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2008-03-02 13:18:11 PST

Created attachment 19485 [details] eliminate the WebCore -> WebKit half of the bridge

Adam Roben (:aroben)

Comment 2 2008-03-03 06:37:15 PST

Comment on attachment 19485 [details] eliminate the WebCore -> WebKit half of the bridge There's a whole lotta patch here! Any chance we can get this broken up and reviewed/landed incrementally?

Darin Adler

Comment 3 2008-03-03 09:25:11 PST

(In reply to comment #2) > There's a whole lotta patch here! Any chance we can get this broken up and > reviewed/landed incrementally? Sure, it could be broken up into 2 pieces, or 10, or 30.

Darin Adler

Comment 4 2008-03-03 10:38:38 PST

Created attachment 19496 [details] smaller piece for Adam to review

Adam Roben (:aroben)

Comment 5 2008-03-03 10:43:39 PST

Comment on attachment 19496 [details] smaller piece for Adam to review r=me

Darin Adler

Comment 6 2008-03-03 10:48:20 PST

Comment on attachment 19496 [details] smaller piece for Adam to review Landed.

Darin Adler

Comment 7 2008-03-03 10:48:50 PST

Created attachment 19497 [details] work in progress (ready to land, but needs to be broken in pieces for review)

Darin Adler

Comment 8 2008-03-04 03:25:48 PST

Created attachment 19515 [details] patch for just issuePasteCommand

Darin Adler

Comment 9 2008-03-04 03:59:58 PST

Created attachment 19517 [details] patch for just dashboardRegionsChanged

Darin Adler

Comment 10 2008-03-04 04:11:31 PST

Created attachment 19518 [details] patch moving code from the bridge to frame loader client on the WebKit side

Darin Adler

Comment 11 2008-03-04 04:16:03 PST

Created attachment 19519 [details] patch for just window method

Darin Adler

Comment 12 2008-03-04 04:33:42 PST

Created attachment 19520 [details] patch for just installInFrame method

Darin Adler

Comment 13 2008-03-04 04:42:33 PST

Created attachment 19521 [details] patch for scrollOverflow method

Darin Adler

Comment 14 2008-03-04 04:52:15 PST

Created attachment 19522 [details] patch for createFrameView method

Darin Adler

Comment 15 2008-03-04 05:01:13 PST

Created attachment 19523 [details] patch for reapplyStyles method

Adam Roben (:aroben)

Comment 16 2008-03-04 06:31:23 PST

Comment on attachment 19515 [details] patch for just issuePasteCommand r=me

Adam Roben (:aroben)

Comment 17 2008-03-04 06:33:12 PST

Comment on attachment 19517 [details] patch for just dashboardRegionsChanged r=me

Adam Roben (:aroben)

Comment 18 2008-03-04 06:46:43 PST

Comment on attachment 19518 [details] patch moving code from the bridge to frame loader client on the WebKit side This patch seems to be missing the removal of the implementation of -[WebFrameBridge windowObjectCleared] Other than that, r=me

Adam Roben (:aroben)

Comment 19 2008-03-04 06:47:29 PST

Comment on attachment 19519 [details] patch for just window method r=me

Adam Roben (:aroben)

Comment 20 2008-03-04 06:49:24 PST

Comment on attachment 19520 [details] patch for just installInFrame method r=me

Adam Roben (:aroben)

Comment 21 2008-03-04 06:50:41 PST

Comment on attachment 19521 [details] patch for scrollOverflow method r=me

Adam Roben (:aroben)

Comment 22 2008-03-04 06:51:34 PST

Comment on attachment 19522 [details] patch for createFrameView method r=me

Adam Roben (:aroben)

Comment 23 2008-03-04 06:52:15 PST

Comment on attachment 19523 [details] patch for reapplyStyles method r=me

Darin Adler

Comment 24 2008-03-04 07:19:20 PST

Comment on attachment 19515 [details] patch for just issuePasteCommand Landed.

Darin Adler

Comment 25 2008-03-04 07:19:35 PST

Comment on attachment 19496 [details] smaller piece for Adam to review Landed.

Darin Adler

Comment 26 2008-03-04 07:19:49 PST

Comment on attachment 19517 [details] patch for just dashboardRegionsChanged Landed.

Darin Adler

Comment 27 2008-03-04 07:20:05 PST

Comment on attachment 19518 [details] patch moving code from the bridge to frame loader client on the WebKit side Landed.

Darin Adler

Comment 28 2008-03-04 07:20:59 PST

Comment on attachment 19519 [details] patch for just window method Landed.

Darin Adler

Comment 29 2008-03-04 07:22:30 PST

Comment on attachment 19520 [details] patch for just installInFrame method Landed.

Darin Adler

Comment 30 2008-03-04 07:25:34 PST

Comment on attachment 19521 [details] patch for scrollOverflow method Landed.

Darin Adler

Comment 31 2008-03-04 07:30:05 PST

Comment on attachment 19522 [details] patch for createFrameView method Landed.

Darin Adler

Comment 32 2008-03-04 07:35:21 PST

Comment on attachment 19523 [details] patch for reapplyStyles method Landed.

Darin Adler

Comment 33 2008-03-04 07:41:06 PST

Created attachment 19524 [details] work in progress

Darin Adler

Comment 34 2008-03-04 08:54:46 PST

Created attachment 19525 [details] patch for runOpenPanel

Adam Roben (:aroben)

Comment 35 2008-03-05 10:22:53 PST

Comment on attachment 19525 [details] patch for runOpenPanel +void ChromeClient::runOpenPanel(PassRefPtr<FileChooser>) +{ +} Why is this empty implementation needed in ChromeMac.mm? Can't you make this method declaration pure virtual and just have WebChromeClient implement it? r=me regardless

Darin Adler

Comment 36 2008-03-07 12:44:54 PST

(In reply to comment #35) > Why is this empty implementation needed in ChromeMac.mm? Can't you make this > method declaration pure virtual and just have WebChromeClient implement it? I could, but then I'd have to add code to the "SVG empty clients". I'm really not sure what the best design for this is going forward, but for now I'm not going to make this pure virtual.

Darin Adler

Comment 37 2008-03-07 13:02:16 PST

Comment on attachment 19525 [details] patch for runOpenPanel Clearing review flag since I committed revision 30875.

Darin Adler

Comment 38 2008-03-07 19:41:30 PST

Created attachment 19606 [details] patch for custom highlight calls

Darin Adler

Comment 39 2008-03-08 06:11:05 PST

Comment on attachment 19606 [details] patch for custom highlight calls Committed revision 30900.

Darin Adler

Comment 40 2008-03-08 07:17:40 PST

Created attachment 19609 [details] patch that moves Java applet loading off the bridge

Darin Adler

Comment 41 2008-03-08 07:29:35 PST

Created attachment 19610 [details] patch that moves keyboard mode off the bridge

Sam Weinig

Comment 42 2008-03-10 08:37:02 PDT

Comment on attachment 19609 [details] patch that moves Java applet loading off the bridge I am really not sure what is the correct possition for the * here, but since the following line has it next to the type, also for an ObjC type, it would be nice to be consistent. #if PLATFORM(MAC) + virtual jobject javaApplet(NSView *) { return 0; } virtual NSCachedURLResponse* willCacheResponse(DocumentLoader*, unsigned long identifier, NSCachedURLResponse*) const = 0; r=me

Sam Weinig

Comment 43 2008-03-10 08:49:36 PDT

Comment on attachment 19610 [details] patch that moves keyboard mode off the bridge I am not too crazy about having ChromeClient implementations in ChromeMac.mm, but I assume it is to get around having to implement it for the SVGImageEmptyClient. If that is the case, we should have a new file, perhaps ChromeClientMac.mm, to contain these base implementations. Could you also explain in the changelog why it is okay to remove - ASSERT_MAIN_THREAD(); from the finalize method of WebFrameBridge. r=me

Darin Adler

Comment 44 2008-03-10 09:40:16 PDT

(In reply to comment #43) > I am not too crazy about having ChromeClient implementations in ChromeMac.mm, > but I assume it is to get around having to implement it for the > SVGImageEmptyClient. Sure, that's right. Not just for SVGImage, but in general to have a policy of default behavior rather than not compiling if you choose not to implement one of the client functions. I'm not completely sure whether pure virtual functions or functions with a default implementation are best. > If that is the case, we should have a new file, perhaps > ChromeClientMac.mm, to contain these base implementations. Sure, I will create a new file. > Could you also explain in the changelog why it is okay to remove > - ASSERT_MAIN_THREAD(); > from the finalize method of WebFrameBridge. Yes.

Darin Adler

Comment 45 2008-03-10 10:02:35 PDT

Comment on attachment 19609 [details] patch that moves Java applet loading off the bridge Committed revision 30935.

Darin Adler

Comment 46 2008-03-10 10:38:52 PDT

Comment on attachment 19610 [details] patch that moves keyboard mode off the bridge Committed revision 30939.

Darin Adler

Comment 47 2008-03-10 22:13:06 PDT

Created attachment 19656 [details] patch that removes the rest of the WebKit -> WebCore bridge

Darin Adler

Comment 48 2008-03-11 12:34:43 PDT

Created attachment 19674 [details] patch that removes use of the bridge to go from an NSView to a WebCore::Frame

Darin Adler

Comment 49 2008-03-11 14:34:59 PDT

Created attachment 19677 [details] revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame

Darin Adler

Comment 50 2008-03-11 17:52:12 PDT

Comment on attachment 19656 [details] patch that removes the rest of the WebKit -> WebCore bridge Committed revision 30975.

Anders Carlsson

Comment 51 2008-03-11 18:22:34 PDT

Comment on attachment 19677 [details] revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame r=me

Darin Adler

Comment 52 2008-03-11 18:25:44 PDT

Comment on attachment 19677 [details] revised patch that removes use of the bridge to go from an NSView to a WebCore::Frame Committed revision 30976.

Darin Adler

Comment 53 2008-03-11 20:35:18 PDT

Created attachment 19693 [details] patch that removes every last use of the bridge in WebCore

Sam Weinig

Comment 54 2008-03-11 20:40:44 PDT

Comment on attachment 19693 [details] patch that removes every last use of the bridge in WebCore r=me. YEAH!

Darin Adler

Comment 55 2008-03-11 22:29:38 PDT

Comment on attachment 19693 [details] patch that removes every last use of the bridge in WebCore Committed revision 30980.

Darin Adler

Comment 56 2008-03-12 10:03:04 PDT

Created attachment 19700 [details] patch that drives a stake through the heart of the bridge

Anders Carlsson

Comment 57 2008-03-12 10:17:58 PDT

Comment on attachment 19700 [details] patch that drives a stake through the heart of the bridge r=me

Darin Adler

Comment 58 2008-03-12 11:47:14 PDT

I can't land this until I get home tonight, because I saw at least one crash running regression tests. It should be trivial to fix. So Hyatt, feel free to land your patch. I'll merge with it.

Darin Adler

Comment 59 2008-03-12 20:49:26 PDT

Committed revision 31014.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Mac

OS OS X 10.5

Product WebKit

Component WebCore Misc.

Assignee

Darin Adler

Reported

2008-03-02 12:42 PST

Modified

2008-03-12 20:49 PDT History

CC List

2 users Show

URL

Keywords

Depends on

Blocks