[CEF 2623] Upgrade to latest CEF #544

nethip · 2015-12-22T08:05:48Z

This PR updates CEF version inside Brackets to ~~2357~~ 2623. I could not upgrade to latest version as the latest MAC binaries are not yet posted to cebuilds.com. I will update this PR with the latest verison once they are built and available at cefbuilds.com.

One important thing to note is that, starting from 2357, CEF is going to support only 64 bit version of MAC. If you are building out of this branch, 64 bit binary is going to be generated on MAC.

nethip · 2015-12-22T08:06:51Z

@marcelgerber We might want to revert the mouse scroll hack once this PR is merged with master.

ingorichter · 2015-12-22T19:23:43Z

I think we need to make some changes to the xcode project. I'm currently on 10.11.2 with Xcode 7.2 and compilation failed, because C++ Exceptions are currently not explicitly enabled for the project. I've changed it locally and now it compiles and brackets is able to start. I can't verify the changes on older OSX versions right now.

nethip · 2015-12-23T05:23:06Z

@ingorichter Thanks for trying out this branch and for the workaround as well. I tested this with XCode 6. It seems to pass fine.

Let me setup XCode 7.0 and try building this branch.

ficristo · 2015-12-24T14:15:26Z

I tried this PR on Windows and cannot find the Brackets executable in Release/Debug folders, only the wow_helper.exe.
Is this because I used VS2015?
I simply run grunt, I need additional commands?

PS: Will this works on Linux? I think it would be nice to update it...

ficristo · 2015-12-24T15:58:29Z

The version 2526 of cef should support VS2015. I'll wait for it.

nethip · 2015-12-24T18:04:34Z

@ficristo That is right! We have not ported our build scripts to be able to build on VS2015. I will update the scripts and will let you know once it is ready. Thanks for building and posting the results.

ficristo · 2016-01-09T11:41:35Z

If I want to test the new version of CEF, but that version it should be uploaded to a private server first. Right?
@nethip How I can point to a public source? Or can you upload the new version?

nethip · 2016-01-11T11:51:40Z

@ficristo

Refer to https://github.com/adobe/brackets-shell/blob/master/Gruntfile.js#L226. This is where we set the download location. You can change this to any other domain (maybe dropbox). But be sure to name the binary in this format.

cef_binary_3.2171.1902_macosx32.zip

Notice the "3.2171.1902" part. This version number would be got from https://github.com/adobe/brackets-shell/blob/master/Gruntfile.js#L227. So you might want to change the version number as well. At the end, both the URL location and the version number are combined along with _<platform32/64> to form the final download URL.

But I am afriad just this is not enough. The app-shell code needs to be changed too. I would suggest you could branch out of prashant/cef-upgrade-latest and do your changes on top of it.

Oh, by the way you could also look at this PR #440 which adds support for VS 2013, which I think is a mandate, if we want to shift to the latest CEF.

Let me know how it goes.

ficristo · 2016-01-11T12:09:31Z

@nethip Thanks for the answer. I'll see if I have the time to look at it.

ficristo · 2016-01-23T16:20:16Z

It seems easy enough to add support to vs2015: I've updated the gyp in tree to latest master plus I've removed a couple of warning.
I've tried to update to CEF 2526 but I got lost trying to update follow the new structure...
Just to realize that CEF 2357 has changed it folders structure too.
I don't see any changes here: any reason to not reflect that structure here?
Will everything work without any change?

nethip · 2016-01-25T05:39:27Z

@ficristo Could you mention which folders have changed? I have VS 2010 and the windows build seems to go fine. I will try updating to 2526 as well (now that the builds are available at https://cefbuilds.com).

ficristo · 2016-01-25T07:42:44Z

More or less like this:

In cef 3.2171

cef_binary
- cef_client/
  - res/
  - cefclient_win.cpp

In cef 3.2526

cef_binary
- cef_client/
  - browser/
  - common/
  - renderer/
  - resources/
    - win/
  - cefclient_win.cc

nethip · 2016-05-02T10:08:55Z

@ingorichter @vdhawal I have fixed all the issues and now the PR is ready for review. We want to merge this for Brackets 1.7. Will you guys be able to review this PR?

ficristo · 2016-05-02T13:15:51Z

Hi @nethip I have a couple of questions:

so can you confirm that aren't necessary any changes to the structure of the folders / files?
were it too much work to update to a newer release of CEF?
if OSX users were able to install Brackets on a 32bit platform what will happen to them? (I tried to see if the architecture were available in the HealtData Report but seems not)
Thank you.

vdhawal · 2016-05-02T14:13:35Z

Changes look good to me. I did build on 10.11.4 with Xcode 7.3 and the app launched seamlessly. OnBeforePopup is also getting invoked when a popup window is launched. 👍

nethip · 2016-05-02T17:41:48Z

Thanks @vdhawal for reviewing this trying to build it .

And thanks to @ingorichter for pointing out the issue with exceptions.

@ficristo

so can you confirm that aren't necessary any changes to the structure of the folders / files?

I think the grunt setup is going to take of creating necessary structure. We did a good job at segregating CEF specific files with our appshell specific files. You could have a look at this.

were it too much work to update to a newer release of CEF?

That is a good point you brought up. I will give it a spin.

if OSX users were able to install Brackets on a 32bit platform what will happen to them? (I tried to see if >the architecture were available in the HealtData Report but seems not) Thank you.

Unfortunately, CEF stopped supporting 32bit binaries for Mac OS. Hence have to switch to 64 bit only for MAC, from 2357. One solution I can think of is,the user can build Brackets shell from a prior commits, once this PR is merged. We will be retaining the CEF 2171 binaries on our S3 server.

ficristo · 2016-05-02T18:11:53Z

That is a good point you brought up. I will give it a spin.

I was only interested for the vs2015 compatibility. I think this will already bring some fixes (I think at you scroll issue!).
I can wait the next release.

For the platform thing, I kinda hope that the upgrade will fail gracefully, maybe with some sort of message. But without numbers is rough to know if is an important issue.

@nethip Thanks fo the answers!

nethip · 2016-05-03T19:30:31Z

@ficristo I have created a new branch c32ee49 for upgrading to CEF 2623. Would you mind building this branch on Windows? I have verified this on MAC.

ficristo · 2016-05-03T21:23:55Z

I've gave a try.
I've merged #546 and #549 to solve some issues.
But I still have some errors, all of them are LNK2019 and LNK2001.
The following are a couple of examples:
appshell_extensions.obj : error LNK2019: unresolved external symbol __imp__GetParent@4 referenced in function "void * __cdecl getMenuParent(class CefRefPtr<class CefBrowser>)" (?getMenuParent@@YAPAXV?$CefRefPtr@VCefBrowser@@@@@Z)
appshell_extensions_win.obj : error LNK2001: unresolved external symbol __imp__GetParent@4

…fault toolbar, provided by OS, is not allowing for any custom drawing of the window title

…e toolbar

nethip · 2016-05-11T19:10:40Z

@ficristo Yeah this PR was not meant to be mixed with VS2015 changes.

Is it OK if we can work on your VS2015 related PRs post 1.7? One of the major hurdles we are facing is to do with upgrading our build system, as the system is currently equipped to churn out builds using VS2010.

ficristo · 2016-05-11T19:15:53Z

@nethip yes, no problem 😄

nethip · 2016-05-12T10:02:52Z

Tagging @abhijitapte

marcelgerber · 2016-05-12T16:40:47Z

Will CEF 2623 come to Linux as well? Would be very nice to finally have the Linux version updated!

nethip · 2016-05-12T16:48:28Z

@marcelgerber unfortunately not. :(. I guess Linux has to be worked ground up since lot has changed in cef-client.

abhijitapte · 2016-05-12T16:49:51Z

appshell/cefclient_mac.mm

@@ -650,7 +662,6 @@ - (void)createApp:(id)object {
  content_rect = [mainWnd contentRectForFrameRect:[mainWnd frame]];

  // Configure the rest of the window
-  [mainWnd setTitle:WINDOW_TITLE];


@nethip where is this being set now?

Drawing of window title is going to be taken care of CustomToolBarView. Actually Apple officially dropped support for installing views in NSThemeFrame and CustomToolBarView is getting added as a subview inside NSThemFrame. I think going forward we need to investigate and figure out how to officially get a dark toolbar. There is a [mainWnd setAppearance: NSAppearanceNameVibrantLight] but that is going to give UI toolbar a more darker look. But this is the actual way to do it. Also there is a way to generate custom appearances and install it via setAppearance. That is something I will investigate in the future.

For now to answer your question, OS is drawing the title along with CustomTitleBarView and hence need to suppress one of them. We require CustomTitleBarView for dark toolbar.

nethip · 2016-05-12T17:05:30Z

@marcelgerber Will you be able to build out of this branch and pull adobe/brackets#12415 to check if the fast scrolling issue is resolved?

marcelgerber · 2016-05-12T17:17:00Z

I have already done so and I can say, the scroll issue is finally gone! Good job on your side!

ingorichter · 2016-05-12T17:18:04Z

@marcelgerber did you test on Windows or OSX?

nethip · 2016-05-12T17:27:07Z

@marcelgerber Oh that is a relief. And thanks to you for fixing the actual problem via adobe/brackets#12415. Really appreciate it!

@ingorichter I guess he would have built this on Windows itself.

abhijitapte · 2016-05-12T17:28:48Z

appshell/cefclient_mac.mm

+        // Since we have nuked the title, we will have
+        // to set the string back as we are hiding the
+        // custom title bar.
+        [window setTitle:[customTitlebar titleString]];


@nethip I did run into a console message viz., the following. Was that happening earlier also?

2016-05-12 22:52:11.306 Brackets[2370:84493] NSWindow warning: adding an unknown subview: <CustomTitlebarView: 0x608000131440>. Break on NSLog to debug.
2016-05-12 22:52:11.351 Brackets[2370:84493] Call stack:
(
0 AppKit 0x00007fff851c88bb -[NSThemeFrame addSubview:] + 107
1 AppKit 0x00007fff851c8600 -[NSView addSubview:positioned:relativeTo:] + 211
2 Brackets 0x00000001000588bb -[ClientWindowDelegate windowDidBecomeKey:] + 603
3 CoreFoundation 0x00007fff9768ebbc CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER + 12
4 CoreFoundation 0x00007fff9768eb4f ___CFXRegistrationPost_block_invoke + 63
5 CoreFoundation 0x00007fff9768eac7 _CFXRegistrationPost + 407
6 CoreFoundation 0x00007fff9768e832 ___CFXNotificationPost_block_invoke + 50
7 CoreFoundation 0x00007fff9764b5e2 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1922
8 CoreFoundation 0x00007fff9764a835 _CFXNotificationPost + 693
9 Foundation 0x00007fff88399fda -[NSNotificationCenter postNotificationName:object:userInfo:] + 66
10 AppKit 0x00007fff852e6048 -[NSWindow becomeKeyWindow] + 1425
11 AppKit 0x00007fff852e5a75 _NXSendWindowNotification + 252
12 AppKit 0x00007fff852e5378 -[NSWindow _changeKeyAndMainLimitedOK:] + 868
13 AppKit 0x00007fff853af173 -[NSWindow _makeKeyRegardlessOfVisibility] + 98
14 AppKit 0x00007fff852e8429 NSPerformVisuallyAtomicChange + 147
15 AppKit 0x00007fff853af0af -[NSWindow makeKeyAndOrderFront:] + 79
16 Brackets 0x0000000100059a5f -[ClientAppDelegate createApp:] + 2575
17 Foundation 0x00007fff883c7f4e -[NSObject(NSThreadPerformAdditions) performSelector:onThread:withObject:waitUntilDone:modes:] + 1115
18 Foundation 0x00007fff883c7a75 -[NSObject(NSThreadPerformAdditions) performSelectorOnMainThread:withObject:waitUntilDone:] + 131
19 Brackets 0x000000010005b276 main + 3030
20 Brackets 0x0000000100003d54 start + 52
21 ??? 0x0000000000000003 0x0 + 3
)
Got xpc error message: Connection interrupted
2016-05-12 22:54:16.043 Brackets[2370:85805] Communications error: <OS_xpc_error: <error: 0x7fff76257b90> { count = 1, contents =
"XPCErrorDescription" => <string: 0x7fff76257f40> { length = 22, contents = "Connection interrupted" }
}>

This was happening earlier with 10.11 sdk. Please refer to my comment https://github.com/adobe/brackets-shell/pull/544/files/d3cd41c8fb964cd5634b3babdeec9dd25e681861#r63060349

for more explanation on this,

@nethip
There seems to be an issue with custom title bar view in case "Debug > New Brackets Window" is selected and the same is maximized and/or restored. See screenshot below -

Which SDK are you on? I have built this using 10.11 sdk and it seems to work fine.

Oh I see that you are using OS dark theme. Let me give that a try.

I'm on the latest SDK 10.11.

And on OSX 10.11.4

@nethip
I also see that

OS X Deployment Target is 10.6.

Shouldn't that be updated? Changing that causes appshell build to fail. The errors and deprecations should be fixed.
Also npm install for Brackets throws these deprecations -

npm WARN deprecated graceful-fs@1.2.3: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.
npm WARN deprecated npmconf@2.1.1: this package has been reintegrated into npm and is now out of date with respect to npm
npm WARN deprecated graceful-fs@1.1.14: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.
npm WARN deprecated graceful-fs@3.0.8: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.
npm WARN deprecated lodash@1.0.2: lodash@<3.0.0 is no longer maintained. Upgrade to lodash@^4.0.0.

This is weird. can you bring your laptop to the office tomorrow?

About the deprecation and 10.6 target, I think that is something we can work on post 1.7. Agree with you updating the target as well as the removing the npm deprecation warnings.

nethip · 2016-05-12T18:41:44Z

Thanks @abhijitapte in taking some time to review this PR.

abhijitapte · 2016-05-13T00:55:30Z

@nethip please have a look at the video capture. This has the steps for reproducing blank title. I am using the brackets dist folder built using grunt buildafter syncing from origin master by the way.
BracketsShellCapture.zip

…he main window and the fix had to be migrated to popup window as well

nethip · 2016-05-13T03:26:57Z

Great catch @abhijitapte . I fixed the issue and the change should be reflecting in this PR. Thanks for testing this PR so extensively. 👍

abhijitapte · 2016-05-13T09:53:21Z

looks good to me @nethip

swmitra · 2016-05-13T12:50:10Z

LGTM 👍.
Kudos to @nethip in pulling this out and resolving the long pending scrolling issue finally. Great job by @abhijitapte and @marcelgerber for helping in critical review and testing.
Merging...

nethip · 2016-05-15T18:37:11Z

Thanks to @ingorichter @ficristo @vdhawal @abhijitapte who helped in shaping this PR really good. Really appreciate your efforts!

abhijitapte · 2016-05-18T03:23:49Z

@nethip Linux build doesn't work. The CEF binary 2623 zip folder itself is not accessible.

nethip · 2016-05-18T03:59:33Z

@abhijitapte unfortunately app shell on Linux is going to get built from an older branch as it requires some more work. I could upload the binaries to S3. Let me know if you want to have a look at it (which would be great :))

nethip added 2 commits December 8, 2015 17:41

[MAC] Changes required for upgrading CEF to 2357 on MAC

cbb77ee

Chancing cef binary name from macosx32 to macosx64

11d66b7

petetnt mentioned this pull request Jan 6, 2016

Brackets susceptible for Chromium bug #533361 adobe/brackets#11727

Closed

CEF upgrade related changes. Fixed the following issues

25ddc3a

Changes required for upgrading to CEF 2623

c32ee49

nethip added 4 commits May 6, 2016 02:55

Fixing Title drawing issue. Resurrected Traffic lights view,as the de…

adc91b8

…fault toolbar, provided by OS, is not allowing for any custom drawing of the window title

[MAC] Avoiding setTitle on mainWnd, as text getting drawn twice in th…

165ffb1

…e toolbar

Merge branch 'master' into prashant/cef-upgrade-latest

3ce2497

Cleaned up some unwanted code.

d3cd41c

nethip changed the title ~~[Review only] CEF Upgrade~~ CEF Upgrade to 2623 May 11, 2016

nethip mentioned this pull request May 11, 2016

[CEF 2623] Upgrade to latest CEF adobe/brackets#12415

Merged

abhijitapte reviewed May 12, 2016
View reviewed changes

Fixing the title drawing issue for popup window. This was fixed for t…

25a643c

…he main window and the fix had to be migrated to popup window as well

swmitra merged commit 0894803 into master May 13, 2016

swmitra deleted the prashant/cef-upgrade-latest branch May 13, 2016 12:54

nethip restored the prashant/cef-upgrade-latest branch May 24, 2016 03:56

marcelgerber deleted the prashant/cef-upgrade-latest branch August 18, 2016 22:49

marcelgerber restored the prashant/cef-upgrade-latest branch August 18, 2016 22:50

marcelgerber deleted the prashant/cef-upgrade-latest branch June 16, 2017 09:11

This was referenced Aug 30, 2021

[CLOSED] Brackets susceptible for Chromium bug #533361 brackets-archive/bracketsIssues#10153

Open

[CLOSED] [CEF 2623] Upgrade to latest CEF brackets-archive/bracketsIssues#10633

Open

[CEF 2623] Upgrade to latest CEF #544

[CEF 2623] Upgrade to latest CEF #544

Conversation

nethip commented Dec 22, 2015 • edited Loading

nethip commented Dec 22, 2015

ingorichter commented Dec 22, 2015

nethip commented Dec 23, 2015

ficristo commented Dec 24, 2015

ficristo commented Dec 24, 2015

nethip commented Dec 24, 2015

ficristo commented Jan 9, 2016

nethip commented Jan 11, 2016

ficristo commented Jan 11, 2016

ficristo commented Jan 23, 2016

nethip commented Jan 25, 2016

ficristo commented Jan 25, 2016

nethip commented May 2, 2016

ficristo commented May 2, 2016

vdhawal commented May 2, 2016

nethip commented May 2, 2016

ficristo commented May 2, 2016

nethip commented May 3, 2016 • edited Loading

ficristo commented May 3, 2016

nethip commented May 11, 2016

ficristo commented May 11, 2016

nethip commented May 12, 2016

marcelgerber commented May 12, 2016

nethip commented May 12, 2016

Choose a reason for hiding this comment

nethip May 12, 2016 • edited Loading

Choose a reason for hiding this comment

nethip commented May 12, 2016

marcelgerber commented May 12, 2016

ingorichter commented May 12, 2016

nethip commented May 12, 2016

abhijitapte May 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhijitapte May 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhijitapte May 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nethip commented May 12, 2016

abhijitapte commented May 13, 2016

nethip commented May 13, 2016

abhijitapte commented May 13, 2016

swmitra commented May 13, 2016

nethip commented May 15, 2016

abhijitapte commented May 18, 2016

nethip commented May 18, 2016

nethip commented Dec 22, 2015 •

edited

Loading

nethip commented May 3, 2016 •

edited

Loading

nethip May 12, 2016 •

edited

Loading

abhijitapte May 12, 2016 •

edited

Loading

abhijitapte May 12, 2016 •

edited

Loading

abhijitapte May 12, 2016 •

edited

Loading