Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Fix Brackets Live Development Issues with Chrome OSX #359

Closed
wants to merge 36 commits into from

Conversation

fungl164
Copy link
Contributor

This provides better real-time detection/separation of Chrome debug sessions, as well as better connectivity + window/tab management from within Brackets.

This requires brackets pull request adobe/brackets#5569

…a (e.g. instead of manually parsing process-related arguments).
@redmunds
Copy link
Contributor

This seems to work, but I'm hitting a crash. Try this:

  1. Open Brackets and a Chrome window
  2. Start LD and it opens a second Chrome window
  3. Stop LD and it closes Chrome window that it opened
  4. Switch to Chrome and press Cmd-Q to quit

Results
Brackets crashes. Let me know if you want me to post a crash log.

FYI, I was already happy with the functionality of how LD was working with Chrome, so I'm not sure what your latest changes are trying to fix. Maybe try reverting commits back to this comment and work on figuring out the @try/@catch problem.

@fungl164
Copy link
Contributor Author

fungl164 commented Nov 1, 2013

@redmunds @couzteau Your reports confirmed we no longer need to check for 'enableRemoteDebugging' since this is true all the time now. :)

Let me know if your problems still persist after this last update...thnxs.

@redmunds
Copy link
Contributor

redmunds commented Nov 2, 2013

Yes, that fixes it for me. I will start my final code review of the brackets-shell code.

}
}
return false;
return GetLiveBrowser() ? YES : NO;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function signature has a return of bool, so this should be:

    return (GetLiveBrowser()) ? true : false;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@redmunds
Copy link
Contributor

redmunds commented Nov 4, 2013

Done with review.

@couzteau
Copy link
Member

couzteau commented Nov 4, 2013

Works well for me now.


private:
// private so this class cannot be instantiated externally
LiveBrowserMgrMac();
virtual ~LiveBrowserMgrMac();

int m_openLiveBrowserRetryCount;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be initialized in actor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@redmunds
Copy link
Contributor

redmunds commented Nov 5, 2013

This looks good! I tested on Windows and did not see any problems.

Last thing to do is to squash all of the commits in both brackets-shell and brackets.

@fungl164
Copy link
Contributor Author

fungl164 commented Nov 5, 2013

Want me to force push into this or create a new pull?

@redmunds
Copy link
Contributor

redmunds commented Nov 5, 2013

I find it easier to create a new branch and a new pull request, but if you can squash this branch that's ok too.

@fungl164
Copy link
Contributor Author

fungl164 commented Nov 5, 2013

I agree with you on the former. Created new pull request (see #371).

@redmunds
Copy link
Contributor

redmunds commented Nov 6, 2013

Replaced by #371. Closing this one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants